Mobile: Accessibility: Improve focus handling in the note actions menu and modal dialogs #11929
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Note: This commit has been manually tested by: 1. Enabling VoiceOver (MacOS). 2. Moving VoiceOver focus into the iOS simulator. 3. Opening a note. 4. Opening the note actions menu. 5. Navigating through the options in the menu, verifying that focus does not visit the note title or other parts of the app. - An exception to this is the "edit" button, which is shown using a React Native <Portal>, which displays content outside of the region with disabled focus. 6. Moving focus back to the "Attach" button. 7. Pressing the button. 8. Selecting "Record audio" in the attach menu. 9. Verifying that the "Voice recorder" heading is auto-focused.
personalizedrefrigerator
added
mobile
| > | ||
| <AccessibleView | ||
| // Auto-focuses an empty view at the beginning of the sidemenu -- if we instead | ||
| // focus the container view, VoiceOver fails to focus to any components within | ||
| // the sidebar. | ||
| refocusCounter={!open ? 1 : undefined} | ||
| testID='sidemenu-menu-focus-region' |
There was a problem hiding this comment.
Currently, testID, if provided, is included in the focus log message. It may make sense to use a different prop for this (e.g. debugTag) and make it required if refocusCounter is specified.
dismissing modals
When closed, Android modals re-focus whatever had focus before the modal was open. This was conflicting with the logic that disables focusability for the main app content until all modals are closed.
Adding onAccessibilityEscape to the note actions menu should be done separately.
Comment on lines
+5
to
+11
| const FocusControl = { | ||
| Provider: FocusControlProvider, | ||
| ModalWrapper, | ||
| MainAppContent, | ||
| }; | ||
|
|
||
| export default FocusControl; |
There was a problem hiding this comment.
The goal of wrapping these components in a FocusControl object was to make it clear to a reader that MainAppContent (and similar components) are specific to the FocusControl logic. An alternative could be to rename ModalWrapper to FocusControlModalWrapper and MainAppContent to FocusControlMainAppContent (or similar).
84 tasks
|
The most recent CI failure seems related to desktop end-to-end tests (and thus unrelated to this pull request): Logs |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This pull request adds custom focus-handling logic to the mobile app. Specifically, it:
inert(web) and not accessibility focusable (iOS and Android). This should prevent focus from leaving open modals and focusing other parts of the app. (WCAG 2.2 SC 2.4.3. Focus order, SC 2.4.11: Focus not obscured).FocusControl.ModalContentcomponent hasvisible={true}.AccessibleViewthat was auto-focused while the menu was open. (WCAG 2.2 SC 2.4.3. Focus order)This pull request adds a document to
readme/dev/specwith more information about how the updated focus management works.To-do
This pull request, if feasible:
AccessibilityInfo.setAccessibilityFocus.This pull request or a follow-up pull request:
<Portal>. This causes React Native Paper to render it outside of theinert/importantForAccessibility={false}region. As a result, it's still possible to click the "edit" button while the note actions menu is open. Currently, this switches to edit mode, but does not dismiss the note actions menu. Either:Testing plan
This pull request has been tested manually (iOS 18.1 simulator & Android 13) by:
<Portal>, which displays content outside of the region with disabled focus.Additionally, on Android 13,