Skip to content

fix(accessibility): Derive accessibilityTraits from role and state (#57515) #57516

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

Open
cgoldsby wants to merge 1 commit into
base: main
Choose a base branch
from
+41 −33
Open

fix(accessibility): Derive accessibilityTraits from role and state (#57515) #57516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions packages/react-native/React/Fabric/Mounting/ComponentViews/View/RCTViewComponentView.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,18 +472,6 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &
RCTUIAccessibilityTraitsFromAccessibilityTraits(newViewProps.accessibilityTraits);
}

// `accessibilityState`
if (oldViewProps.accessibilityState != newViewProps.accessibilityState) {
self.accessibilityTraits &= ~(UIAccessibilityTraitNotEnabled | UIAccessibilityTraitSelected);
const auto accessibilityState = newViewProps.accessibilityState.value_or(AccessibilityState{});
if (accessibilityState.selected) {
self.accessibilityTraits |= UIAccessibilityTraitSelected;
}
if (accessibilityState.disabled) {
self.accessibilityTraits |= UIAccessibilityTraitNotEnabled;
}
}

// `accessibilityIgnoresInvertColors`
if (oldViewProps.accessibilityIgnoresInvertColors != newViewProps.accessibilityIgnoresInvertColors) {
self.accessibilityIgnoresInvertColors = newViewProps.accessibilityIgnoresInvertColors;
Expand Down
62 changes: 41 additions & 21 deletions packages/react-native/ReactCommon/react/renderer/components/view/AccessibilityProps.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,31 @@

namespace facebook::react {

// Derive accessibilityTraits from the resolved role and accessibilityState
// members. This makes accessibilityTraits the single source of truth.
// See: github.com/facebook/react-native/issues/57515
static AccessibilityTraits deriveAccessibilityTraits(
Role role,
const std::string& accessibilityRole,
const std::optional<AccessibilityState>& accessibilityState) {
AccessibilityTraits traits = AccessibilityTraits::None;
if (role != Role::None) {
fromString(toString(role), traits);
} else if (!accessibilityRole.empty()) {
fromString(accessibilityRole, traits);
}

const auto state = accessibilityState.value_or(AccessibilityState{});
if (state.selected) {
traits = traits | AccessibilityTraits::Selected;
}
if (state.disabled) {
traits = traits | AccessibilityTraits::NotEnabled;
}

return traits;
}

AccessibilityProps::AccessibilityProps(
const PropsParserContext& context,
const AccessibilityProps& sourceProps,
Expand Down Expand Up @@ -146,19 +171,9 @@ AccessibilityProps::AccessibilityProps(
ImportantForAccessibility::Auto)),
testId(
convertRawProp(context, rawProps, "testID", sourceProps.testId, "")) {
// It is a (severe!) perf deoptimization to request props out-of-order.
// Thus, since we need to request the same prop twice here
// (accessibilityRole) we "must" do them subsequently here to prevent
// a regression. It is reasonable to ask if the `at` function can be improved;
// it probably can, but this is a fairly rare edge-case that (1) is easy-ish
// to work around here, and (2) would require very careful work to address
// this case and not regress the more common cases.
auto* accessibilityRoleValue = rawProps.at("accessibilityRole");
auto* roleValue = rawProps.at("role");

auto* precedentRoleValue =
roleValue != nullptr ? roleValue : accessibilityRoleValue;

if (accessibilityRoleValue == nullptr ||
!accessibilityRoleValue->hasValue()) {
accessibilityRole = sourceProps.accessibilityRole;
Expand All @@ -172,11 +187,8 @@ AccessibilityProps::AccessibilityProps(
fromRawValue(context, *roleValue, role);
}

if (precedentRoleValue == nullptr || !precedentRoleValue->hasValue()) {
accessibilityTraits = sourceProps.accessibilityTraits;
} else {
fromRawValue(context, *precedentRoleValue, accessibilityTraits);
}
accessibilityTraits =
deriveAccessibilityTraits(role, accessibilityRole, accessibilityState);
}

void AccessibilityProps::setProp(
Expand All @@ -188,7 +200,12 @@ void AccessibilityProps::setProp(

switch (hash) {
RAW_SET_PROP_SWITCH_CASE_BASIC(accessible);
RAW_SET_PROP_SWITCH_CASE_BASIC(accessibilityState);
case CONSTEXPR_RAW_PROPS_KEY_HASH("accessibilityState"): {
fromRawValue(context, value, accessibilityState, defaults.accessibilityState);
accessibilityTraits =
deriveAccessibilityTraits(role, accessibilityRole, accessibilityState);
return;
}
RAW_SET_PROP_SWITCH_CASE_BASIC(accessibilityLabel);
RAW_SET_PROP_SWITCH_CASE(
accessibilityOrder, "experimental_accessibilityOrder");
Expand All @@ -209,18 +226,21 @@ void AccessibilityProps::setProp(
RAW_SET_PROP_SWITCH_CASE_BASIC(onAccessibilityEscape);
RAW_SET_PROP_SWITCH_CASE_BASIC(onAccessibilityAction);
RAW_SET_PROP_SWITCH_CASE_BASIC(importantForAccessibility);
RAW_SET_PROP_SWITCH_CASE_BASIC(role);
case CONSTEXPR_RAW_PROPS_KEY_HASH("role"): {
fromRawValue(context, value, role, defaults.role);
accessibilityTraits =
deriveAccessibilityTraits(role, accessibilityRole, accessibilityState);
return;
}
RAW_SET_PROP_SWITCH_CASE(testId, "testID");
case CONSTEXPR_RAW_PROPS_KEY_HASH("accessibilityRole"): {
AccessibilityTraits traits = AccessibilityTraits::None;
std::string roleString;
if (value.hasValue()) {
fromRawValue(context, value, traits);
fromRawValue(context, value, roleString);
}

accessibilityTraits = traits;
accessibilityRole = roleString;
accessibilityTraits =
deriveAccessibilityTraits(role, accessibilityRole, accessibilityState);
return;
}
}
Expand Down