Skip to content

Conversation

@severo
Copy link
Contributor

@severo severo commented Dec 22, 2025

Add an explicit numRowsPerPage option instead of reusing padding

Remove dependency on padding and overscan in Slice.

Remove dependence on padding in Scroller.

Copy link
Contributor

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 refactors the virtualization logic by introducing an explicit numRowsPerPage option for keyboard navigation, decoupling it from the padding prop. The refactoring moves padding calculations into the RowsAndColumnsProvider, creating a cleaner separation of concerns where Scroller handles viewport management, the provider computes padded ranges, and Slice handles rendering.

Key Changes

  • Added numRowsPerPage prop specifically for keyboard navigation (PageUp/PageDown), replacing the overloaded use of padding
  • Refactored RowsAndColumnsContext to provide rowsRangeWithPadding instead of raw rowsRange, with padding calculations centralized in the provider
  • Updated Scroller and Slice components to consume the new rowsRangeWithPadding interface

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/components/HighTable/constants.ts Added defaultNumRowsPerPage constant for keyboard navigation
src/contexts/RowsAndColumnsContext.ts Introduced RowsRangeWithPadding interface and updated context to provide padded range
src/providers/RowsAndColumnsProvider.tsx Added padding calculation logic and exposed rowsRangeWithPadding instead of raw rowsRange
src/components/HighTable/Wrapper.tsx Updated prop threading to pass padding to provider and removed it from Scroller
src/components/HighTable/Slice.tsx Removed padding prop dependency, now uses numRowsPerPage for keyboard navigation and rowsRangeWithPadding from context
src/components/HighTable/Scroller.tsx Removed padding prop dependency, now uses rowsRangeWithPadding from context
src/components/HighTable/HighTable.keyboard.test.tsx Updated test to use numRowsPerPage instead of padding
README.md Added documentation for the new numRowsPerPage option

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

@severo severo merged commit 1e27495 into master Dec 22, 2025
5 checks passed
@severo severo deleted the no-dependency-on-padding-in-slice branch December 22, 2025 14:29
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