Closed amarlette closed 3 years ago
huzaifaaak
commented
3 years ago @kacieb @lunaleaps Image component doesn't have a disabled prop. Does adding accessibilityState to it make's sense or Talkback should just announce the accessibilityLabel ?
Edit: I don't think accessibilityState would make sense on image. accessible={true} will make it focusable on android.
huzaifaaak
commented
3 years ago @lunaleaps @blavalla Can you please confirm this ☝️ . My PR is almost ready for this
blavalla
commented
3 years ago @huzaifaaak, just to clarify, the "disabled" field in accessibilityState isn't used to block something from becoming focusable. As you called out, setting accessible={true} (or false) is used for that purpose, and should properly work already. What "disabled" does is let the user know that this element is currently in a disabled state and can't be clicked or interacted with. They can still focus on it however and hear a description of that element. It's most commonly used for form fields that aren't enabled until some requirement has been met, such as filling out another field.
While it may be strange to commonly want to make an image "disabled", there are at least some use cases I can think of. For example, those captchas where you select "all the images that contain X". Once you select one, maybe it becomes disabled to make it clear it can no longer be selected.
The main issue here is that disabled: true on it, but this has no effect at all. In the end, if the API allows us to set this property, it should at least work as intended, even if its real-world uses are edge-cases.
huzaifaaak
commented
3 years ago I have broken this down into two cases that announces the disabled accessibilityState:
ViewProps.ACCESSIBILITY_STATE in ReactImageManager.java.View focusable in setViewState function in BaseViewManager.java.I think it would not be a good idea to add it in the BaseViewManager.java (Assumption). I am pausing this. If anyone want's to work on this
fabOnReact
commented
3 years ago I can not move the focus on the Image
CLICK TO OPEN TESTS RESULTS -
| | |:-------------------------:| | |
blavalla
commented
3 years ago @fabriziobertoglio1987, do you have a link to the code that rendered that image? It's possible that it had no accessibilityLabel, or was not set as "accessible", so would not get focused by default.
fabOnReact
commented
3 years ago Thanks a lot @blavalla, I'm trying to troubleshoot this issue and also better understand both BaseViewManager and ReactAccessibilityDelegate
I'm taking the new Button functionality as a reference to understand how it should work and debug the native code.
CLICK TO OPEN TESTS RESULTS - BUTTON - ACCESSIBILITY DISABLED ANNOUNCED
TEST SCENARIO - The `Button` with `accessibilityState={{disabled: true}}` will announce `disabled` after reading the `accessibilityLabel` RESULT - SUCCESSFUL - The screen reader will read the Accessibility Label and add the `disabled` at the end of this label ```javascript
CLICK TO OPEN TESTS RESULTS - IMAGE - ACCESSIBILITY DISABLED NOT ANNOUNCED
TEST SCENARIO
- The image has `accessibilityState={{disabled: true}}` and `accessibilityLabel="testing"`. The screen reader should announce `testing disabled`.
RESULT - FAILURE
- The screen reader announces `testing`
```javascript
Currently I am investigating and debugging how ReactAndroid generates the string Press to submit your application button disabled instead of Press to submit your application.
fabOnReact
commented
3 years ago The Accessibility info are initially set with the setViewState ReactProp on the View
The onAfterUpdateTransaction callback is used to set the ReactAccessibilityDelegate
ReactImageManager enhances this methdo by calling view.maybeUpdateView()
after this all the Accessibility functionalities are delegated to ReactAccessibilityDelegate https://github.com/facebook/react-native/blob/eacc94005b1b1d33d47b3bea88a6fd242560dafb/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageManager.java#L256-L260
blavalla
commented
3 years ago @fabriziobertoglio1987 , From an Android standpoint, the thing that matters is whether the "enabled" property of the AccessibilityNodeInfo is set to false. If it is, it will announce "disabled", if it's not, it won't. This property is set in one of two ways:
1.) Setting it on the View directly, which when the view generates the AccessibilityNodeInfo will pull its own value for enabled and populate it to be the same. 2.) Setting it on the AccessibilityNodeInfo associated with the view via something like an AccessibilityDelegate.
In React Native, we seem to be doing both of these things, and they seem to potentially conflict. I'm not certain that this conflict is the cause of the issue, but it definitely can't be helping.
In BaseViewManager, we are always setting enabled to be "true" here:
And in ReactAccessibilityDelegate we are setting it to be the value of the accessibilityState's "disabled" key here:
What that means is that if you set accessibilityState={{disabled: true}} you will end up with a View that has enabled="true" and an AccessibilityNodeInfo that has enabled="false". In theory this shouldn't cause a problem, as the AccessibilityNodeInfo should take precedence, but when these are specifically set may make a difference. You could try removing the call in BaseViewManager to see if it has any impact.
I think the more likely culprit here though is the same one that was on the <Button> component, which is that the accessibilityState prop is simply being ignored. Check out this pull request to see how it was solved for Button, as that same approach may work here as well.
fabOnReact
commented
3 years ago Thanks @blavalla, if I comment the setEnabled line, the Button will not announce disabled.
The line is executed both with the Image and Button (I debug both).
In both cases the method is called with accessibilityState={{disabled: true}}, but the Image does not announce disabled.
Is it an issue with the ReactImageView?
https://github.com/facebook/react-native/blob/eacc94005b1b1d33d47b3bea88a6fd242560dafb/ReactAndroid/src/main/java/com/facebook/react/views/image/ReactImageView.java#L69
The DraweeView inherits from ImageView and View
https://github.com/facebook/fresco/blob/master/drawee/src/main/java/com/facebook/drawee/view/GenericDraweeView.java
`setEnabled` is defined in View and overriden in each subclass
https://github.com/aosp-mirror/platform_frameworks_base/blob/master/core/java/android/view/View.java
**
```java
/**
* Set the enabled state of this view. The interpretation of the enabled state varies by subclass.
*
* @param enabled True if this view is enabled, false otherwise.
*/
@RemotableViewMethod
public void setEnabled(boolean enabled) {
if (enabled == isEnabled()) return;
setFlags(enabled ? ENABLED : DISABLED, ENABLED_MASK);
/*
* The View most likely has to change its appearance, so refresh
* the drawable state.
*/
refreshDrawableState();
// Invalidate too, since the default behavior for views is to be
// be drawn at 50% alpha rather than to change the drawable.
invalidate(true);
if (!enabled) {
cancelPendingInputEvents();
}
}
```
OPEN TO SEE SOURCECODE View#setEnabled
**
Is this an issue with the ShadowNode (ReactImageManager)?
**
https://github.com/facebook/react-native/blob/eacc94005b1b1d33d47b3bea88a6fd242560dafb/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNodeImpl.java#L32-L54
For example ReactPickerManager over-rides the setEnabled method.
https://github.com/facebook/react-native/blob/eacc94005b1b1d33d47b3bea88a6fd242560dafb/ReactAndroid/src/main/java/com/facebook/react/views/picker/ReactPickerManager.java#L47-L50
The definition of ReactShadowNodeImpl
**
Unluckily I don't have a solution for this problem right now, but I plan to work on other related issues and gain a better understanding of the ReactAndroid and Android relevant sourcecode. Thanks :🙏 ☮
fabOnReact
commented
3 years ago I have broken this down into two cases that announces the disabled accessibilityState:
1. Add the `ViewProps.ACCESSIBILITY_STATE` in ReactImageManager.java. 2. Make `View` focusable in `setViewState` function in BaseViewManager.java.I think it would not be a good idea to add it in the BaseViewManager.java (Assumption). I am pausing this. If anyone want's to work on this
CLICK TO OPEN TEST CASE
adding setFocusable to ReactImageManager and passing the accessible prop to Image component fixes the issue.
definition of ShadowNodes in React Native
https://github.com/facebook/react-native/blob/eacc94005b1b1d33d47b3bea88a6fd242560dafb/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactShadowNodeImpl.java#L32-L54
Information available in Android Open Source View Class on Focus
https://github.com/aosp-mirror/platform_frameworks_base/blob/master/core/java/android/view/View.java ```java *
The framework will handle routine focus movement in response to user input. This includes * changing the focus as views are removed or hidden, or as new views become available. Views * indicate their willingness to take focus through the {@link #isFocusable} method. To change * whether a view can take focus, call {@link #setFocusable(boolean)}. When in touch mode (see notes * below) views indicate whether they still would like focus via {@link #isFocusableInTouchMode} and * can change this via {@link #setFocusableInTouchMode(boolean)}. * *
Focus movement is based on an algorithm which finds the nearest neighbor in a given direction. * In rare cases, the default algorithm may not match the intended behavior of the developer. In * these situations, you can provide explicit overrides by using these XML attributes in the layout * file: * *
* nextFocusDown * nextFocusLeft * nextFocusRight * nextFocusUp *``` ```java /** * Initializes an {@link AccessibilityNodeInfo} with information about this view. The base * implementation sets: * *
Subclasses should override this method, call the super implementation, and set additional * attributes. * *
If an {@link AccessibilityDelegate} has been specified via calling {@link * #setAccessibilityDelegate(AccessibilityDelegate)} its {@link * AccessibilityDelegate#onInitializeAccessibilityNodeInfo(View, AccessibilityNodeInfo)} is * responsible for handling this call. * * @param info The instance to initialize. */ @CallSuper public void onInitializeAccessibilityNodeInfo(AccessibilityNodeInfo info) { if (mAccessibilityDelegate != null) { mAccessibilityDelegate.onInitializeAccessibilityNodeInfo(this, info); } else { onInitializeAccessibilityNodeInfoInternal(info); } } ``` `setFocusable` in Android View ```java /** * Set whether this view can receive the focus. * *
Setting this to false will also ensure that this view is not focusable in touch mode. * * @param focusable If true, this view can receive the focus. * @see #setFocusableInTouchMode(boolean) * @see #setFocusable(int) * @attr ref android.R.styleable#View_focusable */ public void setFocusable(boolean focusable) { setFocusable(focusable ? FOCUSABLE : NOT_FOCUSABLE); } /** * Sets whether this view can receive focus. * *
Setting this to {@link #FOCUSABLE_AUTO} tells the framework to determine focusability * automatically based on the view's interactivity. This is the default. * *
Setting this to NOT_FOCUSABLE will ensure that this view is also not focusable in touch * mode. * * @param focusable One of {@link #NOT_FOCUSABLE}, {@link #FOCUSABLE}, or {@link #FOCUSABLE_AUTO}. * @see #setFocusableInTouchMode(boolean) * @attr ref android.R.styleable#View_focusable */ public void setFocusable(@Focusable int focusable) { if ((focusable & (FOCUSABLE_AUTO | FOCUSABLE)) == 0) { setFlags(0, FOCUSABLE_IN_TOUCH_MODE); } setFlags(focusable, FOCUSABLE_MASK); } ```
I attach a DRAFT pr to keep track of my progress. Next week I will investigate:
ViewManagers on AndroidThanks a lot. :🙏 ☮
Description
When using a screen reader this component does not announce that it is disabled.
This issue is covered in-depth by parent issue #30840, please check there for more details.
React Native version:
v0.63