-
Notifications
You must be signed in to change notification settings - Fork 268
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: trap focus in opened modal #2278
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #2278 +/- ##
=======================================
Coverage 84.82% 84.82%
=======================================
Files 344 344
Lines 7573 7574 +1
Branches 2106 2106
=======================================
+ Hits 6424 6425 +1
Misses 806 806
Partials 343 343 ☔ View full report in Codecov by Sentry. |
Size Change: +27.2 kB (+2%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
"@types/react-dom": "^18.0.3", | ||
"@types/react-image-gallery": "^1.0.5", | ||
"@types/react-is": "^17.0.3", | ||
"@types/react": "^18.2.55", |
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.
React Aria packages use React.JSX
in their typings, instead of the deprecated global JSX
. That's why we needed to bump @types/react
to the latest version in v18 branch: our previous version @types/react@^18.0.3
didn't have React.JSX
(see DefinitelyTyped/DefinitelyTyped#64464).
"@types/react-dom": "^18.2.19", | ||
"@types/react-image-gallery": "^1.2.4", | ||
"@types/react-is": "^18.2.4", |
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.
Bringing those up-to-date as well :)
Without running this explicitly - is the exit button tabbable as well (top right corner) - would it be also expected behavior cosidering elements are on the same one visible layer? |
@arnautov-anton Yes, you can tab into the close button! I think this is the correct behavior, as you said: both the close button and the rest of the modal belong to the same "layer". |
## [11.9.0](v11.8.0...v11.9.0) (2024-02-21) ### Bug Fixes * add aria-expanded attribute to emoji picker and reactions selector ([#2274](#2274)) ([b15cdd5](b15cdd5)) * mark channel read on scroll to bottom of the main message list ([#2283](#2283)) ([d04d0ab](d04d0ab)) * prevent layout shifts in reactions modal ([#2272](#2272)) ([706cf3d](706cf3d)) * trap focus in opened modal ([#2278](#2278)) ([8f48b52](8f48b52)) ### Features * make aria-labels localizable ([#2282](#2282)) ([7867677](7867677)), closes [#1931](#1931) [#1994](#1994)
🎉 This PR is included in version 11.9.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
🎯 Goal
When a modal dialog is displayed in chat, it's still possible to
Tab
your way through the focusable elements outside of the modal. The common practice is to "trap" the focus inside the modal, so thatTab
cycles through the focusable elements inside the modal only.🛠 Implementation details
Implementing focus trap properly is not as easy as it seems - so it's better to use a proven solution. This PR uses
FocusScope
from React Aria.🎨 UI Changes
Screen.Recording.2024-02-15.at.15.56.32.mov