[Android] Selected State does not annonce when TextInput Component selected #31144

fabOnReact · 2021-03-12T12:02:25Z

Summary

This issue fixes #30955 and is a follow up to pr #24608 which added the basic Accessibility functionalities to React Native.

TextInput should announce "selected" to the user when screenreader focused.
The focus is moved to the TextInput by navigating with the screenreader to the TextInput.

This PR adds call to View#setSelected in BaseViewManager https://developer.android.com/reference/android/view/View#setSelected(boolean)
The View#setSelected method definition https://github.com/aosp-mirror/platform_frameworks_base/blob/master/core/java/android/view/View.java

/**
 * Changes the selection state of this view. A view can be selected or not.
 * Note that selection is not the same as focus. Views are typically
 * selected in the context of an AdapterView like ListView or GridView;
 * the selected view is the view that is highlighted.
 *
 * @param selected true if the view must be selected, false otherwise
 */
public void setSelected(boolean selected) {
  if (((mPrivateFlags & PFLAG_SELECTED) != 0) != selected) {
    // ... hidden logic
    if (selected) {
      sendAccessibilityEvent(AccessibilityEvent.TYPE_VIEW_SELECTED);
    } // ... hidden logic
  }
}

VoiceOver and TalkBack was tested with video samples included below.

Changelog

[Android] [Fixed] - Fix Selected State does not announce when TextInput Component selected on Android

Test Plan

CLICK TO OPEN TESTS RESULTS

ENABLE THE AUDIO to hear the TalkBack announcing SELECTED when the user taps on the TextInput

        <TextInput
          accessibilityLabel="element 20"
          accessibilityState={{
            selected: true,
          }} />

selected is true
2021-03-18.16-33-01.mp4

        <TextInput
          accessibilityLabel="element 20"
          accessibilityState={{
            selected: false,
          }} />

selected is false
2021-03-18.16-32-39.mp4

The functionality does not present issues on iOS

iOS testing
RecordIt-1616079277.mp4

facebook#30955

analysis-bot · 2021-03-12T12:23:38Z

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: ac7ba3e

analysis-bot · 2021-03-12T12:32:51Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,952,105	+338
android	hermes	armeabi-v7a	8,440,911	+319
android	hermes	x86	9,445,625	+332
android	hermes	x86_64	9,388,181	+334
android	jsc	arm64-v8a	10,684,242	+248
android	jsc	armeabi-v7a	10,156,166	+256
android	jsc	x86	10,739,264	+235
android	jsc	x86_64	11,322,088	+238

Base commit: ac7ba3e

packages/rn-tester/js/examples/TextInput/TextInputSharedExamples.js

fabOnReact · 2021-03-16T09:45:32Z

ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java

@@ -166,8 +166,12 @@ public void setViewState(@NonNull T view, @Nullable ReadableMap accessibilitySta
    if (accessibilityState == null) {
      return;
    }
+    if (accessibilityState.hasKey("selected")) {
+      view.setSelected(accessibilityState.getBoolean("selected"));


the java test case does not verify that view.setSelected(true) is invoked, but just verifies view.setTag(R.id.accessibility_state, accessibilityState), I will update the test to verify those scenarios.

Solved with commit 79486b5 more info at #31144 (comment) 🙏 ☮️

packages/rn-tester/js/examples/Accessibility/AccessibilityExample.js

kacieb · 2021-03-16T20:30:53Z

packages/rn-tester/js/examples/Accessibility/AccessibilityExample.js

+      <View>
+        <TextInput
+          accessible={true}
+          accessibilityState={{selected: this.state.focused}}


cc @blavalla - Should the TextInput component always announce "selected" if it's focused and always announce "not selected" if it's blurred? Should this behavior be built into the component?

I think there is some confusion here as to the usage of the "selected" property, which is not surprising given its vague name, and how other things (like text within the input for example) can also be "selected"!

This property is meant to represent an element that is highlighted visually for some reason. Maybe there's a list of text inputs, and only one of them is active at a time for some reason, and is highlighted in yellow to show this visually. This item would be "selected", while the others would be "not selected".

It doesn't really pertain to focus being within the input itself, or whether that input is currently being edited, or anything like that.

This same selected property can be applied to any type of view, and the issue here is simply that it doesn't work for <TextInput> like it does for <View>.

Oh I see, thank you for the explanation! So the goal of the issue is just to allow the selected state to be set at the user's discretion, and not to always set it at any one time.

kacieb · 2021-03-16T20:31:01Z

packages/rn-tester/js/examples/Accessibility/AccessibilityExample.js

@@ -716,10 +725,8 @@ class SetAccessibilityFocusExample extends React.Component<{}> {

    const onClose = () => {
      if (myRef && myRef.current) {
-        AccessibilityInfo.sendAccessiblityEvent_unstable(


This new method does the same thing as setAccessibilityFocus does, with the exception that this unstable method is fabric-friendly! Did this method not work for you? We don't want to change it back if we can avoid it!

@kacieb Yes, I experienced this issue with the current master branch

Do you want me to keep this change, reverse it or to investigate and fix the method?
I can not find sendAccessiblityEvent_unstable in our codebase. Thanks a lot 🙏 ☮️

Ah got it, maybe this only works internally currently. Since this is an RNTester example, it should be ok to keep this change for now, and we can resolve it internally later.

cc @JoshuaGross and @mdvacca for awareness!

Thanks @kacieb I reverted this change with commit 8e76ab3 🙏 ☮️

There's a missing "i", it should be : "sendAccessibilityEvent_unstable" instead of "sendAccessiblityEvent_unstable"

kacieb · 2021-03-16T20:32:05Z

Hey! Thanks so much for working on this! I'm excited for this fix.

I'm adding in @blavalla as a reviewer to help on the Android side!

blavalla · 2021-03-16T22:44:18Z

Just to add further clarification here, when referring to "focus" in the original issue, it is meant to mean screenreader focus, not keyboard focus. Android unfortunately uses the same term for both of these concepts, so discussing them can get confusing!

Keyboard-focus behavior is currently working as expected on TextInput, so doesn't need to be changed.

analysis-bot

Code analysis results:

eslint found some issues. Run yarn lint --fix to automatically fix problems.

packages/rn-tester/js/examples/Accessibility/AccessibilityExample.js

blavalla

The Android code in BaseViewManager.java looks good to me.

I am not really familiar with how the testing system works here, and why it's not able to mock out the view though. @kacieb, is there anyone on the React Core team who knows this system and could take a look?

kacieb · 2021-03-17T22:08:33Z

I am not really familiar with how the testing system works here, and why it's not able to mock out the view though. @kacieb, is there anyone on the React Core team who knows this system and could take a look?

Sorry, I think I'm not understanding what you mean by this? Is the test not working as expected?

blavalla · 2021-03-17T22:15:33Z

I am not really familiar with how the testing system works here, and why it's not able to mock out the view though. @kacieb, is there anyone on the React Core team who knows this system and could take a look?

Sorry, I think I'm not understanding what you mean by this? Is the test not working as expected?

Sorry, I may have been looking at something thats not an issue anymore. There was a few comments about the tests not working here:
https://github.com/facebook/react-native/pull/31144/files#r595064322

@fabriziobertoglio1987, just to confirm, did you get the tests working correctly?

@return

The test below does not fail in the master branch as it does not verify the new behaviour added with pr facebook#31144 The test previously would just verify that the `mView` had a tag `R.id.accessibility_state` with value equal to variable `accessibilityState` ```java assertThat(mView.getTag(R.id.accessibility_state)).isEqualTo(accessibilityState) ```` https://github.com/facebook/react-native/blob/a6cc5871e68b63e2f29bcb3610968dd33b926694/ReactAndroid/src/test/java/com/facebook/react/uimanager/BaseViewManagerTest.java#L72-L77 Now the test verifies that `mView.isSelected()` is true after passing `accessibilityState.selected` true. I previously tried to run this verification by mocking `mView` instance, but it is not possible and I later found the public method in the Android sourcecode. https://developer.android.com/reference/android/view/View#isSelected() ```java /** * Indicates the selection state of this view. * * @return true if the view is selected, false otherwise */ @ViewDebug.ExportedProperty @InspectableProperty(hasAttributeId = false) public boolean isSelected() { return (mPrivateFlags & PFLAG_SELECTED) != 0; } ``` Related to https://github.com/facebook/react-native/pull/31144/files#r595064322

fabOnReact · 2021-03-18T11:27:07Z

Thanks a lot @blavalla @kacieb for your Feedback! The comment https://github.com/facebook/react-native/pull/31144/files#r595007827 is now solved with commit 79486b5

The test discussed would not fail in the master branch as it does not verify the new behaviour added with pr #31144 in java.
The test previously would just verify that the mView had a tag R.id.accessibility_state with value equal to variable accessibilityState (the value is { selected: true}), but this behaviour was not changed in pr #31144.

assertThat(mView.getTag(R.id.accessibility_state)).isEqualTo(accessibilityState)

react-native/ReactAndroid/src/test/java/com/facebook/react/uimanager/BaseViewManagerTest.java

Lines 72 to 77 in a6cc587

    
           public void testAccessibilityStateSelected() { 
        
             WritableMap accessibilityState = Arguments.createMap(); 
        
             accessibilityState.putBoolean("selected", true); 
        
             mViewManager.setViewState(mView, accessibilityState); 
        
             assertThat(mView.getTag(R.id.accessibility_state)).isEqualTo(accessibilityState); 
        
           }

Now the test verifies that mView.isSelected() returns value of true. The test fails in the current master branch (mView.isSelected() is false), because the bug of Issue #30955 is not fixed in the master branch, while the test does not fail in my pr #31144.
I previously tried to run this verification by mocking mView instance and spy on the setSelected() call, but it was not possible and I later found the public method isSelected while reading the Android sourcecode.

https://developer.android.com/reference/android/view/View#isSelected()

    /**
     * Indicates the selection state of this view.
     *
     * @return true if the view is selected, false otherwise
     */
    @ViewDebug.ExportedProperty
    @InspectableProperty(hasAttributeId = false)
    public boolean isSelected() {
        return (mPrivateFlags & PFLAG_SELECTED) != 0;
    }

Thanks a lot to Blavalla and Kacie Bawiec.
Related to https://github.com/facebook/react-native/pull/31144/files#r595064322

is a useful reminder in the future to fix fabric method sendAccessiblityEvent_unstable in the future https://github.com/facebook/react-native/pull/31144/files#r596273281

analysis-bot

Code analysis results:

eslint found some issues. Run yarn lint --fix to automatically fix problems.

packages/rn-tester/js/examples/Accessibility/AccessibilityExample.js

I have been working for a while with typescript and forgot to update my vim setup :-)

fabOnReact · 2021-03-18T15:43:18Z

I removed the prop accessibilityState that I previously added to TextInput. I re-run and updated all video recordings of test cases on Android and iOS #31144 (comment), the accessibilityState prop seems to not be required in the TextInput component for both iOS and Android. Thanks a lot and sorry for my mistake. 🙏 ☮️

react-native/Libraries/Components/TextInput/TextInput.js

Lines 414 to 415 in 6012e98

    
           export type Props = $ReadOnly<{| 
        
             ...$Diff<ViewProps, $ReadOnly<{|style: ?ViewStyleProp|}>>,

react-native/Libraries/Components/View/ViewPropTypes.js

Lines 423 to 426 in 6012e98

    
             /** 
        
              * Indicates to accessibility services that UI Component is in a specific State. 
        
              */ 
        
             accessibilityState?: ?AccessibilityState,

facebook-github-bot · 2021-03-24T21:14:33Z

@kacieb has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2021-04-05T18:51:25Z

@kacieb merged this pull request in 7ee2acc.

amarlette · 2021-04-05T21:48:15Z

Thank you @fabriziobertoglio1987 for this contribution! I would like to give you a shout-out on Twitter and include you in our end-of-month issues update for your contribution. Do you have a Twitter we can tag?

fabOnReact · 2021-04-06T05:14:32Z

Thanks a lot @amarlette I don't use often twitter and I would prefer that you use my github account https://github.com/fabriziobertoglio1987. I have an old twitter account https://twitter.com/fabri_on_rails. Feel free to use any of those account (twitter and/or github). I wish you a good day ☮️ 🙏 😃

fix Selected State does not annonce when TextInput Component selected

5b6d5ed

facebook#30955

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 12, 2021

This comment has been minimized.

Sign in to view

fabOnReact mentioned this pull request Mar 12, 2021

Selected State does not annonce when TextInput Component selected #30955

Closed

fabOnReact added 2 commits March 12, 2021 17:17

add test to verify accessibleState prop is passed to Component

502cac6

minor change

74674ff

fabOnReact changed the title ~~Selected State does not annonce when TextInput Component selected~~ [Android] Selected State does not annonce when TextInput Component selected Mar 12, 2021

fabOnReact added 2 commits March 15, 2021 11:52

java test for setViewState accessibilityState

94c5af8

remove createArray PowerMock

b02a426

fabOnReact commented Mar 15, 2021

View reviewed changes

packages/rn-tester/js/examples/TextInput/TextInputSharedExamples.js Outdated Show resolved Hide resolved

fabOnReact marked this pull request as ready for review March 15, 2021 16:26

amarlette added the Accessibility label Mar 15, 2021

amarlette requested a review from kacieb March 15, 2021 23:14

fabOnReact commented Mar 16, 2021

View reviewed changes

moving TextInput example to AccessabilityExample

e04fa92

fabOnReact commented Mar 16, 2021

View reviewed changes

packages/rn-tester/js/examples/Accessibility/AccessibilityExample.js Outdated Show resolved Hide resolved

kacieb reviewed Mar 16, 2021

View reviewed changes

kacieb requested a review from blavalla March 16, 2021 20:32

rewriting test case in Accessibility Example

efd34da

analysis-bot suggested changes Mar 17, 2021

View reviewed changes

packages/rn-tester/js/examples/Accessibility/AccessibilityExample.js Outdated Show resolved Hide resolved

fabOnReact marked this pull request as draft March 17, 2021 14:31

remove Accessibility TextInput Example

a6cc587

blavalla reviewed Mar 17, 2021

View reviewed changes

revert this change as rn-tester is meant for internal use

8e76ab3

is a useful reminder in the future to fix fabric method sendAccessiblityEvent_unstable in the future https://github.com/facebook/react-native/pull/31144/files#r596273281

fabOnReact marked this pull request as ready for review March 18, 2021 11:49

analysis-bot suggested changes Mar 18, 2021

View reviewed changes

packages/rn-tester/js/examples/Accessibility/AccessibilityExample.js Outdated Show resolved Hide resolved

fabOnReact marked this pull request as draft March 18, 2021 11:59

fix lint issues

3659930

I have been working for a while with typescript and forgot to update my vim setup :-)

fabOnReact marked this pull request as ready for review March 18, 2021 12:11

fabOnReact marked this pull request as draft March 18, 2021 13:54

Merge branch 'master' into fix/text-input-accessible-selected

ec338bc

fabOnReact marked this pull request as ready for review March 18, 2021 14:01

fabOnReact marked this pull request as draft March 18, 2021 15:10

fabOnReact added 2 commits March 18, 2021 16:13

remove accessibilityState from TextInput.js

3a0201b

remove changes to JS tests as there are no JS changes

6012e98

fabOnReact marked this pull request as ready for review March 18, 2021 15:36

fabOnReact added 5 commits March 18, 2021 18:09

adding i to sendAccessiblityEvent_unstable

fe7a0fe

avoid runtime error with sendAccessibilityEvent_unstable

1203852

revert check to avoid runtime error

c5cf843

revert change - does not fix error and out of scope for facebook#30955

969d0c7

Merge branch 'master' into fix/text-input-accessible-selected

76d3169

facebook-github-bot closed this in 7ee2acc Apr 5, 2021

facebook-github-bot added the Merged This PR has been merged. label Apr 5, 2021

[Android] Selected State does not annonce when TextInput Component selected #31144

[Android] Selected State does not annonce when TextInput Component selected #31144

Uh oh!

Conversation

fabOnReact commented Mar 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changelog

Test Plan

Uh oh!

This comment has been minimized.

analysis-bot commented Mar 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

analysis-bot commented Mar 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

fabOnReact Mar 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as outdated.

Uh oh!

fabOnReact Mar 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kacieb Mar 16, 2021

Choose a reason for hiding this comment

Uh oh!

blavalla Mar 16, 2021

Choose a reason for hiding this comment

Uh oh!

kacieb Mar 16, 2021

Choose a reason for hiding this comment

Uh oh!

kacieb Mar 16, 2021

Choose a reason for hiding this comment

Uh oh!

fabOnReact Mar 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kacieb Mar 17, 2021

Choose a reason for hiding this comment

Uh oh!

fabOnReact Mar 18, 2021

Choose a reason for hiding this comment

Uh oh!

mdvacca Mar 18, 2021

Choose a reason for hiding this comment

Uh oh!

kacieb commented Mar 16, 2021

Uh oh!

blavalla commented Mar 16, 2021

Uh oh!

analysis-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

blavalla left a comment

Choose a reason for hiding this comment

Uh oh!

kacieb commented Mar 17, 2021

Uh oh!

blavalla commented Mar 17, 2021

Uh oh!

fabOnReact commented Mar 18, 2021

Uh oh!

analysis-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fabOnReact commented Mar 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fabOnReact commented Mar 12, 2021 •

edited

Loading

analysis-bot commented Mar 12, 2021 •

edited

Loading

analysis-bot commented Mar 12, 2021 •

edited

Loading

fabOnReact Mar 16, 2021 •

edited

Loading

fabOnReact Mar 18, 2021 •

edited

Loading

fabOnReact Mar 17, 2021 •

edited

Loading

fabOnReact commented Mar 18, 2021 •

edited

Loading