-
Notifications
You must be signed in to change notification settings - Fork 75
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(modal, popover): Add disableFocusTrap
property to toggle focus trapping.
#5965
Conversation
disableFocusTrap
property should toggle focus trap.
…cite-components into dris0000/focus-trap-refactoring
disableFocusTrap
property should toggle focus trap.disableFocusTrap
property to toggle focus trapping.
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. Would it be possible to make an e2e test for this that wouldn't crash everything if it fails? It can be in another PR if getting this in for MV is urgent so they can test on next
That was a different PR, but I think |
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! Not sure how it might affect accessibility to disable focus trapping, but as far as implementation goes for enabling a feature like this, it is spot on!
Ah, I thought this PR was needed to disable the focus trap in Input to fix the crashing issue. Could modal/popover have a test that opens the component, tabs a full circle with focus trap on, turns focus trap off, and then tabs until it gets to an element outside of the modal/popover? |
* master: (55 commits) build: update browserslist db (#5986) docs: update component READMEs (#5980) 1.0.0-next.670 refactor(input-date-picker,date-picker)!: Removing deprecated locale properties (#5977) 1.0.0-next.669 refactor(input-date-picker)!: Remove deprecated active property (#5974) 1.0.0-next.668 fix(modal, popover): Add `disableFocusTrap` property to toggle focus trapping. (#5965) 1.0.0-next.667 refactor(input-time-picker)!: Remove deprecated active property (#5970) docs(changelog): fix breaking change indent levels (#5973) 1.0.0-next.666 refactor(time-picker)!: Remove deprecated locale property (#5962) 1.0.0-next.665 fix(input, input-number, input-text): Fix infinite loop crashing browser. #5882 (#5961) test(floating-ui): fix type errors (#5966) docs: update component READMEs (#5964) 1.0.0-next.664 fix(alert): auto-dismissible retains close button and dismisses timer while a user is hovering over (#5872) chore(color-picker): add opacity string (#5959) ...
fix(modal, popover): Add
disableFocusTrap
property to toggle focus trapping.