-
Notifications
You must be signed in to change notification settings - Fork 116
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
fix: combobox inside of popover bug #4049
Conversation
Run & review this pull request in StackBlitz Codeflow. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: 09d9390 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 09d9390. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
Size Change: +72 B (0%) Total Size: 1.09 MB
ℹ️ View Unchanged
|
Paste Run #8520
Run Properties:
|
Project |
Paste
|
Branch Review |
fix/combobox-in-popover
|
Run status |
Passed #8520
|
Run duration | 01m 59s |
Commit |
09d9390f44: feat(combobox): add usePortal prop
|
Committer | “nora |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
67
|
View all changes introduced in this branch ↗︎ |
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 09d9390:
|
2683de1
to
09d9390
Compare
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.
This is such a clean and elegant solution. It works so well, I also checked the storybooks to see if the original responsive would be reintroduced and it's all working perfectly. Really great job on this. Filter patterns is saved!!
Paste Run #8522
Run Properties:
|
Project |
Paste
|
Branch Review |
fix/combobox-in-popover
|
Run status |
Passed #8522
|
Run duration | 06m 01s |
Commit |
bf2a337ba4 ℹ️: Merge 09d9390f4471ecda858cecb2068304375d0debc7 into e95c7121a52d111114fc5acb95f4...
|
Committer | Nora Krantz |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
125
|
View all changes introduced in this branch ↗︎ |
Fixes a bug where Comboboxes (Singleselect and Multiselect) can't be used when placed inside of Popovers. The bug was caused by the Combobox Listbox Portal interfering with the Popover Reakit Portal, and was preventing hover styles from being applied as well as closing the entire Popover on a selection of an item (Multiselect only). See this CodeSandbox for a reproduction and more context.
This PR adds the
usePortal
prop to Combobox (which defaults totrue
). It should be used (and set tofalse
) when Combobox is used in a Popover. When Combobox doesn't use a Portal, it uses an absolutely positioned Box to position the listbox and attach it to the input box. This means that the ListboxPositioner isn't used on these Comboboxes, so the listbox won't position itself above the input box when the input is at the bottom of the window. If this bug comes up it will be addressed in the future.Examples added to Singleselect and Multiselect Combobox docs.