react-modal
react-modal copied to clipboard
reactjs
[fixed] element with display 'contents' is visible and is tabbable.
Fixes #905.
Changes proposed:
- element with display 'contents' is visible and is tabbable.
Upgrade Path (for changed or removed APIs):
- None
Acceptance Checklist:
- [ ] The commit message follows the guidelines in
CONTRIBUTING.md. - [ ] Documentation (README.md) and examples have been updated as needed.
- [ ] If this is a code change, a spec testing the functionality has been added.
- [ ] If the commit message has [changed] or [removed], there is an upgrade path above.
@diasbruno how can we help to get this PR out. We use the react-modal as a base for our Modal component this fix is what we need to unblock us for fixing the tab issue.
@galexandrade That's great! I believe the only thing missing here is to add a tests. You can take this PR and improve it.
@diasbruno @aloker I checked out the branch and tried to add some tests to cover the scenario but since the problem only happens on Safari, it is hard to simulate the same scenario on ReactDOM.
This is the problem I would like to have fixed here:
If there is an element that has the display: contents style applied, on Safari, it won't tab through the elements. The reason I am using display: contents is that I want my form tag to wrap the modal content and footer so the modal buttons are part of the form. However, it is messing with the modal layout so, I use the display: contents so the browser will "ignore" that form tag when painting the screen.
https://user-images.githubusercontent.com/24475665/195448097-b2de8a8c-e7a5-41f1-a1d5-c999966020fa.mov
You can check this example in Safari and Chrome.
That works fine, except for Safari where it won't tab through the elements. With this PR, the issue is fixed.
I'd like to offer help to move this PR forward. Please, how can I help to get this PR merged? 🙏
@galexandrade There are a few comments on the PR. You can take this patch and improve on it.