Skip to content

Commit 87e0cb4

Browse files
Fix View accessibility payloads that Props 2.0 sends wrong or drops (#59079)
Summary: With Props 2.0 on Android (`enableExclusivePropsUpdateAndroid` and the related flags), `BaseViewManager` receives the View payload from `getDiffProps`. Some accessibility props come out wrong: - Changing one member of `accessibilityState`, such as `busy`: only that member is sent, and Java treats the map as the whole state, so a selected, disabled view becomes unselected and enabled. - A removed `accessibilityRole` or `role`: not cleared. - An explicit `role="none"`: not sent on mount, so the view falls back to `accessibilityRole`. - A removed `accessibilityValue`: sent as `{}`, so the old value stays in the content description. This PR sends `accessibilityState` in full, clears a removed role or value, and makes `role` optional so that unset and `"none"` differ. Nothing changes on Android with the flags off. iOS shares the props constructor, so a removed role is now also cleared in its C++ props. I didn't test iOS. ## Changelog: [ANDROID] [FIXED] - Fix accessibilityState, a removed accessibilityRole, role or accessibilityValue, and role="none" with Props 2.0 Pull Request resolved: #59079 Test Plan: - New `ViewPropsDiffTest` cases, using the payload helper from #59077. Each test fails on main. Built with the NDK and run on an Android emulator (there is no open-source gtest target). - RNTester on an Android emulator with the Props 2.0 flags forced on: the Playground screen below, checked with `uiautomator dump`. Setting `busy` keeps the view selected and disabled. Removing the props: | | Flags off | Main, Props 2.0 | This PR, Props 2.0 | |---|---|---|---| | `role` removed | `android.view.ViewGroup` | `android.widget.Button` | `android.view.ViewGroup` | | `accessibilityRole` removed | `android.view.ViewGroup` | `android.widget.Button` | `android.view.ViewGroup` | | `accessibilityValue` removed | content description cleared | `"value target, 50%"` | content description cleared | | `role="none"` + `accessibilityRole="button"` | `android.view.View` | `android.widget.Button` | `android.view.View` | <details><summary><code>RNTesterPlayground.js</code>: paste into <code>packages/rn-tester/js/examples/Playground/RNTesterPlayground.js</code> and open Playground.</summary> ```js /** * Copyright (c) Meta Platforms, Inc. and affiliates. * * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * * flow strict-local * format */ import type {RNTesterModuleExample} from '../../types/RNTesterTypes'; import * as React from 'react'; import {useState} from 'react'; import {Button, StyleSheet, Text, View} from 'react-native'; function A11yState() { const [busy, setBusy] = useState(false); return ( <View> <Button testID="toggle" title={busy ? 'Set busy: false' : 'Set busy: true'} onPress={() => setBusy(v => !v)} /> <Text testID="state"> accessibilityState: selected true, disabled true, busy {String(busy)} </Text> <View style={styles.row}> <View accessible={true} accessibilityLabel="a11y target" testID="a11yTarget" accessibilityState={{selected: true, disabled: true, busy}} style={styles.card} /> </View> </View> ); } function A11y() { const [on, setOn] = useState(true); return ( <View> <Button testID="toggle" title={on ? 'Remove role, accessibilityRole, value' : 'Set them'} onPress={() => setOn(v => !v)} /> <Text testID="state"> {on ? 'role/accessibilityRole button, value 50%' : 'all unset'} </Text> <View style={styles.row}> <View accessible={true} accessibilityLabel="role target" role={on ? 'button' : undefined} style={styles.chip} /> <View accessible={true} accessibilityLabel="accessibilityRole target" accessibilityRole={on ? 'button' : undefined} style={styles.chip} /> <View accessible={true} accessibilityLabel="value target" accessibilityValue={on ? {text: '50%'} : undefined} style={styles.chip} /> <View accessible={true} accessibilityLabel="none target" role="none" accessibilityRole="button" style={styles.chip} /> </View> </View> ); } const demos = { a11yState: A11yState, a11y: A11y, }; function Playground() { const [demo, setDemo] = useState<$Keys<typeof demos>>('a11yState'); const Demo = demos[demo]; return ( <View style={styles.container}> <View style={styles.tabs}> {Object.keys(demos).map(name => ( <Button key={name} testID={'tab-' + name} title={name} onPress={() => setDemo(name)} /> ))} </View> <Demo key={demo} /> </View> ); } const styles = StyleSheet.create({ card: { alignItems: 'center', backgroundColor: '#1e88e5', height: 140, justifyContent: 'center', width: 200, }, chip: {backgroundColor: '#90caf9', height: 48, margin: 8, width: 48}, container: {padding: 10}, row: { alignItems: 'center', flexDirection: 'row', flexWrap: 'wrap', justifyContent: 'center', marginTop: 24, }, tabs: {flexDirection: 'row', flexWrap: 'wrap', gap: 6, marginBottom: 12}, }); export default { title: 'Playground', name: 'playground', description: 'Test out new features and ideas.', render: (): React.Node => <Playground />, } as RNTesterModuleExample; ``` </details> Differential Revision: D125438453 Pulled By: bartlomiejbloniarz
1 parent 281fc5a commit 87e0cb4

17 files changed

Lines changed: 111 additions & 75 deletions

‎packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ static BOOL RCTViewIsInteractiveAccessibilityElement(UIView *view, const ViewPro
6767

6868
// `role` wins over the legacy `accessibilityRole` when both are set, matching
6969
// how the traits themselves are resolved.
70-
if (props.role != Role::None) {
70+
if (props.role.value_or(Role::None) != Role::None) {
7171
static const std::unordered_set<Role> interactiveRoles{
7272
Role::Button,
7373
Role::Checkbox,
@@ -82,7 +82,7 @@ static BOOL RCTViewIsInteractiveAccessibilityElement(UIView *view, const ViewPro
8282
Role::Switch,
8383
Role::Tab,
8484
Role::Treeitem};
85-
return interactiveRoles.contains(props.role);
85+
return interactiveRoles.contains(*props.role);
8686
}
8787

8888
static const std::unordered_set<std::string_view> interactiveAccessibilityRoles{
@@ -1678,8 +1678,9 @@ - (NSString *)accessibilityValue
16781678
}
16791679

16801680
NSMutableArray *valueComponents = [NSMutableArray new];
1681-
NSString *roleString = (props.role != Role::None) ? [NSString stringWithUTF8String:toString(props.role).c_str()]
1682-
: [NSString stringWithUTF8String:props.accessibilityRole.c_str()];
1681+
NSString *roleString = (props.role.value_or(Role::None) != Role::None)
1682+
? [NSString stringWithUTF8String:toString(*props.role).c_str()]
1683+
: [NSString stringWithUTF8String:props.accessibilityRole.c_str()];
16831684

16841685
// In iOS, checkbox and radio buttons aren't recognized as traits. However,
16851686
// because our apps use checkbox and radio buttons often, we should announce

‎packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.cpp‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ namespace facebook::react {
1818
// members. This makes accessibilityTraits the single source of truth.
1919
// See: github.com/facebook/react-native/issues/57515
2020
static AccessibilityTraits deriveAccessibilityTraits(
21-
Role role,
21+
std::optional<Role> role,
2222
const std::string& accessibilityRole,
2323
const std::optional<AccessibilityState>& accessibilityState) {
2424
AccessibilityTraits traits = AccessibilityTraits::None;
25-
if (role != Role::None) {
26-
fromString(toString(role), traits);
25+
if (role.value_or(Role::None) != Role::None) {
26+
fromString(toString(*role), traits);
2727
} else if (!accessibilityRole.empty()) {
2828
fromString(accessibilityRole, traits);
2929
}
@@ -179,16 +179,15 @@ AccessibilityProps::AccessibilityProps(
179179
auto* accessibilityRoleValue = rawProps.at("accessibilityRole");
180180
auto* roleValue = rawProps.at("role");
181181

182-
if (accessibilityRoleValue == nullptr ||
183-
!accessibilityRoleValue->hasValue()) {
182+
if (accessibilityRoleValue == nullptr) {
184183
accessibilityRole = sourceProps.accessibilityRole;
185-
} else {
184+
} else if (accessibilityRoleValue->hasValue()) {
186185
fromRawValue(context, *accessibilityRoleValue, accessibilityRole);
187186
}
188187

189-
if (roleValue == nullptr || !roleValue->hasValue()) {
188+
if (roleValue == nullptr) {
190189
role = sourceProps.role;
191-
} else {
190+
} else if (roleValue->hasValue()) {
192191
fromRawValue(context, *roleValue, role);
193192
}
194193

‎packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ class AccessibilityProps {
5656
bool onAccessibilityEscape{};
5757
bool onAccessibilityAction{};
5858
ImportantForAccessibility importantForAccessibility{ImportantForAccessibility::Auto};
59-
Role role{Role::None};
59+
std::optional<Role> role{};
6060
std::string testId;
6161

6262
#pragma mark - DebugStringConvertible

‎packages/react-native/ReactCommon/react/renderer/components/view/platform/android/react/renderer/components/view/HostPlatformViewProps.cpp‎

Lines changed: 37 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -395,58 +395,39 @@ inline static void updateNativeDrawableProp(
395395
result[propName] = nativeDrawableResult;
396396
}
397397

398-
inline static void updateAccessibilityStateProp(
399-
folly::dynamic& result,
400-
const std::optional<AccessibilityState>& newState,
401-
const std::optional<AccessibilityState>& oldState) {
402-
folly::dynamic resultState = folly::dynamic::object();
403-
404-
if (!newState.has_value() && oldState.has_value()) {
405-
result["accessibilityState"] = resultState;
406-
return;
407-
}
398+
static folly::dynamic toDynamic(const AccessibilityState& state) {
399+
folly::dynamic result = folly::dynamic::object();
400+
result["disabled"] = state.disabled;
408401

409-
if (!oldState.has_value() || newState->disabled != oldState->disabled) {
410-
resultState["disabled"] = newState->disabled;
402+
// Omitting the key when `selected` is unset is what tells the platform the
403+
// component is not selectable. BaseViewManager#setViewState falls back to
404+
// `setSelected(false)` for an absent key, so the rendered result is
405+
// unchanged from when this was a plain `bool`.
406+
if (state.selected.has_value()) {
407+
result["selected"] = state.selected.value();
411408
}
412409

413-
if (!oldState.has_value() || newState->selected != oldState->selected) {
414-
// Omitting the key when `selected` is unset is what tells the platform the
415-
// component is not selectable. BaseViewManager#setViewState falls back to
416-
// `setSelected(false)` for an absent key, so the rendered result is
417-
// unchanged from when this was a plain `bool`.
418-
if (newState->selected.has_value()) {
419-
resultState["selected"] = newState->selected.value();
420-
}
421-
}
410+
result["busy"] = state.busy;
422411

423-
if (!oldState.has_value() || newState->busy != oldState->busy) {
424-
resultState["busy"] = newState->busy;
412+
if (state.expanded.has_value()) {
413+
result["expanded"] = state.expanded.value();
425414
}
426415

427-
if (!oldState.has_value() || newState->expanded != oldState->expanded) {
428-
if (newState->expanded.has_value()) {
429-
resultState["expanded"] = newState->expanded.value();
430-
}
416+
switch (state.checked) {
417+
case AccessibilityState::Unchecked:
418+
result["checked"] = false;
419+
break;
420+
case AccessibilityState::Checked:
421+
result["checked"] = true;
422+
break;
423+
case AccessibilityState::Mixed:
424+
result["checked"] = "mixed";
425+
break;
426+
case AccessibilityState::None:
427+
result["checked"] = "none";
428+
break;
431429
}
432-
433-
if (!oldState.has_value() || newState->checked != oldState->checked) {
434-
switch (newState->checked) {
435-
case AccessibilityState::Unchecked:
436-
resultState["checked"] = false;
437-
break;
438-
case AccessibilityState::Checked:
439-
resultState["checked"] = true;
440-
break;
441-
case AccessibilityState::Mixed:
442-
resultState["checked"] = "mixed";
443-
break;
444-
case AccessibilityState::None:
445-
resultState["checked"] = "none";
446-
break;
447-
}
448-
}
449-
result["accessibilityState"] = resultState;
430+
return result;
450431
}
451432

452433
static folly::dynamic toDynamic(
@@ -540,7 +521,8 @@ folly::dynamic HostPlatformViewProps::getDiffProps(
540521
}
541522

542523
if (role != oldProps->role) {
543-
result["role"] = toString(role);
524+
result["role"] =
525+
role.has_value() ? toString(*role) : folly::dynamic(nullptr);
544526
}
545527

546528
if (opacity != oldProps->opacity) {
@@ -907,8 +889,9 @@ folly::dynamic HostPlatformViewProps::getDiffProps(
907889
// Accessibility
908890

909891
if (accessibilityState != oldProps->accessibilityState) {
910-
updateAccessibilityStateProp(
911-
result, accessibilityState, oldProps->accessibilityState);
892+
result["accessibilityState"] = accessibilityState.has_value()
893+
? toDynamic(*accessibilityState)
894+
: folly::dynamic::object();
912895
}
913896

914897
if (accessibilityLabel != oldProps->accessibilityLabel) {
@@ -941,7 +924,9 @@ folly::dynamic HostPlatformViewProps::getDiffProps(
941924
}
942925

943926
if (accessibilityRole != oldProps->accessibilityRole) {
944-
result["accessibilityRole"] = accessibilityRole;
927+
result["accessibilityRole"] = accessibilityRole.empty()
928+
? folly::dynamic(nullptr)
929+
: folly::dynamic(accessibilityRole);
945930
}
946931

947932
if (accessibilityLanguage != oldProps->accessibilityLanguage) {
@@ -962,7 +947,9 @@ folly::dynamic HostPlatformViewProps::getDiffProps(
962947
if (accessibilityValue.text.has_value()) {
963948
accessibilityValueObject["text"] = accessibilityValue.text.value();
964949
}
965-
result["accessibilityValue"] = accessibilityValueObject;
950+
result["accessibilityValue"] = accessibilityValueObject.empty()
951+
? folly::dynamic(nullptr)
952+
: accessibilityValueObject;
966953
}
967954

968955
if (accessibilityActions != oldProps->accessibilityActions) {

‎packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewPropsDiffTest.cpp‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,55 @@ TEST(ViewPropsDiffTest, gradientColorHintEmitsNullColor) {
103103
"position", nullptr)))))}});
104104
}
105105

106+
// BaseViewManager.setViewState replaces the whole state: an absent `selected`
107+
// deselects the view and an absent `disabled` re-enables it. Props 2.0 sends an
108+
// unset `checked` as "none", which Java ignores.
109+
TEST(ViewPropsDiffTest, emitsTheWholeAccessibilityStateWhenOneMemberChanges) {
110+
auto state = [](bool busy) {
111+
return dynamic::object("selected", true)("disabled", true)("busy", busy);
112+
};
113+
auto sentState = [&](bool busy) {
114+
return dynamic::object(
115+
"accessibilityState", state(busy)("checked", "none"));
116+
};
117+
expectPayloads({
118+
{dynamic::object("accessibilityState", state(false)), sentState(false)},
119+
{dynamic::object("accessibilityState", state(true)), sentState(true)},
120+
});
121+
}
122+
123+
// React sends a removed prop as null on top of the previous props.
124+
// BaseViewManager clears the role only for null, and
125+
// AccessibilityRole.fromValue("") throws.
126+
TEST(ViewPropsDiffTest, removingAccessibilityRoleEmitsNull) {
127+
expectPayloads({
128+
{dynamic::object("accessibilityRole", "button")},
129+
{dynamic::object("accessibilityRole", nullptr)},
130+
});
131+
}
132+
133+
// BaseViewManager falls back to accessibilityRole only for a null role.
134+
TEST(ViewPropsDiffTest, removingRoleEmitsNull) {
135+
expectPayloads({
136+
{dynamic::object("role", "button")},
137+
{dynamic::object("role", nullptr)},
138+
});
139+
}
140+
141+
// An explicit role="none" overrides accessibilityRole.
142+
TEST(ViewPropsDiffTest, emitsAnExplicitRoleNone) {
143+
expectPayloads(
144+
{{dynamic::object("role", "none")("accessibilityRole", "button")}});
145+
}
146+
147+
// BaseViewManager clears the value and the content description only for null.
148+
TEST(ViewPropsDiffTest, removingAccessibilityValueEmitsNull) {
149+
expectPayloads({
150+
{dynamic::object("accessibilityValue", dynamic::object("text", "50%"))},
151+
{dynamic::object("accessibilityValue", nullptr)},
152+
});
153+
}
154+
106155
} // namespace facebook::react
107156

108157
#endif

‎scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -700,8 +700,8 @@ class facebook::react::AccessibilityProps {
700700
public facebook::react::AccessibilityTraits accessibilityTraits;
701701
public facebook::react::AccessibilityValue accessibilityValue;
702702
public facebook::react::ImportantForAccessibility importantForAccessibility;
703-
public facebook::react::Role role;
704703
public std::optional<facebook::react::AccessibilityState> accessibilityState;
704+
public std::optional<facebook::react::Role> role;
705705
public std::string accessibilityHint;
706706
public std::string accessibilityLabel;
707707
public std::string accessibilityLanguage;

‎scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1010,8 +1010,8 @@ class facebook::react::AccessibilityProps {
10101010
public facebook::react::AccessibilityTraits accessibilityTraits;
10111011
public facebook::react::AccessibilityValue accessibilityValue;
10121012
public facebook::react::ImportantForAccessibility importantForAccessibility;
1013-
public facebook::react::Role role;
10141013
public std::optional<facebook::react::AccessibilityState> accessibilityState;
1014+
public std::optional<facebook::react::Role> role;
10151015
public std::string accessibilityHint;
10161016
public std::string accessibilityLabel;
10171017
public std::string accessibilityLanguage;

‎scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -700,8 +700,8 @@ class facebook::react::AccessibilityProps {
700700
public facebook::react::AccessibilityTraits accessibilityTraits;
701701
public facebook::react::AccessibilityValue accessibilityValue;
702702
public facebook::react::ImportantForAccessibility importantForAccessibility;
703-
public facebook::react::Role role;
704703
public std::optional<facebook::react::AccessibilityState> accessibilityState;
704+
public std::optional<facebook::react::Role> role;
705705
public std::string accessibilityHint;
706706
public std::string accessibilityLabel;
707707
public std::string accessibilityLanguage;

‎scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -700,8 +700,8 @@ class facebook::react::AccessibilityProps {
700700
public facebook::react::AccessibilityTraits accessibilityTraits;
701701
public facebook::react::AccessibilityValue accessibilityValue;
702702
public facebook::react::ImportantForAccessibility importantForAccessibility;
703-
public facebook::react::Role role;
704703
public std::optional<facebook::react::AccessibilityState> accessibilityState;
704+
public std::optional<facebook::react::Role> role;
705705
public std::string accessibilityHint;
706706
public std::string accessibilityLabel;
707707
public std::string accessibilityLanguage;

‎scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3430,8 +3430,8 @@ class facebook::react::AccessibilityProps {
34303430
public facebook::react::AccessibilityTraits accessibilityTraits;
34313431
public facebook::react::AccessibilityValue accessibilityValue;
34323432
public facebook::react::ImportantForAccessibility importantForAccessibility;
3433-
public facebook::react::Role role;
34343433
public std::optional<facebook::react::AccessibilityState> accessibilityState;
3434+
public std::optional<facebook::react::Role> role;
34353435
public std::string accessibilityHint;
34363436
public std::string accessibilityLabel;
34373437
public std::string accessibilityLanguage;

0 commit comments

Comments
 (0)