Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add token representing the height of the table with all rows visible #2307

Merged
merged 32 commits into from
Aug 23, 2024

Conversation

mollykreis
Copy link
Contributor

@mollykreis mollykreis commented Jul 29, 2024

Pull Request

🤨 Rationale

Resolves #1624

👩‍💻 Implementation

The table already knows the combined height of all the rows in the table because it uses this value to size the table-scroll container. Therefore, I was able to create a token that gets assigned a value within the scope of the table styling that has a value of the size of the height the table needs to show all rows. The token's name is tableFitRowsHeight.

The table also now sets a default max-height on itself to keep a client from accidentally using tableFitRowsHeight and trying to render thousands of rows at once.

To use the new token, a client would create their table exactly as before and add the following style:

nimble-table {
    height: $ni-nimble-table-fit-rows-height;
}

🧪 Testing

  • Manually tested in storybook
  • Wrote chromatic tests that verify the height of the table is correct when its height is set to the new token value when:
    • There are 5 records, with & without grouping
    • There are 10 records, with & without grouping
    • There are 50 records, with & without grouping

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

@mollykreis
Copy link
Contributor Author

@atmgrifter00, @rajsite, can you take a look at this draft PR and let me know if you agree with the direction? If so, I'll add some tests for it and update the storybook docs referencing the issue being resolved.

@mollykreis mollykreis changed the title Add option to the nimble-table to have its height automatically determined by the number of rows Add token representing the height of the table with all rows visible Aug 19, 2024
@mollykreis mollykreis marked this pull request as ready for review August 19, 2024 16:32
@mollykreis mollykreis requested a review from jattasNI as a code owner August 19, 2024 16:32
@mollykreis mollykreis requested a review from rajsite August 19, 2024 16:32
@mollykreis mollykreis requested a review from fredvisser as a code owner August 20, 2024 18:37
@rajsite rajsite merged commit dc9d005 into main Aug 23, 2024
11 checks passed
@rajsite rajsite deleted the table-height-styling branch August 23, 2024 17:13
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.

nimble-table can't be styled to show all rows without a scrollbar
4 participants