-
-
Notifications
You must be signed in to change notification settings - Fork 5k
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: added close drawer accessibility tap area #11184
fix: added close drawer accessibility tap area #11184
Conversation
Hey mikegarfinkle! Thanks for opening your first pull request in this repo. If you haven't already, make sure to read our contribution guidelines. |
✅ Deploy Preview for react-navigation-example ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
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.
Thanks for the PR. I have same comment as this PR.
If this is not correct, can you share a video of how a native drawer works with talkback so and other accessibility tech so the drawer implementation can match it?
Hi! Thanks so much for taking a look at this. To address your question (I've used Gmail as the example app to show default native behaviour): Android setting: In Android, there is no Talkback accessibility for the overlay area in a drawer. This is because the Android ever-present back button can dismiss the drawer AND the overlay area is not tappable. However, in react-navigation drawer, the overlay is tappable and currently does not have an accessibility label to tell visually impaired users what tapping will do. As all tappable components should have an accessibility label, I see two ways forward: 1) eliminate tappable overlay (swipe would still work, but this would involve simply pressing on the overlay) in Android, or 2) keep the overlay and add accessibility label as I have done in this PR. iOS setting: On iOS, Navigation Drawers are not a native design pattern. However, implementations on a drawer have been created, such as with the Gmail app. In the Gmail implementation, Google has made the overlay as a tappable button (with an accessibility label that is read by iOS VoiceOver) to help visually impaired users dismiss the drawer (as there is no ever-present back button like there is in Android). I believe react-navigation drawer in iOS would benefit from a similar approach: ie. making the overlay tappable to dismiss (which is already done) with an accessibility label (which this PR adds). |
This isn't correct. I use Android as my daily driver and the drawer is always dismissable by tapping on the overlay area. Regarding the back button, there is no such button in recent versions of Android. There's a system back gesture on Android which does the same thing.
Makes sense. Then we can do the same. It may also be necessary to |
040899a
to
36ecb77
Compare
Ah let me clarify. Google, at least in the Gmail app, hasn't made it accessible. So they have made it tappable, but they didn't add an accessibility label on this tap functionality. This I think we can add though for react navigation! |
Good call on explicitly adding the accessibilityRole, and will add default value too (being cognizant that will always be reading english if this is not set by the dev, regardless of local). |
36ecb77
to
11a9b3e
Compare
I was wrong here (thanks for pointing this out!). The overlay IS tappable and dismisses the drawer in native Android. However, Google has not made this tap functionality accessible. I think this is a bit of an oversight, and we could make this accessible, as I have done in my PR, for react-navigation drawer Android. |
11a9b3e
to
00940d9
Compare
00940d9
to
c48f2f0
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.
Thank you!
Codecov ReportBase: 74.09% // Head: 74.09% // No change to project coverage 👍
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #11184 +/- ##
=======================================
Coverage 74.09% 74.09%
=======================================
Files 176 176
Lines 5598 5598
Branches 2193 2193
=======================================
Hits 4148 4148
Misses 1401 1401
Partials 49 49
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
**Motivation** Currently, a user can tap the `Overlay` next to an open drawer to close the drawer. This is useful in an Accessibility setting when swipe actions are not obvious to the user. Unfortunately, it is not obvious to someone with a visual deficit that this area can be tapped to close the drawer, as this overlay does not have an accessibilityLabel prop exposed. This PR exposes an `accessibilityLabel` prop on this overlay component and modifies existing accessibility props to allow for accessibility tap permissions on the overlay component. It then surfaces this prop at the `Drawer` level as `closeAccessibilityLabel`. **Test plan** Open up the example app with TalkBack (Android) or VoiceOver (iOS) enabled. Tap on the overlay next to the drawer and observe that it will now read out as "Close drawer".
**Motivation** Currently, a user can tap the `Overlay` next to an open drawer to close the drawer. This is useful in an Accessibility setting when swipe actions are not obvious to the user. Unfortunately, it is not obvious to someone with a visual deficit that this area can be tapped to close the drawer, as this overlay does not have an accessibilityLabel prop exposed. This PR exposes an `accessibilityLabel` prop on this overlay component and modifies existing accessibility props to allow for accessibility tap permissions on the overlay component. It then surfaces this prop at the `Drawer` level as `closeAccessibilityLabel`. **Test plan** Open up the example app with TalkBack (Android) or VoiceOver (iOS) enabled. Tap on the overlay next to the drawer and observe that it will now read out as "Close drawer".
**Motivation** Currently, a user can tap the `Overlay` next to an open drawer to close the drawer. This is useful in an Accessibility setting when swipe actions are not obvious to the user. Unfortunately, it is not obvious to someone with a visual deficit that this area can be tapped to close the drawer, as this overlay does not have an accessibilityLabel prop exposed. This PR exposes an `accessibilityLabel` prop on this overlay component and modifies existing accessibility props to allow for accessibility tap permissions on the overlay component. It then surfaces this prop at the `Drawer` level as `closeAccessibilityLabel`. **Test plan** Open up the example app with TalkBack (Android) or VoiceOver (iOS) enabled. Tap on the overlay next to the drawer and observe that it will now read out as "Close drawer".
**Motivation** Currently, a user can tap the `Overlay` next to an open drawer to close the drawer. This is useful in an Accessibility setting when swipe actions are not obvious to the user. Unfortunately, it is not obvious to someone with a visual deficit that this area can be tapped to close the drawer, as this overlay does not have an accessibilityLabel prop exposed. This PR exposes an `accessibilityLabel` prop on this overlay component and modifies existing accessibility props to allow for accessibility tap permissions on the overlay component. It then surfaces this prop at the `Drawer` level as `closeAccessibilityLabel`. **Test plan** Open up the example app with TalkBack (Android) or VoiceOver (iOS) enabled. Tap on the overlay next to the drawer and observe that it will now read out as "Close drawer".
Motivation
Currently, a user can tap the
Overlay
next to an open drawer to close the drawer. This is useful in an Accessibility setting when swipe actions are not obvious to the user. Unfortunately, it is not obvious to someone with a visual deficit that this area can be tapped to close the drawer, as this overlay does not have an accessibilityLabel prop exposed.This PR exposes an
accessibilityLabel
prop on this overlay component and modifies existing accessibility props to allow for accessibility tap permissions on the overlay component. It then surfaces this prop at theDrawer
level ascloseAccessibilityLabel
.Test plan
Open up the example app with TalkBack (Android) or VoiceOver (iOS) enabled. Tap on the overlay next to the drawer and observe that it will now read out as "Close drawer".