Skip to content

[Bug Fix] DataTablePerPageSelect: match shadcn's Select and submit without an inline onchange - #579

Merged
cirdes merged 10 commits into
ruby-ui:mainfrom
pierry01:fix/data-table-per-page-select-without-inline-handler
Nov 6, 2026
Merged

cirdes merged 10 commits into
ruby-ui:mainfrom
pierry01:fix/data-table-per-page-select-without-inline-handler

Conversation

@pierry01

@pierry01 pierry01 commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Related issue

None opened, small self-contained fix.

Description

DataTablePerPageSelect submitted through an inline onchange="this.form.requestSubmit()", which a Content-Security-Policy without 'unsafe-inline' blocks, so changing the page size did nothing there. It now submits through a ruby-ui--data-table-per-page-select Stimulus controller.

After the shadcn audit it also renders as shadcn's dashboard-01 block does: RubyUI::Select with SelectTrigger size: :sm, class: "w-20" labelled by the <label for> and the list opening on top, instead of a NativeSelect. SelectInput carries name/value and the submit controller: picking a size fires change there and the form submits GET ?per_page=N (search and sort kept, page reset) into the Turbo Frame. The public API of the component is unchanged.

The server-driven submit is registered in audit/divergences/data_table.yml, and the component map's DataTablePerPageSelect selector now points at the trigger.

Closed Open
closed light open light
closed dark open dark
shadcn audit · main
shadcn audit  PR #579  ruby-ui/ruby_ui
  base ce88825d  head 43adc9a5  shadcn 8a353667
