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.

Sign up for GitHub

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

Jump to bottom

Table: Move row height calculation from virtualizer, monitor for token changes #2388

Merged
merged 5 commits into from
Sep 10, 2024
Merged

Table: Move row height calculation from virtualizer, monitor for token changes #2388

merged 5 commits into from
Sep 10, 2024

Conversation

msmithNI
Copy link
Contributor

@msmithNI msmithNI commented Sep 6, 2024 •
edited
Loading

Pull Request

🤨 Rationale

#2212

👩‍💻 Implementation

  • Moved row height calculation from virtualizer to table class (new internal property there for row height)
  • Added subscribers to listen for changes to the controlHeight and borderWidth design tokens, which are used for row height calculation

🧪 Testing

  • No table visual diffs with this change
  • Added autotests to ensure that rowHeight/pageSize are recomputed when the controlHeight and borderWidth tokens change
  • Manual testing of PageUp/PageDown/Home/End key behavior after tokens change

✅ Checklist

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

@msmithNI
Copy link
Contributor Author

msmithNI commented Sep 6, 2024

@mollykreis can you buddy this PR?

@msmithNI msmithNI marked this pull request as ready for review September 9, 2024 18:20
@msmithNI msmithNI merged commit 4cfd1a6 into main Sep 10, 2024
13 checks passed
@msmithNI msmithNI deleted the users/masmith/moveRowHeightCalcFromVirtualizer branch September 10, 2024 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rajsite rajsite rajsite approved these changes

@jattasNI jattasNI jattasNI approved these changes

@mollykreis mollykreis mollykreis approved these changes

@fredvisser fredvisser Awaiting requested review from fredvisser fredvisser is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@msmithNI @rajsite @jattasNI @mollykreis