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

✨ <Dialog/> Set return focus on clicked element #108

Merged

Conversation

far-fetched
Copy link
Contributor

@far-fetched far-fetched commented Nov 1, 2021

What changed and why

PR adds setReturnFocus option from focus-trap where we can explicitly set which element should be focus after deactivation. Default behavior is to return focus to the element which opened the dialog (trigger button).
When there is no overlay, and the user clicks outside the dialog, focus will be set to the element which is clicked by the user (see test).
By the way of implementing it, I figure out that focus-trap also uses similar pattern to detect outside-click, so far we used dedicated modifier (outside-click-modifier) as we use it only here I think we can use focus-trap and safely remove outside-click-modifier from dependency list.

@far-fetched far-fetched force-pushed the set-return-focus-on-clicked-element branch from 1dc03c5 to 1b8ddd6 Compare November 1, 2021 18:47
@far-fetched far-fetched force-pushed the set-return-focus-on-clicked-element branch from 1b8ddd6 to 12f048f Compare November 1, 2021 18:53
@GavinJoyce
Copy link
Owner

Thanks for this. Does this mirror the behaviour in React and Vue?

@far-fetched
Copy link
Contributor Author

As react and vue use their own implementation of focus-trap link with hooks - implementation is different, but final behavior stays the same - test in react is almost the same (semantic difference).

@GavinJoyce GavinJoyce merged commit f8f6de5 into GavinJoyce:master Nov 9, 2021
@GavinJoyce
Copy link
Owner

Great, thanks!

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