Add Functionality to setAccessibilityFocus Method #7268
Conversation
vnext/Microsoft.ReactNative/Modules/AccessibilityInfoModule.cpp
Outdated
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Modules/AccessibilityInfoModule.cpp
Outdated
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Modules/AccessibilityInfoModule.cpp
Outdated
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Modules/AccessibilityInfoModule.cpp
Outdated
Show resolved
Hide resolved
vnext/Microsoft.ReactNative/Modules/AccessibilityInfoModule.cpp
Outdated
Show resolved
Hide resolved
ghost
added
the
Needs: Author Feedback
vnext/src/Libraries/Components/AccessibilityInfo/AccessibilityInfo.windows.js
Outdated
Show resolved
Hide resolved
NickGerleman
left a comment
NickGerleman
left a comment
There was a problem hiding this comment.
The JS from React Native Core expects this to be exposed as a UIManager method. Can we preserve the existing interface instead of changing it?
ghost
removed
the
Needs: Author Feedback
|
This looks like its moving focus, not accessibility focus. The point of this method is to move accessibility focus without moving focus. (Otherwise there isn't any point in having this API vs the focus API). If you have narrator running the expected result would be that narrators blue focus rect would move, but the actual focus location wouldn't. Similar to moving narrator focus using Caps+arrrow keys. |
@NickGerleman Yea I can do that. Do you want to leave the code as calling the legacySendAccessibilityEvent method and from within that method calling UIManager.sendAccessibilityEvent? Or setAccessibilityFocus calling straight to UIManager.sendAccessibilityEvent? |
@acoates-ms I did some digging in my email and found a thread with Marko from the accessibility team who said "On Windows, we don’t have a separate concept of “accessibility focus”. An element either has keyboard focus or it doesn’t—UIA is supposed to represent the reality of UI, so there’s no separate accessibility focus." So it sounds like even when you are moving narrator focus using the Caps+arrow keys that keyboard focus is being moved during this as well and trying to move narrator focus without keyboard focus would put UIA in a place of undefined/untested behavior. I know with RN we're juggling between the RN standard and the Windows standard, but would I be right in guessing that there is an expectation for accessibility tools to behave consistently across all apps within Windows? In this case setAccessibilityFocus would have the same behavior as the focus() API, but it seems like there is some demand for this API to be implemented specifically, since there will be React Native apps looking to add Windows support which use the setAccessibilityFocus API in their source and may not want to have to add platform specific code to check if Windows is the target and change the API to focus(). I guess the question is, is an implementation that is different from the RN behavior but an adaption of the method for Windows better than the method going unsupported? |
vnext/src/Libraries/Components/AccessibilityInfo/legacySendAccessibilityEvent.windows.js
Outdated
Show resolved
Hide resolved
…aramooney/react-native-windows into cm-fix-setAccessibilityFocus
| void sendAccessibilityEvent(int64_t reactTag, double eventType) noexcept { | ||
| assert(false); | ||
| // TODO | ||
| if (eventType == 0) { // FocusAccessibilityEvent Type |
There was a problem hiding this comment.
I see that in RN this value is coming from
https://github.com/facebook/react-native/blob/6e6443afd04a847ef23fb6254a84e48c70b45896/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerModuleConstants.java#L116-L117
Do we expect apps to ever use the int value instead of the string? if so, the value should be 8:
| // TODO | ||
| if (eventType == 0) { // FocusAccessibilityEvent Type | ||
| focus(reactTag); | ||
| } |
| vm->GetConstants(writer); | ||
| writer.WriteObjectEnd(); | ||
| } | ||
| auto AccessibilityEventTypes = React::JSValueObject{ |
There was a problem hiding this comment.
nit, name local variable name starting with lowercase
Also this can be made const
…ndows into cm-fix-setAccessibilityFocus
| void sendAccessibilityEvent(int64_t reactTag, double eventType) noexcept { | ||
| assert(false); | ||
| // TODO | ||
| if (eventType == 8) { // AccessibilityEventTypes.typeViewFocused Type |
There was a problem hiding this comment.
Could we put this in a constant instead of hatdcodihg "8" in both places?
5 participants
Why
Current implementation of setAccessibilityFocus method resulted in no action.
What
The RN setAccessibilityFocus API is intended to force the narrator focus to a specific element and then trigger narrator to announce that element.
How
Added source code to the sendAccessibilityEvent method inside PaperUIManager. New source code identifies the new element to focus on via its reactTag, focuses the element, and raises an automation event to alert narrator of the focus change. The RN setAccessibilityFocus API now calls the native method.
Testing
Tested by modifying a page in the playground app to call setAccessibilityFocus following the press of a button. The focus target was a second button. With narrator on, the first button was focused and pressed, triggering the API, and moving narrator focus to the second button. Its component type and accessibility label were then announced. I also ran the playground-win32 app to ensure that the functionality continues to work for XAML islands.
Resolves #5907
Microsoft Reviewers: Open in CodeFlow