Repository navigation
fix(macos): Fabric accessibility roles and state, and TextInput read-only and Tab-stop fixes #3136
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
68644c7
533d408
deb87cf
10be049
676c01f
a40ec92
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -398,18 +398,42 @@ - (void)_updatePlaceholder | |
|
|
||
| - (BOOL)isEditable | ||
| { | ||
| #if !TARGET_OS_OSX // [macOS] | ||
| return self.isEnabled; | ||
| #else // [macOS | ||
| return [super isEditable]; | ||
| #endif // macOS] | ||
| } | ||
|
|
||
| - (void)setEditable:(BOOL)editable | ||
| { | ||
| #if TARGET_OS_OSX // [macOS | ||
| // on macos the super must be called otherwise its NSTextFieldCell editable property doesn't get set. | ||
| #if !TARGET_OS_OSX // [macOS] | ||
| self.enabled = editable; | ||
| #else // [macOS | ||
| // On macOS `editable={false}` makes the field read-only, not disabled: it stays enabled, focusable and selectable. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a behavior difference from iOS? Would prefer to match iOS / Web / React Native unless desktop or macOS call for divergent behavior |
||
| // `accessibilityState.disabled` disables it instead. The super call is required for NSTextFieldCell's editable state. | ||
| [super setEditable:editable]; | ||
| self.selectable = YES; | ||
| #endif // macOS] | ||
| self.enabled = editable; | ||
| } | ||
|
|
||
| #if TARGET_OS_OSX // [macOS | ||
| - (BOOL)acceptsFirstResponder | ||
| { | ||
| // NSTextField refuses first responder when it is not editable, which would make read-only fields unreachable with | ||
| // Tab. Accept it while the field is enabled and selectable; disabled fields are skipped. | ||
| return self.isEnabled && self.isSelectable; | ||
| } | ||
|
|
||
| - (void)setAccessibilityState:(NSDictionary<NSString *, id> *)accessibilityState | ||
| { | ||
| // Paper routes `accessibilityState` to the backed text field through -[RCTBaseTextInputView reactAccessibilityElement]. | ||
| [super setAccessibilityState:accessibilityState]; | ||
| id disabled = accessibilityState[@"disabled"]; | ||
| self.enabled = !([disabled isKindOfClass:[NSNumber class]] && [disabled boolValue]); | ||
| } | ||
| #endif // macOS] | ||
|
|
||
| #if !TARGET_OS_OSX // [macOS] | ||
| - (void)setSecureTextEntry:(BOOL)secureTextEntry | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -255,6 +255,21 @@ - (void)reactUpdateResponderOffsetForScrollView:(RCTScrollViewComponentView *)sc | |
|
|
||
| #pragma mark - RCTViewComponentView overrides | ||
|
|
||
| #if TARGET_OS_OSX // [macOS | ||
| // The backed text input owns focus, editing and the accessibility element; this view only hosts it. Keep it out of the | ||
| // key view loop and the responder chain so each TextInput is a single Tab stop. TextInput defaults `focusable` to true, | ||
| // which would otherwise make -[RCTViewComponentView acceptsFirstResponder] return YES. `focus()` targets the backed view. | ||
| - (BOOL)canBecomeKeyView | ||
| { | ||
| return NO; | ||
| } | ||
|
|
||
| - (BOOL)acceptsFirstResponder | ||
| { | ||
| return NO; | ||
| } | ||
| #endif // macOS] | ||
|
Comment on lines
+259
to
+271
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can simplify this comment a bit |
||
|
|
||
| - (NSObject *)accessibilityElement | ||
| { | ||
| return _backedTextInputView; | ||
|
|
@@ -305,6 +320,13 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared & | |
| _backedTextInputView.editable = newTextInputProps.traits.editable; | ||
| } | ||
|
|
||
| #if TARGET_OS_OSX // [macOS | ||
| // `accessibilityState.disabled` disables the backed text input; `editable={false}` only makes it read-only. | ||
| if (newTextInputProps.accessibilityState != oldTextInputProps.accessibilityState) { | ||
| _backedTextInputView.enabled = !newTextInputProps.accessibilityState.value_or(AccessibilityState{}).disabled; | ||
| } | ||
| #endif // macOS] | ||
|
|
||
| #if !TARGET_OS_OSX // [macOS] | ||
| if (newTextInputProps.multiline && | ||
| newTextInputProps.traits.dataDetectorTypes != oldTextInputProps.traits.dataDetectorTypes) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,6 +36,7 @@ | |
| #endif | ||
|
|
||
| #if TARGET_OS_OSX // [macOS | ||
| #import <React/RCTConvert.h> | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We probably shouldn't import the paper |
||
| #import <React/RCTCursor.h> | ||
| #import <React/RCTViewKeyboardEvent.h> | ||
| #import <React/RCTUtils.h> | ||
|
|
@@ -45,6 +46,60 @@ | |
|
|
||
| const CGFloat BACKGROUND_COLOR_ZPOSITION = -1024.0f; | ||
|
|
||
| #if TARGET_OS_OSX // [macOS | ||
| // The role an accessibility element reports when React Native does not override it. NSControl subclasses, such as the | ||
| // NSTextField behind a single-line TextInput, report their role through their cell; the control's own | ||
| // `accessibilityRole` getter returns NSAccessibilityUnknownRole until a role is set explicitly. | ||
| static NSAccessibilityRole RCTDefaultAccessibilityRoleForElement(NSView *element) | ||
| { | ||
| if ([element isKindOfClass:[NSControl class]]) { | ||
| NSAccessibilityRole cellRole = ((NSControl *)element).cell.accessibilityRole; | ||
| if (cellRole != nil) { | ||
| return cellRole; | ||
| } | ||
| } | ||
| return NSAccessibilityUnknownRole; | ||
| } | ||
|
|
||
| // Matches Paper (`-[RCTViewManager updateAccessibilityRole:withDefaultView:]`): `role` takes precedence over | ||
| // `accessibilityRole`, and the element's default role is used when neither is set. | ||
| static NSAccessibilityRole RCTAccessibilityRoleFromViewProps(const ViewProps &props, NSView *element) | ||
| { | ||
| if (props.role != Role::None) { | ||
| return [RCTConvert accessibilityRoleFromTraits:RCTNSStringFromString(toString(props.role)) useAriaMappings:YES]; | ||
| } | ||
| if (!props.accessibilityRole.empty()) { | ||
| return [RCTConvert accessibilityRoleFromTraits:RCTNSStringFromString(props.accessibilityRole) useAriaMappings:NO]; | ||
| } | ||
| NSAccessibilityRole traitsRole = | ||
| RCTAccessibilityRoleFromTraits(RCTUIAccessibilityTraitsFromAccessibilityTraits(props.accessibilityTraits)); | ||
| if (![traitsRole isEqualToString:NSAccessibilityUnknownRole]) { | ||
| return traitsRole; | ||
| } | ||
| return RCTDefaultAccessibilityRoleForElement(element); | ||
| } | ||
|
|
||
| // Setting an NSAccessibility state property makes AppKit advertise that attribute, so like Paper | ||
| // (`-[RCTView isAccessibilitySelectorAllowed:]`) only expose a state when JS provides it. `disabled` and `selected` are | ||
| // plain bools in `AccessibilityState`, so they count as provided whenever `accessibilityState` is set. | ||
| static BOOL RCTViewPropsProvideAccessibilityEnabled(const ViewProps &props) | ||
| { | ||
| const auto traits = RCTUIAccessibilityTraitsFromAccessibilityTraits(props.accessibilityTraits); | ||
| return props.accessibilityState.has_value() || (traits & RCTUIAccessibilityTraitNotEnabled) != 0; | ||
| } | ||
|
|
||
| static BOOL RCTViewPropsProvideAccessibilitySelected(const ViewProps &props) | ||
| { | ||
| const auto traits = RCTUIAccessibilityTraitsFromAccessibilityTraits(props.accessibilityTraits); | ||
| return props.accessibilityState.has_value() || (traits & RCTUIAccessibilityTraitSelected) != 0; | ||
| } | ||
|
|
||
| static BOOL RCTViewPropsProvideAccessibilityExpanded(const ViewProps &props) | ||
| { | ||
| return props.accessibilityState.has_value() && props.accessibilityState->expanded.has_value(); | ||
| } | ||
|
Comment on lines
+81
to
+100
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this file the best place for these functions? There isn't some fabric specific accessibility handlers file? |
||
| #endif // macOS] | ||
|
|
||
| @implementation RCTViewComponentView { | ||
| RCTPlatformColor *_backgroundColor; // [macOS] | ||
| CALayer *_backgroundColorLayer; | ||
|
|
@@ -487,20 +542,45 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared & | |
| } | ||
| } | ||
|
|
||
| #if !TARGET_OS_OSX // [macOS] | ||
| // `accessibilityTraits` | ||
| if (oldViewProps.accessibilityTraits != newViewProps.accessibilityTraits) { | ||
| #if !TARGET_OS_OSX // [macOS] | ||
| self.accessibilityElement.accessibilityTraits = | ||
| RCTUIAccessibilityTraitsFromAccessibilityTraits(newViewProps.accessibilityTraits); | ||
| } | ||
| #else // [macOS | ||
| // On macOS, accessibilityElement returns self (NSView*) which doesn't have accessibilityTraits. | ||
| // Set role directly on self based on traits. | ||
| RCTUIAccessibilityTraits traits = RCTUIAccessibilityTraitsFromAccessibilityTraits(newViewProps.accessibilityTraits); | ||
| self.accessibilityRole = RCTAccessibilityRoleFromTraits(traits); | ||
| self.accessibilityEnabled = (traits & RCTUIAccessibilityTraitNotEnabled) == 0; | ||
| #endif // macOS] | ||
| // `role`, `accessibilityRole` and `accessibilityTraits` | ||
| // NSAccessibility has no traits, so all three resolve to a single role on the accessibility element. | ||
| if (oldViewProps.role != newViewProps.role || oldViewProps.accessibilityRole != newViewProps.accessibilityRole || | ||
| oldViewProps.accessibilityTraits != newViewProps.accessibilityTraits) { | ||
| self.accessibilityElement.accessibilityRole = | ||
| RCTAccessibilityRoleFromViewProps(newViewProps, self.accessibilityElement); | ||
| } | ||
|
|
||
| // `accessibilityState` | ||
| // `disabled` and `selected` can also come from the NotEnabled and Selected traits. Only update a state that is, or | ||
| // was, provided so that elements without it do not start advertising it. | ||
| if (oldViewProps.accessibilityState != newViewProps.accessibilityState || | ||
| oldViewProps.accessibilityTraits != newViewProps.accessibilityTraits) { | ||
| const auto accessibilityState = newViewProps.accessibilityState.value_or(AccessibilityState{}); | ||
| const auto traits = RCTUIAccessibilityTraitsFromAccessibilityTraits(newViewProps.accessibilityTraits); | ||
| if (RCTViewPropsProvideAccessibilityEnabled(newViewProps) || | ||
| RCTViewPropsProvideAccessibilityEnabled(oldViewProps)) { | ||
| self.accessibilityElement.accessibilityEnabled = | ||
| !accessibilityState.disabled && (traits & RCTUIAccessibilityTraitNotEnabled) == 0; | ||
| } | ||
| if (RCTViewPropsProvideAccessibilitySelected(newViewProps) || | ||
| RCTViewPropsProvideAccessibilitySelected(oldViewProps)) { | ||
| self.accessibilityElement.accessibilitySelected = | ||
| accessibilityState.selected || (traits & RCTUIAccessibilityTraitSelected) != 0; | ||
| } | ||
| if (RCTViewPropsProvideAccessibilityExpanded(newViewProps) || | ||
| RCTViewPropsProvideAccessibilityExpanded(oldViewProps)) { | ||
| self.accessibilityElement.accessibilityExpanded = accessibilityState.expanded.value_or(false); | ||
| } | ||
| } | ||
| #endif // macOS] | ||
|
Comment on lines
+525
to
+545
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we simplify this diff? I'd like to follow the iOS handling more closely instead of diffing out the check of |
||
|
|
||
| #if !TARGET_OS_OSX // [macOS] | ||
| // `accessibilityState` | ||
| if (oldViewProps.accessibilityState != newViewProps.accessibilityState) { | ||
|
|
@@ -1685,6 +1765,23 @@ - (BOOL)isAccessibilityElement | |
| return [super isAccessibilityElement]; | ||
| } | ||
|
|
||
| #if TARGET_OS_OSX // [macOS | ||
| - (BOOL)isAccessibilitySelectorAllowed:(SEL)selector | ||
| { | ||
| const auto &props = static_cast<const ViewProps &>(*_props); | ||
| if (selector == @selector(isAccessibilityEnabled)) { | ||
| return RCTViewPropsProvideAccessibilityEnabled(props); | ||
| } | ||
| if (selector == @selector(isAccessibilitySelected)) { | ||
| return RCTViewPropsProvideAccessibilitySelected(props); | ||
| } | ||
| if (selector == @selector(isAccessibilityExpanded)) { | ||
| return RCTViewPropsProvideAccessibilityExpanded(props); | ||
| } | ||
| return [super isAccessibilitySelectorAllowed:selector]; | ||
| } | ||
| #endif // macOS] | ||
|
|
||
| - (NSString *)accessibilityValue | ||
| { | ||
| const auto &props = static_cast<const ViewProps &>(*_props); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer no early return and to do the work in the body