-
Notifications
You must be signed in to change notification settings - Fork 468
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(@clayui/focus-trap): Add FocusTrap component #5427
Conversation
95ed7e8
to
90b18ff
Compare
8c82f82
to
6aae847
Compare
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.
@veroglez looks good now, I just added a comment of a small detail but looks good to go ahead now. Thanks for working on it.
Hi @matuzalemsteles! I have added two commits with the tests and documentation (3c8a1df, af2e8f6) , could you take a look? Thanks in advance! |
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.
LGTM! I send a commit to add the functionality to suppress elements outside the scope of the focus trap, so it will work better when using the mouse for example by clicking elsewhere on the page, the focus will go back into scope again.
It fixes: #5409
Hi @matuzalemsteles! Here is the draft for the new FocusTrap component. I have proposed the following: the component has an active props, which is in charge of activating the focus trap. In this way, it's the responsibility of the user to activate the focus trap. It could be always activated or, for example, activate it with the click of a button.
I have also added stories for Storybook to verify its behavior (video attached). What do you think?
focusTrap.mov
TO DO: