-
Notifications
You must be signed in to change notification settings - Fork 38
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
Adjust min-height for VirtualizedScroll #1143
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like VirtualScroll
is a utility that can be and is used for more than just ComboBox; did you take that into consideration?
Also, I'm a bit uncertain on what problem you're trying to solve with this. Can you send me the code you used to test it? Nevermind I see the linked issue now.
@elephantcatdog You're right, it is probably better to calculate the height of each node individually instead, which I just updated. I also included sandbox in the description for the testing code that can be copied into vite. Also my bad I forgot to add the "closes x" or linking the problem 😅 |
|
@LostABike You've got some visual storybook tests failing for combobox and table. |
Ok, I pushed a VERY OLD code here |
Changes
ComboBox min-height calculation (line 420
VirtualScroll.tsx
) is no longer accurate, because it assumes all middle child height are the same which is not always true as there can be child with sublabel and child without. I made a new function to calculate each individual heights and use that to set min-height instead.But side note, because it is a "min-height", I feel like it might be better to calculate shortest node multiply by the amount of node instead? Since min-height is supposed to be the floor.
Closes #1107
Testing
Code I use to test in vite: https://codesandbox.io/s/dawn-sea-tjzr2j?file=/src/App.tsx
Docs