Skip to content

fix(datatables): keep the per-page control in sync with perPage - #289

Merged
markdumay merged 1 commit into
mainfrom
fix/perpage-select-sync
Jul 31, 2026
Merged

fix(datatables): keep the per-page control in sync with perPage#289
markdumay merged 1 commit into
mainfrom
fix/perpage-select-sync

Conversation

@markdumay

Copy link
Copy Markdown
Contributor

The defect

perPage was passed straight to simple-datatables without being checked against perPageSelect. When the value is not one of the offered options, the select falls back to rendering its first entry while the table pages at the requested size — so the control and the behaviour disagree, and nothing signals it.

The reader sees "5" above a page of 25 rows. The only way to notice is to count them.

Measured on a real site with perPage: 25 against the default options:

data-table-paging-option-perPage = "25"
select.value                     = "5"        <- first option, not 25
visible tbody rows               = 25
select options                   = 5, 10, 20, 50, All

The fix

Insert the value into the option list. That honours the author's intent and makes the control show what is actually happening:

before   select "5",  25 rows, options 5/10/20/50/All
after    select "25", 25 rows, options 5/10/20/25/50/All

Of the three available outcomes — inject, warn and fall back, or render a mismatched value — the last is the worst, because it is silent. Injecting is closest to intent, since perPage is an explicit request.

Details

The value lands in sorted position among the numeric entries, so "All" stays last. Entries may be a bare number or a [label, value] pair (that is how "All" carries -1), so the comparison reads the value in either shape and the sort ignores non-positive values.

A value already present is untouched: perPage: 10 still yields 5/10/20/50/All with the select on 10 and no injection.

Related

A build-time counterpart shipped in mod-blocks#185, which warns at Hugo time when a list block's pagination is not a selectable size. That catches the common authoring path early but cannot cover callers reaching this module directly, or a custom perPageSelect — hence this runtime fix as well.

Checks

pnpm test passes. Both cases exercised in a browser on a real 106-row table.

`perPage` was passed through without being checked against
`perPageSelect`. When the value is not one of the offered options the
select falls back to rendering its first entry while the table pages at
the requested size, so the control and the behaviour disagree and nothing
signals it -- the reader sees "5" above a page of 25 rows, and the only
way to notice is to count them.

Insert the value into the option list instead. That honours the author's
intent and makes the control show what is actually happening. Of the
three available outcomes -- inject, or warn and fall back, or render a
mismatched value -- the last is the worst, because it is silent.

The value lands in sorted position among the numeric entries, so "All"
(a [label, value] pair with value -1) stays last. Entries are compared
through a helper that reads either shape.

Measured on a real site with `perPage: 25` against the default options:

  before   select "5",  25 rows rendered, options 5/10/20/50/All
  after    select "25", 25 rows rendered, options 5/10/20/25/50/All

A value already present is untouched: `perPage: 10` still yields
5/10/20/50/All with the select on 10 and no injection.

`pnpm test` passes.
@markdumay
markdumay merged commit 39cd7fb into main Jul 31, 2026
8 checks passed
@markdumay

Copy link
Copy Markdown
Contributor Author

🎉 This PR is included in version 4.1.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@markdumay
markdumay deleted the fix/perpage-select-sync branch July 31, 2026 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant