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

fix: clean dead reference to floating content #1059

Merged
merged 4 commits into from
Mar 6, 2024

Conversation

anatolzak
Copy link
Contributor

closes #1058, #1039

Screen.Recording.2024-03-03.at.11.58.29.AM.mov

This has been an ongoing issue with multiple components--the content jumps to the top left of the page when you unmount the component externally while the content is open when using an out transition on the content and when using an else if block to render the content.

The previous workaround that solved this issue for all components but the tooltip was to introduce sleep(0) before setting the $open state to false. So when the component get unmounted externally while the content is open, svelte can unmount the whole component instead of, for some reason, starting the transition out of the content, at which stage the content loses its anchor and causes the content to jump to the top left of the page.

However, for some reason, adding sleep(0) to the tooltip as a workaround does not work. It requires adding a pretty significant delay (~80ms) to prevent this issue. However adding a significant delay is not preferable.

I dug deeper into this issue and found out that the reason the content jumps to the top left of the page is because in useFloating, the compute function reruns to set the new position of the content as it is transitioning out.

I found out that the client rect of the reference (the anchor) is { top: 0, left: 0 } during the unmounting of the content. The reason for that is that we are holding a reference to the trigger which was already unmounted, but in JavaScript, you can still hold a reference to an html element that is not in the document anymore. An html element that is not the document has client rect of 0s.

So our code that recomputes the position of the content is rerun with a trigger that is "at the top left of the page", but in reality it's no longer in the document.

So we should check if the reference is in the document before computing a new position for the content.

And so this finally solves this issue without having to add delay(0) to so many builders in addition to the delay not working in the case of the tooltip.

Finally :)

Copy link

changeset-bot bot commented Mar 5, 2024

🦋 Changeset detected

Latest commit: 6a956ba

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@melt-ui/svelte Patch

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

@anatolzak
Copy link
Contributor Author

For some reason a test in Combobox wasn't passing after removing sleep(0) from closeMenu in listbox/create.ts, even though I just recently added the delay in a PR. I fixed the tests in question

@huntabyte
Copy link
Member

This makes complete sense!

Copy link
Contributor

github-actions bot commented Mar 6, 2024

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
melt-ui ✅ Ready (View Log) Visit Preview 6a956ba

@huntabyte huntabyte changed the title [Tooltip, Link Preview, Menu, Popover, Listbox] fix content jump on unmount by handling stale anchor reference fix: clean dead reference to floating content Mar 6, 2024
@huntabyte huntabyte merged commit 30f6412 into melt-ui:develop Mar 6, 2024
5 checks passed
@github-actions github-actions bot mentioned this pull request Mar 6, 2024
@anatolzak anatolzak deleted the fix/content-jump-on-unmount branch March 6, 2024 06:48
lolcabanon pushed a commit to lolcabanon/melt-ui that referenced this pull request Apr 20, 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
2 participants