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

Ensure FocusTrap is only active when the given enabled value is true #2456

Merged
merged 7 commits into from Apr 26, 2023

Conversation

milhamm
Copy link
Contributor

@milhamm milhamm commented Apr 26, 2023

Currently, the tab focus is wrong when navigating on a Tab containing a Dialog component.

Previous Behavior

Screencast.from.04-26-2023.04.22.32.PM.webm

This PR Fix

Screencast.from.04-26-2023.04.23.58.PM.webm

Fixes #2406

@vercel
Copy link

vercel bot commented Apr 26, 2023

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

Name Status Preview Comments Updated (UTC)
headlessui-react ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 26, 2023 0:49am
headlessui-vue ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 26, 2023 0:49am

@vercel
Copy link

vercel bot commented Apr 26, 2023

@milhamm is attempting to deploy a commit to the Tailwind Labs Team on Vercel.

A member of the Team first needs to authorize it.

@milhamm milhamm changed the title fix(tabs): wrong tab focus when Tab contains a Dialog fix(tabs): wrong tab focus when navigate Tabs that contains a Dialog Apr 26, 2023
@milhamm milhamm marked this pull request as ready for review April 26, 2023 09:28
@milhamm milhamm changed the title fix(tabs): wrong tab focus when navigate Tabs that contains a Dialog fix(tabs): wrong tab focus when navigating Tabs that contains a Dialog Apr 26, 2023
@milhamm milhamm changed the title fix(tabs): wrong tab focus when navigating Tabs that contains a Dialog fix(tabs, focus-trap): wrong tab focus when navigating Tabs that contains a Dialog Apr 26, 2023
@RobinMalfait RobinMalfait self-assigned this Apr 26, 2023
This will allow us to make the code relatively similar between React and
Vue.
@RobinMalfait RobinMalfait changed the title fix(tabs, focus-trap): wrong tab focus when navigating Tabs that contains a Dialog Ensure FocusTrap is only active when the given enabled value is true Apr 26, 2023
@RobinMalfait RobinMalfait merged commit 5cfbb4b into tailwindlabs:main Apr 26, 2023
7 checks passed
@milhamm milhamm deleted the fix/tabs-focus-with-dialog branch April 26, 2023 12:51
@RobinMalfait
Copy link
Collaborator

Hey, thanks so much for this contribution! 🙏

While your fix worked as expected, there was a bigger problem going on where the FocusTrap was also restoring focus to the last known focused element even if the FocusTrap should have never been enabled in the first place.

I also ported the fix + test to the Vue version. The added test was very helpful, thanks!

@milhamm
Copy link
Contributor Author

milhamm commented Apr 26, 2023

Hey @RobinMalfait, thanks for sorting out the root cause of the problem.

Quick question: would it be okay if I also update the Portal component to utilize the new useOnUnmount hook?

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.

Focus outline out of sync when <Tab> contains a <Dialog>
2 participants