fix: notebook pagination styling by dmitriy-kostianetskiy · Pull Request #479 · sqlrooms/sqlrooms

dmitriy-kostianetskiy · 2026-03-18T10:17:26Z

Task: #438

Fix outline\overflow issue and hide arrows in numeric input

Refactor DataTablePaginated

Summary by CodeRabbit

Refactor
- Pagination and footer logic reorganized: footer moved to a dedicated component and paginated table exported as a named module (import shape updated).
New Features
- Added a reusable page-size selector and enhanced pagination controls (first/prev/next/last, page input).
Style
- Added a utility to hide number-input spinners for consistent numeric input styling.
Documentation
- Formatting and layout updates to CLAUDE.md.
Bug Fixes
- Improved JSON-schema autocomplete to more reliably resolve property names.

coderabbitai · 2026-03-18T10:17:44Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d6e75c56-24f9-42c6-a98f-85d1e6c66680

📥 Commits

Reviewing files that changed from the base of the PR and between 5567490 and 7085ae2.

📒 Files selected for processing (1)

packages/data-table/src/DataTablePaginatedFooter.tsx

🚧 Files skipped from review as they are similar to previous changes (1)

packages/data-table/src/DataTablePaginatedFooter.tsx

📝 Walkthrough

Walkthrough

Refactors DataTablePaginated into a named export, extracts footer and page-size UI into new components, updates import/export sites and typings, adds a .no-spinner CSS utility, tweaks CodeMirror JSON property name resolution, and applies formatting changes to CLAUDE.md.

Changes

Cohort / File(s)	Summary
Export Shape Updates `packages/data-table/src/DataTablePaginated.tsx`, `packages/data-table/src/DataTableArrowPaginated.tsx`, `packages/data-table/src/index.ts`	Converted `DataTablePaginated` from a default export to a named export and updated imports/exports accordingly.
Footer Component Extraction `packages/data-table/src/DataTablePaginatedFooter.tsx` (new)	Introduced `DataTablePaginatedFooter` to encapsulate footer UI/state: page navigation, page input, page size control, optional row count and footer actions.
DataTablePaginated Refactor `packages/data-table/src/DataTablePaginated.tsx`	Removed inline footer and internal page-index state; delegates footer rendering to `DataTablePaginatedFooter`; reduced UI-library imports.
Page Size Selection `packages/data-table/src/PageSizeSelect.tsx` (new)	Added `PageSizeSelect` component providing selectable page-size options and change callback.
Import Typings Update `packages/data-table/src/QueryDataTable.tsx`	Changed `DataTablePaginatedProps` import to a type-only import (no runtime change).
Styling Utilities `packages/ui/tailwind-preset.css`	Added `.no-spinner` utility to remove number-input spinners via WebKit rules.
Editor Autocomplete Fix `packages/codemirror/src/extensions/json-schema-autocomplete.ts`	Improved `getPropertyName` to climb the AST to the nearest `Property` before extracting `PropertyName`.
Docs Formatting `CLAUDE.md`	Whitespace and Markdown reflow changes; formatting-only edits to examples and code fences.

Sequence Diagram

sequenceDiagram
    participant Client
    participant DataTablePaginated
    participant DataTablePaginatedFooter
    participant PageSizeSelect
    participant TableInstance

    Client->>DataTablePaginated: render(pagination, isFetching, numRows, footerActions)
    DataTablePaginated->>TableInstance: create/pass table instance
    DataTablePaginated->>DataTablePaginatedFooter: provide table, pagination, isFetching, numRows, footerActions
    DataTablePaginatedFooter->>DataTablePaginatedFooter: sync internalPageIndex with pagination.pageIndex
    DataTablePaginatedFooter->>PageSizeSelect: render(current pageSize)

    Client->>DataTablePaginatedFooter: click Next / Prev or edit page input
    DataTablePaginatedFooter->>TableInstance: table.setPageIndex / table.nextPage / table.previousPage
    TableInstance->>DataTablePaginatedFooter: update pagination state
    Client->>PageSizeSelect: choose page size
    PageSizeSelect->>TableInstance: table.setPageSize(newSize)
    TableInstance->>DataTablePaginatedFooter: reflect new pageSize/pagination

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

feat: code mirror editors #418: Overlaps with edits to packages/codemirror/src/extensions/json-schema-autocomplete.ts (same file/logic area).
fix: Don't show empty footer in data table #412: Related to DataTablePaginated footer rendering and visibility logic (strong overlap).
feat: Data table row selection #358: Also touches DataTablePaginated refactor and public export surface (related export/usage changes).

Poem

🐰 I nudged the footer out to play,
Named exports hopped into the day,
Spinners vanished, sizes picked with glee,
JSON names climbed up the tree,
A tiny refactor—hip hooray! 🥕

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: notebook pagination styling' accurately summarizes the main focus of the PR—fixing pagination-related styling issues, which aligns with the core changes refactoring DataTablePaginated and adding pagination footer styling.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/notebook-pagination

📝 Coding Plan

