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

Adds Support for Dynamic Sizing to ComboBox Virtualization #2061

Merged
merged 64 commits into from
Jul 12, 2024

Conversation

Ben-Pusey-Bentley
Copy link
Contributor

@Ben-Pusey-Bentley Ben-Pusey-Bentley commented May 22, 2024

Changes

Adds support for dynamic sizing to ComboBox virtualization. This is achieved through adding the tanstack virtual package, and adding the measureElement ref to a wrapper around the virtualized items. This fixes an issue where the list box would not be sized correctly for the content if virtualization was enabled and some options had a subLabel value.
Before:
image
After:
image

The spacing between options on the virtualized ComboBox is also now slightly different, as a result of using the virtualizer's gap. The react workshop screenshot has been updated accordingly.

  • After PR TODO: Add tanstack virtual to Tree and Table components in order to remove older virtualization code.

Testing

Tested with this vite playground code. Also ran all automated tests. Replaced the failing unit test for keyboard navigation on virtualized ComboBox with an e2e test. Added an e2e test for the checking the dynamic sizing as well. Also updated react workshop test for virtualized ComboBox with it's new spacing.

Docs

Added react changesets for the dynamic sizing support and the added dependency on @tanstack/virtual-react.

@Ben-Pusey-Bentley Ben-Pusey-Bentley self-assigned this May 22, 2024
@Ben-Pusey-Bentley Ben-Pusey-Bentley linked an issue May 23, 2024 that may be closed by this pull request
@Ben-Pusey-Bentley Ben-Pusey-Bentley marked this pull request as ready for review May 23, 2024 15:21
@Ben-Pusey-Bentley Ben-Pusey-Bentley requested review from a team as code owners May 23, 2024 15:21
@Ben-Pusey-Bentley Ben-Pusey-Bentley requested review from mayank99 and removed request for a team May 23, 2024 15:21
packages/itwinui-react/src/core/ComboBox/ComboBoxMenu.tsx Outdated Show resolved Hide resolved
packages/itwinui-react/src/core/ComboBox/ComboBoxMenu.tsx Outdated Show resolved Hide resolved
packages/itwinui-react/src/core/ComboBox/ComboBoxMenu.tsx Outdated Show resolved Hide resolved
pnpm-lock.yaml Outdated Show resolved Hide resolved
.changeset/wet-trainers-heal.md Show resolved Hide resolved
packages/itwinui-react/src/core/ComboBox/ComboBoxMenu.tsx Outdated Show resolved Hide resolved
Co-authored-by: R <45748283+r100-stack@users.noreply.github.com>
Copy link
Member

@r100-stack r100-stack left a comment

Choose a reason for hiding this comment

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

No additional comments, LGTM 🚀

Copy link
Contributor

@mayank99 mayank99 left a comment

Choose a reason for hiding this comment

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

LGTM! I re-confirmed that #1107 is fixed

@Ben-Pusey-Bentley Ben-Pusey-Bentley merged commit 161c4fc into main Jul 12, 2024
16 checks passed
@Ben-Pusey-Bentley Ben-Pusey-Bentley deleted the BenPusey/AddTanstackVirtualForVirtualScroll branch July 12, 2024 15:48
@imodeljs-admin imodeljs-admin mentioned this pull request Jul 12, 2024
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.

Virtualized ComboBox miscalculates height of options
3 participants