-
-
Notifications
You must be signed in to change notification settings - Fork 206
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
Feat: add sibling portal support & implement interact outside the right way #1080
Feat: add sibling portal support & implement interact outside the right way #1080
Conversation
🦋 Changeset detectedLatest commit: 01c0f8b 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 |
Oh no not the timezone tests failing due to DST again! I'll get that fixed later today on the develop branch so you can merge it in! |
That failing test may just need an |
@huntabyte not sure why the tests are not passing. I tried to manually test the functionality that's not passing the tests and at first glance, things seem to be working correctly. Will look into it tomorrow |
cf1ea93
to
8005ebf
Compare
@huntabyte it seems like merging the following PR will fix the tests for Date Picker #1055 |
…anatolzak/melt-ui into feat/add-sibling-portal-support
…anatolzak/melt-ui into feat/add-sibling-portal-support
Hi everyone, the work here is amazing 🚀 Just wanted to let you all know that in 21RISK we are also facing this issue, so it will be so good when it's fixed. If there is anything we can help with let us know. |
Hey @alexbjorlig, thanks for your patience! There are a lot of small (but mighty) internal changes going on simultaneously, so we're doing our best to ensure we don't introduce a regression into the library! |
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.
Sorry for the delay! Amazing PR as always @anatolzak 🎉
closes #1077, #1153, #1167
This PR adds support for sibling portals by correctly implementing outside interactions using a stack. Previously outside interactions did not work correctly when using sibling portals.
Also, please don't be intimidated by the "size" of this PR. It's mostly tests and other than that, this PR is 70 additions and 161 deletions.
Throughout this PR, I will mention the term "floating element", what I mean by that is anything that can be mounted into a portal (i.e. dialog, tooltip, popover, link preview, menu, tooltip, listbox etc.).
The correct way to implement outside interactions is by only closing the top-most floating element given the top-most element has
closeOnOutsideClick: true
. If the top-most element hascloseOnOutsideClick: false
, no elements should close since the top-most element "asked" to not close on an outside interaction.I also fixed an issue with the Tags Input when it's nested inside another floating element like a Popover. When editing a tag, if you click outside, we should just cancel editing. However, the popover would also close. This is solved by using the new
useInteractOutside()
on theedit
builder to prevent other floating elements from reacting to outside interactions.Here is a video showing nested floating elements and outside interactions only closing the top-most floating element. (ignore the craziness of the nested elements :))
Screen.Recording.2024-03-15.at.4.55.22.PM.mp4
Also, to prevent issue #1167, where interact outside doesn't fire when clicking outside opens a new floating element, which is caused by checking too late whether the element is at the top of the stack. This is because the outside interaction triggers the opening of another floating element, so by the time pointerup fires for the first floating element, the second floating element had already been added to the stack. This is prevented in this PR by checking in pointerup whether the element was at the top of the stack during pointerdown. I added a regression test for this as well.
Before
Screen.Recording.2024-04-08.at.1.28.18.PM.mov
After
Screen.Recording.2024-04-08.at.1.48.51.PM.mov