⚠️ Environment  font: "Inter Variable", "system-ui", -apple-system, "system-ui", "Segoe UI", Roboto, "Helvetica Neue", Arial, "sans-serif" vs "Inter Variable", sans-serif (light)
⚠️ Environment  font: "Inter Variable", "system-ui", -apple-system, "system-ui", "Segoe UI", Roboto, "Helvetica Neue", Arial, "sans-serif" vs "Inter Variable", sans-serif (dark)
✅ Theme  neutral tokens match shadcn
DataTable  (ref: bases/base/blocks/dashboard-01/components/data-table.tsx + style-vega.css @ 8a353667)
  ❌ Look       display: inline-flex
               vega: display: flex
               DataTableColumnToggle — closed/light, open/light, focus/light, after-esc/light, closed/dark, open/dark, focus/dark, after-esc/dark
               ↳ computed styles (shadcn-audit collect)
  ❌ Look       background-color: rgba(0, 0, 0, 0); border-bottom-color: oklch(1 0 0 / 0.1); border-left-color: oklch(1 0 0 / 0.1); border-right-color: oklch(1 0 0 / 0.1); border-top-color: oklch(1 0 0 / 0.1); box-shadow: rgba(0, 0, 0, 0) 0px 0px 0px 0px, rgba(0, 0, 0, 0) 0px 0px 0px 0px, rgba(0, 0, 0, 0) 0px 0px 0px 0px, oklab(0 0 0 / 0) 0px 0px 0px 0px, rgba(0, 0, 0, 0.05) 0px 1px 2px 0px; column-gap: normal; display: inline-block; gap: normal; padding-bottom: 2px; padding-right: 32px; padding-top: 2px; row-gap: normal; text-align: start
               vega: background-color: oklab(1 0 0 / 0.045); border-bottom-color: oklch(1 0 0 / 0.15); border-left-color: oklch(1 0 0 / 0.15); border-right-color: oklch(1 0 0 / 0.15); border-top-color: oklch(1 0 0 / 0.15); box-shadow: rgba(0, 0, 0, 0) 0px 0px 0px 0px, rgba(0, 0, 0, 0) 0px 0px 0px 0px, rgba(0, 0, 0, 0) 0px 0px 0px 0px, rgba(0, 0, 0, 0) 0px 0px 0px 0px, rgba(0, 0, 0, 0.05) 0px 1px 2px 0px; column-gap: 6px; display: flex; gap: 6px; padding-bottom: 8px; padding-right: 8px; padding-top: 8px; row-gap: 6px; text-align: center
               DataTablePerPageSelect renders RubyUI::NativeSelect size :sm (h-7 py-0.5 pr-8 pl-2.5 bg-transparent border-border); the shadcn block renders SelectTrigger size=sm className=w-20 (h-8 gap-1.5 py-2 pr-2 pl-2.5 dark:bg-input/30 border-input). Pre-existing: this PR doesn't touch the select's classes. DataTablePerPageSelect — closed/light, open/light, focus/light, after-esc/light, closed/dark, open/dark, focus/dark, after-esc/dark
               ↳ computed styles (shadcn-audit collect)
  ❌ Look       color: oklch(0.556 0 0); color: oklch(0.708 0 0); display: block; text-align: start
               vega: color: oklch(0.145 0 0); color: oklch(0.985 0 0); display: flex; text-align: left
               DataTableRowCheckbox — closed/light, open/light, focus/light, after-esc/light, closed/dark, open/dark, focus/dark, after-esc/dark
               ↳ computed styles (shadcn-audit collect)
  ⏸ Look       properties: font-family, height, width
               DataTableColumnToggle, DropdownMenuContent, DataTablePerPageSelect, DataTableRowCheckbox — explained by environment diff: font
  ⚠️ ARIA       aria-expanded: (none); aria-haspopup: (none); snapshot: only here: - combobox "Rows per page": |   - option "5" |   - option "10" [selected] |   - option "25" |   - option "50"
               shadcn: aria-expanded: false; aria-haspopup: listbox; snapshot: only here: - combobox "Rows per page": "10"
               Native <select> exposes its options in the accessibility tree and has no aria-expanded/aria-haspopup; Base UI's trigger is a combobox with haspopup=listbox. Accessible name matches ("Rows per page", label[for]). Pre-existing. DataTablePerPageSelect — closed/light, open/light, focus/light, after-esc/light, closed/dark, open/dark, focus/dark, after-esc/dark
               ↳ accessibility tree (shadcn-audit collect)
  ✅ ARIA       DataTableColumnToggle, DropdownMenuContent, DataTableRowCheckbox: accessibility tree matches
               ↳ accessibility tree (shadcn-audit collect)
  ⚠️ Behavior   change on the select submits the form through ruby-ui--data-table-per-page-select#submit (form.requestSubmit) -> GET ?per_page=50; no inline handler in the DOM (0 on* attributes)
               shadcn: onValueChange -> table.setPageSize, client-side, no request
               Verified in the browser (fill 50 -> http://localhost:3179/__audit/data_table?per_page=50). Same outcome, server-driven, as the registered data_table HTML divergence explains for pagination.
               ↳ audit/fixtures/data_table/shadcn.tsx:147
  ✅ Native     native <select> + <label for> (keyboard, typeahead, Esc and the picker come from the browser) — native
               The only JS left is the auto-submit on change, which has no native equivalent. The Stimulus controller replaces the inline onchange, so it also works under a CSP without 'unsafe-inline'.
               ↳ https://developer.mozilla.org/en-US/docs/Web/HTML/Element/select
  ⏸ Behavior   DataTableColumnToggle menu set (arrows, typeahead, Esc focus return, outside click) not exercised in this run: outside this PR's change; it renders RubyUI::DropdownMenu, audited in #630.
  ⏸ Animation  Not exercised: the native select picker is drawn by the browser; the ColumnToggle menu animation belongs to the DropdownMenu audit (#630).
DataTablePerPageSelect  (not in the component map)
  ⏸ Look       Not in the component map. False positive of `target`: the folder comes from docs/app/javascript/controllers/ruby_ui/data_table_per_page_select_controller.js; the component lives in gem/lib/ruby_ui/data_table/ and is audited under data_table.
shadcn audit · this PR
shadcn audit  PR #579  ruby-ui/ruby_ui
  base 1fd93db0  head 32b1f9a8  shadcn 8a353667
⚠️ Environment  font: "Inter Variable", "system-ui", -apple-system, "system-ui", "Segoe UI", Roboto, "Helvetica Neue", Arial, "sans-serif" vs "Inter Variable", sans-serif (light)
⚠️ Environment  font: "Inter Variable", "system-ui", -apple-system, "system-ui", "Segoe UI", Roboto, "Helvetica Neue", Arial, "sans-serif" vs "Inter Variable", sans-serif (dark)
✅ Theme  neutral tokens match shadcn
DataTable  (ref: bases/base/blocks/dashboard-01/components/data-table.tsx + style-vega.css @ 8a353667)
  ❌ Look       display: inline-flex
               vega: display: flex
               Pre-existing, not touched by this PR; follow-up PR. DataTableColumnToggle — closed/light, open/light, focus/light, after-esc/light, closed/dark, open/dark, focus/dark, after-esc/dark
               ↳ computed styles (shadcn-audit collect)
  ❌ Look       display: block; text-align: start
               vega: display: flex; text-align: left
               Pre-existing, not touched by this PR; follow-up PR. DataTableRowCheckbox — closed/light, open/light, focus/light, after-esc/light, closed/dark, open/dark, focus/dark, after-esc/dark
               ↳ computed styles (shadcn-audit collect)
  ⏸ Look       properties: font-family
               DataTableColumnToggle, DropdownMenuContent, DataTablePerPageSelect, DataTableRowCheckbox — explained by environment diff: font
  ✅ ARIA       DataTableColumnToggle, DropdownMenuContent, DataTablePerPageSelect, DataTableRowCheckbox: accessibility tree matches
               ↳ accessibility tree (shadcn-audit collect)
  ✅ Look       DataTablePerPageSelect: RubyUI::SelectTrigger size: :sm class: w-20 (h-8 gap-1.5 py-2 pr-2 pl-2.5 border-input dark:bg-input/30)
               No computed-style difference left on the per-page trigger, light and dark (only the font environment diff).
               ↳ computed styles (shadcn-audit collect)
  ✅ Behavior   click opens with focus on the selected option (10); typeahead 5 -> 50; Enter selects; Esc closes and returns focus to the trigger
               Driven with real clicks/keys on both sides (chrome-devtools).
               ↳ bases/base/ui/select.tsx
  ✅ Behavior   picking a size submits GET ?per_page=N (verified: Enter on 50 -> /__audit/data_table?per_page=50; docs demo frame goes 5 -> 10 rows); no inline handler in the DOM — intentional
               RubyUI's DataTable is server-driven, as for pagination: the page size is a query param the server reads, so the list comes back from the server and the URL can be shared. The Select itself (open, arrows, typeahead, Enter, Esc) behaves as shadcn's.
               ↳ audit/divergences/data_table.yml
  ✅ Props      SelectContent side: :top opens beside the trigger — intentional
               Overlaying the selected item needs custom positioning beyond Floating UI offset/flip/shift; the beside-the-trigger placement is what shadcn falls back to when there is no room.
               ↳ audit/divergences/select.yml
  ✅ Native     the only JS the component adds is the auto-submit on change, via a Stimulus controller on SelectInput
               No native feature submits a form on change; requestSubmit keeps validation and submit events. Works under a CSP without 'unsafe-inline'.
               ↳ https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement/requestSubmit
  ⏸ Behavior   DataTableColumnToggle menu set (arrows, typeahead, Esc focus return, outside click, animation) not exercised here: outside this PR; follow-up PR.

The two ❌ left (Columns trigger and row checkbox display) are pre-existing and go to a separate PR, together with the menu behavior this audit didn't cover.

Testing instructions

  • cd gem && bundle exec rake → 780 runs, 0 failures, StandardRB clean. cd mcp && bundle exec rake → 33 runs, 0 failures.
  • /docs/data_table_demo: open "Rows per page" and pick 10 → the frame reloads with 10 rows.
  • Keyboard: focus the trigger, ArrowDown, type 5 → 50, Enter → ?per_page=50. Esc closes it and focus returns to the trigger.
  • CSP: the rendered table has no on* attribute.

🤖 Generated with Claude Code

…nchange

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pierry01
pierry01 marked this pull request as ready for review October 29, 2026 03:23
@pierry01
pierry01 requested a review from cirdes as a code owner October 29, 2026 03:23

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 5 files

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

@cirdes cirdes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cubic

pierry01 and others added 2 commits October 30, 2026 12:39
…ta is left untouched

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pierry01 pierry01 closed this Nov 3, 2026
@pierry01
pierry01 deleted the fix/data-table-per-page-select-without-inline-handler branch November 3, 2026 17:09
@pierry01
pierry01 restored the fix/data-table-per-page-select-without-inline-handler branch November 3, 2026 17:13
@pierry01 pierry01 reopened this Nov 3, 2026
…e-handler

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pierry01
pierry01 requested a review from cirdes November 3, 2026 17:19

@cirdes cirdes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pierry01, great timing on this PR. Over the last few days I refactored every ruby-ui component to match shadcn exactly, and I created a skill called /shadcn-audit or something close to that. It runs a full comparison between our component and the shadcn one.

Could you run that skill so it takes the screenshots and does the audit, then request review again? Thanks!

@pierry01 pierry01 changed the title [Bug Fix] DataTablePerPageSelect: submit through Stimulus instead of an inline onchange [Bug Fix] DataTablePerPageSelect: match shadcn's Select and submit without an inline onchange Nov 5, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 5 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

Comment thread audit/divergences/data_table.yml Resolved
…n SelectInput's actions

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pierry01

pierry01 commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

shadcn audit

Repo ruby-ui/ruby_ui
PR #579
RubyUI base 1fd93db037814b05f96a0fb46f4fa2933079dea1
RubyUI head 32b1f9a88dcce10eec12a7ebe1bcb1db8e7e461c
shadcn 8a3536678a528cc012b9e62484290e79d0be410f
⚠️ Environment  font: "Inter Variable", "system-ui", -apple-system, "system-ui", "Segoe UI", Roboto, "Helvetica Neue", Arial, "sans-serif" vs "Inter Variable", sans-serif (light)
⚠️ Environment  font: "Inter Variable", "system-ui", -apple-system, "system-ui", "Segoe UI", Roboto, "Helvetica Neue", Arial, "sans-serif" vs "Inter Variable", sans-serif (dark)
✅ Theme  neutral tokens match shadcn
DataTable  (ref: bases/base/blocks/dashboard-01/components/data-table.tsx + style-vega.css @ 8a353667)
  ❌ Look       display: inline-flex
               vega: display: flex
               Pre-existing, not touched by this PR; follow-up PR. DataTableColumnToggle — closed/light, open/light, focus/light, after-esc/light, closed/dark, open/dark, focus/dark, after-esc/dark
               ↳ computed styles (shadcn-audit collect)
  ❌ Look       display: block; text-align: start
               vega: display: flex; text-align: left
               Pre-existing, not touched by this PR; follow-up PR. DataTableRowCheckbox — closed/light, open/light, focus/light, after-esc/light, closed/dark, open/dark, focus/dark, after-esc/dark
               ↳ computed styles (shadcn-audit collect)
  ⏸ Look       properties: font-family
               DataTableColumnToggle, DropdownMenuContent, DataTablePerPageSelect, DataTableRowCheckbox — explained by environment diff: font
  ✅ ARIA       DataTableColumnToggle, DropdownMenuContent, DataTablePerPageSelect, DataTableRowCheckbox: accessibility tree matches
               ↳ accessibility tree (shadcn-audit collect)
  ✅ Look       DataTablePerPageSelect: RubyUI::SelectTrigger size: :sm class: w-20 (h-8 gap-1.5 py-2 pr-2 pl-2.5 border-input dark:bg-input/30)
               No computed-style difference left on the per-page trigger, light and dark (only the font environment diff).
               ↳ computed styles (shadcn-audit collect)
  ✅ Behavior   click opens with focus on the selected option (10); typeahead 5 -> 50; Enter selects; Esc closes and returns focus to the trigger
               Driven with real clicks/keys on both sides (chrome-devtools).
               ↳ bases/base/ui/select.tsx
  ✅ Behavior   picking a size submits GET ?per_page=N (verified: Enter on 50 -> /__audit/data_table?per_page=50; docs demo frame goes 5 -> 10 rows); no inline handler in the DOM — intentional
               RubyUI's DataTable is server-driven, as for pagination: the page size is a query param the server reads, so the list comes back from the server and the URL can be shared. The Select itself (open, arrows, typeahead, Enter, Esc) behaves as shadcn's.
               ↳ audit/divergences/data_table.yml
  ✅ Props      SelectContent side: :top opens beside the trigger — intentional
               Overlaying the selected item needs custom positioning beyond Floating UI offset/flip/shift; the beside-the-trigger placement is what shadcn falls back to when there is no room.
               ↳ audit/divergences/select.yml
  ✅ Native     the only JS the component adds is the auto-submit on change, via a Stimulus controller on SelectInput
               No native feature submits a form on change; requestSubmit keeps validation and submit events. Works under a CSP without 'unsafe-inline'.
               ↳ https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement/requestSubmit
  ⏸ Behavior   DataTableColumnToggle menu set (arrows, typeahead, Esc focus return, outside click, animation) not exercised here: outside this PR; follow-up PR.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…, toolbar spacing) (ruby-ui#659)

* [Bug Fix] DataTable: toggle trigger as a toolbar flex item, dashboard-01 status badges in the audit fixture

DataTableColumnToggle's wrapper and its DropdownMenu root are display:
contents, so the Columns button is a flex item of the toolbar like shadcn's
trigger (its inline-flex blockifies to flex). The audit fixture's badges
follow the dashboard-01 block (default size, px-1.5 text-muted-foreground,
CircleCheck/Loader status icons), and the native checkbox's display and
text-align differences are registered as divergences.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* [Bug Fix] DataTableToolbar: 24px before the table, as dashboard-01

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@cirdes
cirdes merged commit 18d968b into ruby-ui:main Nov 6, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants