Skip to content

Autosize the row numbers column#316

Merged
severo merged 6 commits intomasterfrom
296-autosize-row-number-column
Nov 6, 2025
Merged

Autosize the row numbers column#316
severo merged 6 commits intomasterfrom
296-autosize-row-number-column

Conversation

@severo
Copy link
Copy Markdown
Contributor

@severo severo commented Nov 4, 2025

The code is simpler now: we just compute the number of characters of the number of rows, and the first column width is computed in CSS.

It works for different fonts (monospace or not). We use the ch unit which is more adapted to this use case than em.

Screencast.From.2025-11-04.16-20-47.mp4

@severo severo marked this pull request as draft November 4, 2025 10:05
@severo severo linked an issue Nov 4, 2025 that may be closed by this pull request
@severo severo requested review from bleakley and platypii November 4, 2025 15:29
@severo severo marked this pull request as ready for review November 4, 2025 15:30
@bleakley
Copy link
Copy Markdown
Contributor

bleakley commented Nov 4, 2025

This isn't working for my use case because the row indices are greater than the number of current rows in the dataframe when the filter is applied. For example here I have a filter applied, that the title must contain "be":

image

At first there are 64 rows in the dataframe, and already you can see there are some 3 digit row numbers, then when I scroll it gets worse:

image

I need to be able to pass in another number, which is the total number of rows in the dataset, which may be greater than the number in the dataframe. If this isn't a good idea I could instead calculate width and override the styles in hyperparam.app

@severo
Copy link
Copy Markdown
Contributor Author

severo commented Nov 5, 2025

I need to be able to pass in another number, which is the total number of rows in the dataset, which may be greater than the number in the datafra

Oh, yes, it makes sense: we want to compute the width on the maximum row number, not necessarily on the number of rows in the dataframe. I'll add this possibility, and fallback to the dataframe number of rows by default.

@severo
Copy link
Copy Markdown
Contributor Author

severo commented Nov 5, 2025

Done. Does it work better for you?

Screenshot From 2025-11-05 10-16-40

@severo severo requested a review from Copilot November 5, 2025 09:17
Copy link
Copy Markdown
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 adds support for displaying custom maximum row numbers in the HighTable component, particularly useful for filtered datasets where the visible row count differs from the actual data index range. The implementation shifts from JavaScript-calculated row header widths to CSS-based dynamic width calculations.

Key changes:

  • Added maxRowNumber prop to HighTable and DataProvider to support custom row number display
  • Refactored row header width calculation from JavaScript to CSS using character-based sizing
  • Changed canMeasureWidth from boolean to per-column record in TableHeader for more granular control

Reviewed Changes

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

Show a summary per file
File Description
src/hooks/useData.tsx Added maxRowNumber to DataContext with memoized fallback to data.numRows
src/components/TableHeader/TableHeader.tsx Changed canMeasureWidth prop from boolean to canMeasureColumn record for per-column measurement control
src/components/TableHeader/TableHeader.test.tsx Removed canMeasureWidth prop from test cases (now optional)
src/components/HighTable/HighTable.tsx Integrated maxRowNumber prop, replaced JavaScript width calculation with CSS variables, refactored canMeasureWidth logic to per-column basis, removed unused cellStyle import, reordered prop documentation alphabetically
src/components/HighTable/HighTable.stories.tsx Added FilteredData story demonstrating the maxRowNumber feature with custom row numbering
src/components/ColumnHeader/ColumnHeader.tsx Added clarifying comment about using offsetWidth instead of clientWidth
src/HighTable.module.css Replaced fixed --row-label-width with dynamic --row-number-width calculated from character count, updated comment clarity
README.md Updated documentation with new maxRowNumber prop and reordered props alphabetically for consistency
.storybook/main.ts Simplified Storybook configuration to use string format for framework
.storybook/global.css Fixed typo in comment: "HightTable" to "HighTable"

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

Comment thread .storybook/main.ts
Copy link
Copy Markdown
Contributor

@bleakley bleakley left a comment

Choose a reason for hiding this comment

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

Yes, this is perfect, thank you 👍

@severo severo merged commit 8ab977a into master Nov 6, 2025
11 checks passed
@severo severo deleted the 296-autosize-row-number-column branch November 6, 2025 09:15
@bleakley bleakley mentioned this pull request Nov 18, 2025
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.

Autosize the row numbers column, without hardcoded value

3 participants