Skip to content

[Bug Fix] Fix DataTable pagination with array params and adapter naming#359

Merged
djalmaaraujo merged 4 commits intomainfrom
fix/build-query-array-params
Apr 29, 2026
Merged

[Bug Fix] Fix DataTable pagination with array params and adapter naming#359
djalmaaraujo merged 4 commits intomainfrom
fix/build-query-array-params

Conversation

@djalmaaraujo
Copy link
Copy Markdown
Contributor

Summary

  • Fix build_query losing array param values: When query params contain arrays (e.g. combobox filters), Array#to_s was producing inspect output like ["uuid"] instead of proper query string encoding. Replaced with Array(v) to handle both scalar and array values correctly using only stdlib (CGI.escape).

  • Move pagination adapters into data_table/ folder: Renamed DataTablePaginationAdapters::{Pagy,Kaminari,Manual} to DataTable{Pagy,Kaminari,Manual}Adapter directly under RubyUI namespace. This fixes two issues:

    • Class name collision when host app includes RubyUI module (e.g. Pagy clashing with the pagy gem)
    • Generator (ruby_ui:component DataTable) only copies the data_table/ folder, so adapters in a separate data_table_pagination_adapters/ directory were missing from the generated output
  • Customizable prev/next labels: Added prev_label: and next_label: params (default "<" and ">") instead of hardcoded English strings

  • Hide pagination for single page: Skip rendering when total_pages <= 1

Test plan

  • All 157 existing tests pass
  • StandardRB lint passes
  • Verified generator copies adapters correctly inside data_table/
  • Verified array params preserved in Rails context (ids=uuid-1&ids=uuid-2)
  • Verified no adapter name collision in Rails app with include RubyUI

Array#to_s returns inspect output (e.g. '["uuid"]'), which Rails
re-parses as a literal string inside the array. Use Array(v) to
handle both scalar and array values correctly.
Rename DataTablePaginationAdapters::{Pagy,Kaminari,Manual} to
DataTable{Pagy,Kaminari,Manual}Adapter under RubyUI namespace.

Fixes class name collision when host app includes RubyUI module
(e.g. Pagy clashing with the pagy gem). Also fixes generator
only copying data_table/ folder, missing the separate adapters dir.
Add prev_label and next_label params (default "<" and ">").
Skip rendering when total pages <= 1.
@djalmaaraujo djalmaaraujo requested a review from cirdes as a code owner April 29, 2026 18:53
Comment on lines +4 to +6
require_relative "data_table_manual_adapter"
require_relative "data_table_pagy_adapter"
require_relative "data_table_kaminari_adapter"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@djalmaaraujo I believe it's not necessary anymore.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The gem doesn't use Zeitwerk, so require_relative is needed here. When the generator copies the files into a Rails app, the app's autoloader handles it — but in the gem context these are required.

if current <= 1
li do
span(class: "opacity-50 pointer-events-none px-3 h-9 inline-flex items-center text-sm") { plain "Previous" }
span(class: "opacity-50 pointer-events-none px-3 h-9 inline-flex items-center text-sm") { plain @prev_label }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One line plain is unnecessary.

span(class: "...") { @prev_label } # it works

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, fixed in 4940ef2.

Copy link
Copy Markdown
Contributor

@pierry01 pierry01 left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!!!

Phlex renders ivars directly in tag blocks without plain.
@djalmaaraujo djalmaaraujo merged commit 0dabaed into main Apr 29, 2026
2 checks passed
@djalmaaraujo djalmaaraujo deleted the fix/build-query-array-params branch April 29, 2026 19:46
djalmaaraujo pushed a commit that referenced this pull request Apr 30, 2026
Bumps [tailwindcss](https://github.com/tailwindlabs/tailwindcss/tree/HEAD/packages/tailwindcss) from 4.1.14 to 4.1.17.
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.1.17/packages/tailwindcss)

---
updated-dependencies:
- dependency-name: tailwindcss
  dependency-version: 4.1.17
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

3 participants