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

bugfix: prevent infinite recursion #6

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

joleeee
Copy link

@joleeee joleeee commented Dec 12, 2024

I don't have time to make a minimal reproduction but I was having an issue when using shadcn-svelte and this together.

If I had a shadcn-svelte popup open, and clicked on a button outside which traps focus into something else (using trap-focus-svelte), then the expected behavior is the shadcn-svelte popup closes and releases its own focus lock. Instead I assume shadcn-svelte kept its lock for a little while.

Anyway this fixes the website crashing for me. Maybe it's useful for someone else too

image

Copy link

vercel bot commented Dec 12, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
trap-focus-svelte ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 12, 2024 5:38am

@henrygd
Copy link
Owner

henrygd commented Dec 12, 2024

Thanks, sounds like shadcn and this library are continuously pulling focus between each other until the shadcn lock ends.

Your solution looks good. I'd like to reproduce it and play around with it myself but won't have time for at least another few days. Please feel free to ping me if I forget.

@joleeee
Copy link
Author

joleeee commented Jan 6, 2025

@henrygd

@henrygd
Copy link
Owner

henrygd commented Jan 7, 2025

My apologies, went to visit family over the holidays and forgot about this. 🤦

I tested it out a bit. The current release is definitely not compatible with shadcn-svelte or other libraries that also try to trap focus.

Your change is much better, but I'm still seeing some unwanted behavior. For example, tabbing through items in a sheet takes multiple tab presses to advance to the next item.

Maybe the best solution is to allow users to define a custom CSS selector. When focus exits our trap, we check if the outside element (or a parent) matches that selector. If so, we leave it alone. I tried with [role=dialog] and it seems to work great with shadcn-svelte.

Or we can use something predefined like data-notrap and people can just add that to their element whenever they run into something like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants