fix(menu): Fixed maxHeight calculation for menu portal. #5716
Conversation
If the viewSpaceBelow is less than 300px (default max height): The first time the menu renders, the viewSpaceBelow will probably not be greater than the menuHeight. So it will take path 2. or 3. for bottom/auto. If the viewable area is greater than the minimum height then it will take path 3. On the next render, the menuHeight is now less than viewSpaceBelow because it was constrained by the third path. So the first path is taken, but preferredMaxHeight will be greater than viewSpaceBelow because path 3 was taken on the previous render. The menu renders but overflows outside the view. On the following render, path 1 is not taken because the height is greater than the viewSpaceBelow again (because its overflowing). So path 3 is taken again. This resolves the potential for flickering by making sure that the size that _would_ be rendered will actually fit. But comparing the `viewSpaceBelow` and `scrollSpaceBelow` to the `maxHeight` that would actually be returned, not to the height of the menu currently.
🦋 Changeset detectedLatest commit: 1b3571b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 1b3571b:
|
BugEach hover causes a re-render, causing the height to be recalculated. react-select-bug-2023-07-30_17.22.25.mp4PatchedThe menu will not be larger than the space available. react-select-fixed-2023-07-30_17.23.48.mp4 |
|
@Methuselah96 I see you've been pushing some changes lately, if you have time are you able to take a look at this patch? Are there any changes I could make to get this in as well? Thank you very much! |
If the viewSpaceBelow is less than 300px (default max height):
The first time the menu renders, the viewSpaceBelow will probably not be greater than the menuHeight. So it will take path 2. or 3. for bottom/auto. If the viewable area is greater than the minimum height then it will take path 3.
On the next render, the menuHeight is now less than viewSpaceBelow because it was constrained by the third path. So the first path is taken, but preferredMaxHeight will be greater than viewSpaceBelow because path 3 was taken on the previous render. The menu renders but overflows outside the view.
On the following render, path 1 is not taken because the height is greater than the viewSpaceBelow again (because its overflowing). So path 3 is taken again.
This resolves the potential for flickering by making sure that the size that would be rendered will actually fit. But comparing the
viewSpaceBelowandscrollSpaceBelowto themaxHeightthat would actually be returned, not to the height of the menu currently.TLDR; When the space the menu would be rendered in is less than the maxHeight, but greater than the minHeight; the menu will flicker because it alternates the maxHeight value because of how it checks space available. This changes it so that the check is against what the new height would be, instead of against what it is now.
If needed I can provide a video of the behavior; its absolutely possible I'm doing something wrong here. But the fact that
menuHeightcould be less thanpreferredMaxHeightseems like this is still a bug.