Generate coding plan for human review comments

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.}

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/data-table/src/DataTablePaginatedFooter.tsx (1)
40-48: State synchronization pattern is correct but consider adding a comment.

The "state-in-render" pattern (comparing pagination?.pageIndex with prevPageIndex and calling setInternalPageIndex during render) is a valid React pattern for syncing derived state with props. This replaces the deprecated getDerivedStateFromProps. Consider adding a brief comment explaining this synchronization to help future maintainers.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/data-table/src/DataTablePaginatedFooter.tsx` around lines 40 - 48,
Add an inline comment above the render-time state-sync block explaining why the
code compares pagination?.pageIndex with prevPageIndex and calls
setInternalPageIndex/setPrevPageIndex during render: note that this is an
intentional "state-in-render" pattern to synchronize derived local state
(internalPageIndex) with incoming prop pagination?.pageIndex (replacing older
getDerivedStateFromProps) and that it avoids extra effects; reference the
variables internalPageIndex, setInternalPageIndex, prevPageIndex,
setPrevPageIndex and pagination?.pageIndex in the comment for clarity.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/data-table/src/DataTablePaginatedFooter.tsx`:
- Around line 140-144: In DataTablePaginatedFooter.tsx replace the global
isFinite check with Number.isFinite to match other uses and avoid coercion:
update the conditional that currently reads {numRows !== undefined &&
isFinite(numRows) ? ...} to use Number.isFinite(numRows) so only actual finite
numeric values pass (this affects the rendering block that shows
{`${formatCount(numRows)} rows`}).

---

Nitpick comments:
In `@packages/data-table/src/DataTablePaginatedFooter.tsx`:
- Around line 40-48: Add an inline comment above the render-time state-sync
block explaining why the code compares pagination?.pageIndex with prevPageIndex
and calls setInternalPageIndex/setPrevPageIndex during render: note that this is
an intentional "state-in-render" pattern to synchronize derived local state
(internalPageIndex) with incoming prop pagination?.pageIndex (replacing older
getDerivedStateFromProps) and that it avoids extra effects; reference the
variables internalPageIndex, setInternalPageIndex, prevPageIndex,
setPrevPageIndex and pagination?.pageIndex in the comment for clarity.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d34ce40f-ff2b-47e2-98fc-467250e883b5

📥 Commits

Reviewing files that changed from the base of the PR and between 5100260 and b6ebdc5.

📒 Files selected for processing (8)

CLAUDE.md
packages/data-table/src/DataTableArrowPaginated.tsx
packages/data-table/src/DataTablePaginated.tsx
packages/data-table/src/DataTablePaginatedFooter.tsx
packages/data-table/src/PageSizeSelect.tsx
packages/data-table/src/QueryDataTable.tsx
packages/data-table/src/index.ts
packages/ui/tailwind-preset.css

Copilot · 2026-03-18T11:22:42Z

@dmitriy-kostianetskiy I've opened a new pull request, #482, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: dmitriy-kostianetskiy <[email protected]>

Co-authored-by: Copilot <[email protected]> Co-authored-by: dmitriy-kostianetskiy <[email protected]>

dmitriy-kostianetskiy added 3 commits March 18, 2026 10:39

feat: refactor DataTable components and add pagination footer

3752aeb

fix: outline clipped

e0591e5

refactor: extract PageSizeSelect and hide arrow buttons in numeric input

b6ebdc5

dmitriy-kostianetskiy requested review from ilyabo and mihajlo-stojkovic March 18, 2026 10:17

dmitriy-kostianetskiy self-assigned this Mar 18, 2026

coderabbitai Bot reviewed Mar 18, 2026

View reviewed changes

Comment thread packages/data-table/src/DataTablePaginatedFooter.tsx Outdated

ilyabo approved these changes Mar 18, 2026

View reviewed changes

fix: failing test

5567490

Copilot AI mentioned this pull request Mar 18, 2026

fix(data-table): use Number.isFinite instead of global isFinite #482

Merged

[WIP] [WIP] Address feedback on notebook pagination styling fix (#482)

7085ae2

Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: dmitriy-kostianetskiy <[email protected]>

dmitriy-kostianetskiy merged commit ed9b08a into main Mar 18, 2026
4 checks passed

dmitriy-kostianetskiy deleted the fix/notebook-pagination branch March 18, 2026 11:30

dmitriy-kostianetskiy added a commit that referenced this pull request Mar 20, 2026

fix: notebook pagination styling (#479)

f0ce94d

Co-authored-by: Copilot <[email protected]> Co-authored-by: dmitriy-kostianetskiy <[email protected]>

Navigation Menu

Repository navigation

fix: notebook pagination styling - #479

fix: notebook pagination styling#479
dmitriy-kostianetskiy merged 5 commits into
mainfrom
fix/notebook-pagination

dmitriy-kostianetskiy commented Mar 18, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Mar 18, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Copilot AI commented Mar 18, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

dmitriy-kostianetskiy commented Mar 18, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI commented Mar 18, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dmitriy-kostianetskiy commented Mar 18, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 18, 2026 •

edited

Loading