Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Adding pager, scrollview, viewgroup, webview, drawer roles #34477

Closed
wants to merge 35 commits into from
Closed

Adding pager, scrollview, viewgroup, webview, drawer roles #34477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
343279e
adding roles and traits to java/js files
fabOnReact Aug 18, 2022
b0c8d91
update class names
fabOnReact Aug 19, 2022
82b309a
adding scrollview and horizontal scrollview
fabOnReact Aug 19, 2022
1d246f1
Merge branch 'main' into roles-and-traits
fabOnReact Aug 19, 2022
a48d4f0
adding new roles examples
fabOnReact Aug 19, 2022
c8084c5
Merge branch 'main' into roles-and-traits
fabOnReact Aug 19, 2022
c9e5d7a
Merge branch 'main' into roles-and-traits
fabOnReact Aug 22, 2022
a38d81c
Add custom roles not available in compositor.json (#30839, commit 2a1…
fabOnReact Aug 23, 2022
569cd1c
adding missing roles to AccessibilityRole type
fabOnReact Aug 23, 2022
d2bddb2
minor changes to Example
fabOnReact Aug 23, 2022
d267860
Fix Talkback annoucements with Drawer
fabOnReact Aug 23, 2022
578e29c
using setTag instead of instance variable for accessibilityRole
fabOnReact Aug 24, 2022
83960b7
ScrollView AccessibilityRole
fabOnReact Aug 24, 2022
d7bcd28
datepicker and timepicker are deprecated
fabOnReact Aug 24, 2022
805cee4
running google-java-format
fabOnReact Aug 24, 2022
a4ae90c
removing not relevant roles
fabOnReact Aug 24, 2022
2d73468
minor changes
fabOnReact Aug 24, 2022
42667ff
fix circle ci errors
fabOnReact Aug 24, 2022
58c2fc4
Merge branch 'main' into roles-and-traits
fabOnReact Aug 25, 2022
bd5f11e
Merge branch 'main' into roles-and-traits
fabOnReact Aug 30, 2022
1b69235
Add UIAccessibilityTraitNone (link) to missing roles on iOS (#34477)
fabOnReact Aug 31, 2022
d460d09
seekcontrol already exist on Android/iOS as adjustable
fabOnReact Aug 31, 2022
45235d3
remove checkedtextview
fabOnReact Aug 31, 2022
f694992
remove duplicate role checkbox
fabOnReact Aug 31, 2022
0f07b2e
adding AppCompatEditText for edittext and fix runtime
fabOnReact Aug 31, 2022
fc60bd6
TextInput does not need an EditText role on Android. Adding the EditT…
fabOnReact Aug 31, 2022
1955558
minor changes
fabOnReact Aug 31, 2022
2504fbd
minor changes
fabOnReact Aug 31, 2022
a87b298
Merge branch 'main' into roles-and-traits
fabOnReact Sep 2, 2022
d5690b6
code review from Brett
fabOnReact Sep 2, 2022
9764c48
adding drawerlayout example
fabOnReact Sep 2, 2022
ab893d9
drawer layout
fabOnReact Sep 2, 2022
9801320
fix circleci errors flowandroid
fabOnReact Sep 2, 2022
9f89049
Merge branch 'main' into roles-and-traits
fabOnReact Sep 5, 2022
e1c2878
update example
fabOnReact Sep 5, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Libraries/Components/DrawerAndroid/DrawerLayoutAndroid.android.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import * as React from 'react';
import StatusBar from '../StatusBar/StatusBar';
import StyleSheet from '../../StyleSheet/StyleSheet';
import View from '../View/View';
import type {AccessibilityRole} from '../../Components/View/ViewAccessibility';

import dismissKeyboard from '../../Utilities/dismissKeyboard';
import nullthrows from 'nullthrows';
Expand All @@ -38,6 +39,8 @@ type DrawerSlideEvent = $ReadOnly<{|
|}>;

type Props = $ReadOnly<{|
accessibilityRole?: ?AccessibilityRole,

/**
* Determines whether the keyboard gets dismissed in response to a drag.
* - 'none' (the default), drags do not dismiss the keyboard.
Expand Down
11 changes: 10 additions & 1 deletion Libraries/Components/View/ViewAccessibility.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type {SyntheticEvent} from '../../Types/CoreEventTypes';
export type AccessibilityRole =
| 'none'
| 'button'
| 'dropdownlist'
| 'togglebutton'
| 'link'
| 'search'
Expand Down Expand Up @@ -44,7 +45,15 @@ export type AccessibilityRole =
| 'timer'
| 'list'
| 'toolbar'
| 'grid';
| 'grid'
| 'pager'
| 'scrollview'
| 'horizontalscrollview'
| 'viewgroup'
| 'webview'
| 'drawerlayout'
| 'slidingdrawer'
| 'iconmenu';
Comment on lines +48 to +56
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the intent was for these to map roughly to ARIA roles? https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles

Are there existing cases where we have custom roles for things like this? Some of these seem like they don't quite fit with the theme of specifying a semantic role (e.g. I cannot imagine there ever being a "drawerlayout" or "webview" ARIA role).

cc @necolas

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NickGerleman , @necolas

This PR isn't related to aria, so there may be two different issues in this space. This was a PR that resolved the issue of not all native Android roles being supported in RN, which means components built in RN have different behavior than if those components were built natively.

Aria has no concepts that map to many of these (and "webview" specifically wouldn't even make sense for a web-spec like aria), but RN components need these roles if we want them to work like their native implementations.

Whether we want to support native-specific functionality like this comes down to a pretty core question about the RN framework itself, which is whether it's trying to be a framework that bridges the gap between iOS, Android, and other native platforms, or if it's trying to bring web-specific paradigms (such as aria) to those native platforms.

If it's meant to bring web paradigms to those platforms, we'll basically need to rethink the entire set of accessibility APIs and start from scratch, as they are currently almost entirely focused on the native-specific approaches that accessibility takes. The native approach to accessibility is often considerably different from the approach taken on the web, mainly due to the differences in the way accessibility services themselves are built on native platforms.

Things like custom actions, hints, magic tap, certain roles/traits, etc. that are native-specific would need to be left out. And things like tab-index, landmarks, and other web-specific features would have to be implemented from scratch.

The end result of this is that apps built with React Native would "feel" more like the web to users of assistive technology, and not feel like native apps. We've already heard this feedback about pretty basic roles like "radio group", which confused some users as they didn't expect it to be in a native application (@alextait1 has more details on that feedback).

Overall I think this is a decision the RN team needs to make, and provide clarity on, as right now the accessibility API is in a pretty weird state, with some web-specific features attempted to be included in (and not working the way they do on the web), some web-specific features left out (when they could work on native), some native-specific features left out (that have no web equivalents), and some native-specifics features includes with slightly different APIs, when there is a similar enough web equivalent to map to.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The role prop is typed separately and equivalent values are then mapped to accessibilityRole values, so at a glance this looks ok as it's adding values to the latter.

Copy link
Contributor

@NickGerleman NickGerleman Sep 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, if we have separated out role and accessibilityRole, then it seems like this could be a clean separation, where accessibilityRole can have roles that might be platform specific, divergent from ARIA. As long as any mapping still works correctly, I think this change should make sense then.

It does seem like we have a bit of a messy experience for developers trying to reason about what a role does, if it works, what platforms it works on, etc. If platforms do really have specific needs not represented in a common spec (and it seems like they still do), I am all in favor of letting RN expose them.

It might be worth leaving documentation as to what roles are platform specific. In code/on the website (@fabriziobertoglio1987 I'm guessing this change will be reflected in a documentation update?).

The TypeScript typings adjacent in ViewAccessibility.d.ts (which we should update, either as part of this change or a followup), have a split between platforms for individual properties, but not role. Maybe that can be split, so that users get some hint? Though platform-specific typechecking is a more general open problem (FYI @acoates-ms who has cared about that in the past).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also FYI to @lunaleaps that it looks like role and the ARIA types are missing from the TS declarations. We should update those over, if we are not immediately moving that file to TS generation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@NickGerleman this notifications did not go in my inbox for some reason. Sorry. The documentation is available at https://github.com/facebook/react-native-website/pull/3287/files


// the info associated with an accessibility action
export type AccessibilityActionInfo = $ReadOnly<{
Expand Down
10 changes: 10 additions & 0 deletions React/Views/RCTViewManager.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ @implementation RCTConvert (UIAccessibilityTraits)
(@{
@"none" : @(UIAccessibilityTraitNone),
@"button" : @(UIAccessibilityTraitButton),
@"dropdownlist" : @(UIAccessibilityTraitNone),
@"togglebutton" : @(UIAccessibilityTraitButton),
@"link" : @(UIAccessibilityTraitLink),
@"header" : @(UIAccessibilityTraitHeader),
Expand Down Expand Up @@ -63,6 +64,15 @@ @implementation RCTConvert (UIAccessibilityTraits)
@"tablist" : @(UIAccessibilityTraitNone),
@"timer" : @(UIAccessibilityTraitNone),
@"toolbar" : @(UIAccessibilityTraitNone),
@"grid" : @(UIAccessibilityTraitNone),
@"pager" : @(UIAccessibilityTraitNone),
@"scrollview" : @(UIAccessibilityTraitNone),
@"horizontalscrollview" : @(UIAccessibilityTraitNone),
@"viewgroup" : @(UIAccessibilityTraitNone),
@"webview" : @(UIAccessibilityTraitNone),
@"drawerlayout" : @(UIAccessibilityTraitNone),
@"slidingdrawer" : @(UIAccessibilityTraitNone),
@"iconmenu" : @(UIAccessibilityTraitNone),
@"list" : @(UIAccessibilityTraitNone),
}),
UIAccessibilityTraitNone,
Expand Down
29 changes: 28 additions & 1 deletion ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactAccessibilityDelegate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ private void scheduleAccessibilityEventSender(View host) {
public enum AccessibilityRole {
NONE,
BUTTON,
DROPDOWNLIST,
TOGGLEBUTTON,
LINK,
SEARCH,
Expand Down Expand Up @@ -123,20 +124,30 @@ public enum AccessibilityRole {
TIMER,
LIST,
GRID,
PAGER,
SCROLLVIEW,
HORIZONTALSCROLLVIEW,
VIEWGROUP,
WEBVIEW,
DRAWERLAYOUT,
SLIDINGDRAWER,
ICONMENU,
TOOLBAR;

public static String getValue(AccessibilityRole role) {
switch (role) {
case BUTTON:
return "android.widget.Button";
case DROPDOWNLIST:
return "android.widget.Spinner";
Copy link
Contributor Author

@fabOnReact fabOnReact Aug 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I searched the source code of all these libraries and I found one library using the native Component android.widget.Spinner.

The library is react-native-picker/picker. I would keep the role.

case TOGGLEBUTTON:
return "android.widget.ToggleButton";
case SEARCH:
return "android.widget.EditText";
case IMAGE:
return "android.widget.ImageView";
case IMAGEBUTTON:
return "android.widget.ImageButon";
return "android.widget.ImageButton";
case KEYBOARDKEY:
return "android.inputmethodservice.Keyboard$Key";
case TEXT:
Expand All @@ -155,6 +166,22 @@ public static String getValue(AccessibilityRole role) {
return "android.widget.AbsListView";
case GRID:
return "android.widget.GridView";
case SCROLLVIEW:
return "android.widget.ScrollView";
case HORIZONTALSCROLLVIEW:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think SCROLLVIEW and HORIZONTALSCROLLVIEW should be returning their own classes.

See the talkback source here:
https://github.com/google/talkback/blob/f5d564fdc915a74d8cde4868608f307de9ccf957/utils/src/main/java/com/google/android/accessibility/utils/Role.java#L341-L347

It's only if they have collectionInfo that they fall back to the Grid or List role, and that can happen internally to Talkback, so we don't need to worry about that logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blavalla Thanks. I made the changes and also added an example of ScrollView to the Accessibility Example. I reviewed the react-native implementation of these accessibility functionalities, comparing them with the original Android implementation in their respective widgets. I tested the TalkBack functionalities and I did not detect any issue.

return "android.widget.HorizontalScrollView";
case PAGER:
return "androidx.viewpager.widget.ViewPager";
case DRAWERLAYOUT:
return "androidx.drawerlayout.widget.DrawerLayout";
case SLIDINGDRAWER:
return "android.widget.SlidingDrawer";
case ICONMENU:
return "com.android.internal.view.menu.IconMenuView";
case VIEWGROUP:
return "android.view.ViewGroup";
case WEBVIEW:
return "android.webkit.WebView";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/react-native-webview/react-native-webview/search?q=android.webkit.WebView
https://github.com/google/talkback/blob/771de7cdbf55b6adb4ca4c64c27a52584f2337cc/compositor/src/main/res/raw/compositor.json#L1104
https://github.com/google/talkback/blob/771de7cdbf55b6adb4ca4c64c27a52584f2337cc/compositor/src/main/res/values/strings.xml#L244

case NONE:
case LINK:
case SUMMARY:
Expand Down
1 change: 1 addition & 0 deletions ReactAndroid/src/main/java/com/facebook/react/views/drawer/BUCK
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,6 @@ rn_android_library(
react_native_target("java/com/facebook/react/uimanager/annotations:annotations"),
react_native_target("java/com/facebook/react/views/scroll:scroll"),
react_native_root_target(":generated_components_java-FBReactNativeComponentSpec"),
react_native_target("res:uimanager"),
],
)
30 changes: 30 additions & 0 deletions ReactAndroid/src/main/java/com/facebook/react/views/drawer/ReactDrawerLayout.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,16 @@
import android.view.Gravity;
import android.view.MotionEvent;
import android.view.View;
import android.view.accessibility.AccessibilityEvent;
import androidx.core.view.AccessibilityDelegateCompat;
import androidx.core.view.ViewCompat;
import androidx.core.view.accessibility.AccessibilityNodeInfoCompat;
import androidx.drawerlayout.widget.DrawerLayout;
import com.facebook.common.logging.FLog;
import com.facebook.react.R;
import com.facebook.react.bridge.ReactContext;
import com.facebook.react.common.ReactConstants;
import com.facebook.react.uimanager.ReactAccessibilityDelegate.AccessibilityRole;
import com.facebook.react.uimanager.events.NativeGestureUtil;

/**
Expand All @@ -29,6 +35,30 @@

public ReactDrawerLayout(ReactContext reactContext) {
super(reactContext);
ViewCompat.setAccessibilityDelegate(
this,
new AccessibilityDelegateCompat() {
@Override
public void onInitializeAccessibilityNodeInfo(
View host, AccessibilityNodeInfoCompat info) {
super.onInitializeAccessibilityNodeInfo(host, info);
final AccessibilityRole accessibilityRole =
(AccessibilityRole) host.getTag(R.id.accessibility_role);
if (accessibilityRole != null) {
info.setClassName(AccessibilityRole.getValue(accessibilityRole));
}
}

@Override
public void onInitializeAccessibilityEvent(View host, AccessibilityEvent event) {
super.onInitializeAccessibilityEvent(host, event);
final AccessibilityRole accessibilityRole =
(AccessibilityRole) host.getTag(R.id.accessibility_role);
if (accessibilityRole != null) {
event.setClassName(AccessibilityRole.getValue(accessibilityRole));
}
}
});
}

@Override
Expand Down
Loading