Skip to content

Update Slider and DataTable pagination handling#411

Merged
kylengn merged 2 commits intomainfrom
fix/slider-and-table
Feb 12, 2026
Merged

Update Slider and DataTable pagination handling#411
kylengn merged 2 commits intomainfrom
fix/slider-and-table

Conversation

@kylengn
Copy link
Contributor

@kylengn kylengn commented Feb 12, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced Slider component stability with improved rendering consistency.
    • Refined DataTable pagination to intelligently manage display controls based on data volume and configuration.
  • Chores

    • Updated storybook examples and test data to reflect component refinements.

@kylengn kylengn self-assigned this Feb 12, 2026
Copilot AI review requested due to automatic review settings February 12, 2026 13:47
@vercel
Copy link

vercel bot commented Feb 12, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
react-ui Ready Ready Preview, Comment Feb 12, 2026 1:47pm

Request Review

@coderabbitai
Copy link

coderabbitai bot commented Feb 12, 2026

Walkthrough

This PR introduces conditional pagination logic to DataTable, ensuring pagination only renders when both enabled and dataset exceeds page size, updates Slider Thumb element key generation for stability, and adjusts test data accordingly.

Changes

Cohort / File(s) Summary
Changeset
.changeset/nice-states-share.md
Adds changeset documenting minor version bump for @shipfox/react-ui with update to Slider and DataTable pagination handling.
Slider Component
libs/react/ui/src/components/slider/slider.tsx
Changes Thumb element key generation from value-based to index-based (key="thumb-<index>"), improving key stability without affecting visual output.
DataTable Component
libs/react/ui/src/components/table/data-table.tsx
Introduces isPaginationNeeded flag (true when pagination enabled AND data.length > pageSize) to conditionally gate pagination rendering, state tracking, layout calculations, and empty row padding.
Table Stories & Test Data
libs/react/ui/src/components/table/table.stories.tsx, libs/react/ui/src/components/table/table.stories.data.ts
Updates sample data size from 51 to 11 jobs and WithPagination story pageSize from 10 to 5 to better demonstrate pagination behavior with smaller datasets.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • noe-charmet
  • EnzalRad
  • dvxam

Poem

🐰 A slider hops with better keys,
No thumb shall tremble in the breeze,
While tables spin their pages small,
Only when data fills the hall!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title accurately summarizes the main changes across the changeset, directly referencing the two primary components being modified: Slider key handling and DataTable pagination logic.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/slider-and-table

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
libs/react/ui/src/components/table/data-table.tsx (2)

187-187: Consider the boundary: data.length === pageSize disables pagination.

With strict >, if data has exactly pageSize items, pagination won't render. This is likely intentional (no second page needed), but worth confirming it matches the expected UX — users won't see page-size selector or page info in that scenario.

Also, if data length fluctuates (e.g., via filtering) around the pageSize boundary, the pagination UI will appear/disappear dynamically. Ensure this doesn't cause a jarring layout shift given the minTableHeight also toggles.


195-208: Consider using manualPagination flag instead of conditionally omitting getPaginationRowModel.

While toggling getPaginationRowModel between a function and undefined is type-safe and technically supported, TanStack Table's recommended pattern for conditional pagination is to keep getPaginationRowModel defined and use manualPagination: true to toggle behavior. This provides clearer intent and better maintainability:

const table = useReactTable({
  data,
  columns,
  getCoreRowModel: getCoreRowModel(),
  getPaginationRowModel: getPaginationRowModel(),
  manualPagination: !isPaginationNeeded, // Toggle this instead
  // ... rest of config
});
libs/react/ui/src/components/table/table.stories.data.ts (1)

101-101: Note: reduced dataset limits multi-page pagination testing in stories.

With 11 items and pageSize=10 (used by WithRowSelection and JobsOverview), there are only 2 pages with a single item on page 2. This is fine for validating the isPaginationNeeded logic but provides minimal coverage for navigating through multiple pages in Storybook.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@argos-ci
Copy link

argos-ci bot commented Feb 12, 2026

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) 👍 Changes approved 15 changed Feb 12, 2026, 1:48 PM

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates UI component behavior around pagination in the DataTable and stabilizes Slider thumb rendering, alongside Storybook story/data tweaks to better demonstrate the updated pagination behavior.

Changes:

  • Make DataTable pagination conditional so pagination is only enabled/rendered when the dataset exceeds the configured page size.
  • Update Slider thumb keys to avoid using the thumb value as the React key.
  • Adjust Table Storybook pagination defaults and sample dataset size.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
libs/react/ui/src/components/table/table.stories.tsx Updates the pagination story’s default pageSize to better reflect the intended pagination behavior.
libs/react/ui/src/components/table/table.stories.data.ts Reduces story sample data size to align with updated pagination handling and story expectations.
libs/react/ui/src/components/table/data-table.tsx Introduces conditional pagination enabling/rendering based on data length vs. page size.
libs/react/ui/src/components/slider/slider.tsx Uses stable thumb keys based on index instead of thumb value.
.changeset/nice-states-share.md Declares a minor release for the UI package reflecting the behavior changes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +187 to +195
const isPaginationNeeded = pagination && data.length > pageSize;

const table = useReactTable({
data,
columns: columnsWithSelection,
getCoreRowModel: getCoreRowModel(),
getFilteredRowModel: getFilteredRowModel(),
getSortedRowModel: getSortedRowModel(),
getPaginationRowModel: pagination ? getPaginationRowModel() : undefined,
getPaginationRowModel: isPaginationNeeded ? getPaginationRowModel() : undefined,
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

This change makes pagination conditional on data.length > pageSize, so pagination={true} no longer guarantees paginated behavior or a visible footer. Since pagination is a public prop, consider either (a) updating the component JSDoc/prop contract to reflect the new behavior, or (b) keeping pagination enabled when pagination is true and only conditionally hiding the navigation UI (e.g., when table.getPageCount() <= 1).

Copilot uses AI. Check for mistakes.
@kylengn kylengn merged commit 5617554 into main Feb 12, 2026
11 checks passed
@kylengn kylengn deleted the fix/slider-and-table branch February 12, 2026 15:20
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