VoiceOver accessibility issues in RN .62 #603
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -270,6 +270,9 @@ void UIGraphicsEndImageContext(void); | |
| // semantically equivalent types | ||
| // | ||
|
|
||
| // UIAccessibility.h/NSAccessibility.h | ||
| @compatibility_alias UIAccessibilityCustomAction NSAccessibilityCustomAction; | ||
|
Member
There was a problem hiding this comment. @tom-un When we move
Collaborator
Author
There was a problem hiding this comment. Yes, they should all be normalized to a consistent convention if/when we contribute upstream. I'm partial to things like |
||
|
|
||
| // UIColor.h/NSColor.h | ||
| #define RCTUIColor NSColor | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,9 @@ | |
| #import "RCTUtils.h" | ||
| #import "UIView+React.h" | ||
| #import "RCTI18nUtil.h" | ||
| #if TARGET_OS_OSX // [TODO(macOS ISS#2323203) | ||
tom-un marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| #import "RCTTextView.h" | ||
| #endif // ]TODO(macOS ISS#2323203) | ||
|
|
||
| #if !TARGET_OS_OSX // TODO(macOS ISS#2323203) | ||
| UIAccessibilityTraits const SwitchAccessibilityTrait = 0x20000000000001; | ||
|
|
@@ -92,7 +95,18 @@ - (RCTPlatformView *)react_findClipView // TODO(macOS ISS#2323203) | |
| { | ||
| NSMutableString *str = [NSMutableString stringWithString:@""]; | ||
| for (RCTUIView *subview in view.subviews) { // TODO(macOS ISS#3536887) | ||
| #if !TARGET_OS_OSX // TODO(macOS ISS#2323203) | ||
| NSString *label = subview.accessibilityLabel; | ||
| #else // [TODO(macOS ISS#2323203) | ||
| NSString *label; | ||
| if ([subview isKindOfClass:[RCTTextView class]]) { | ||
| // on macOS VoiceOver a text element will always have its accessibilityValue read, but will only read it's accessibilityLabel if it's value is set. | ||
| // the macOS RCTTextView accessibilityValue will return its accessibilityLabel if set otherwise return its text. | ||
tom-un marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| label = subview.accessibilityValue; | ||
| } else { | ||
| label = subview.accessibilityLabel; | ||
| } | ||
| #endif // ]TODO(macOS ISS#2323203) | ||
| if (!label) { | ||
| label = RCTRecursiveAccessibilityLabel(subview); | ||
| } | ||
|
|
@@ -188,10 +202,14 @@ - (NSString *)accessibilityLabel | |
| if (label) { | ||
| return label; | ||
| } | ||
| #if TARGET_OS_OSX // [TODO(macOS ISS#2323203) | ||
| // calling super.accessibilityLabel above on macOS causes the return value of this accessor to be ignored by VoiceOver. | ||
| // Calling the super's setAccessibilityLabel with nil ensures that the return value of this accessor is used by VoiceOver. | ||
|
Member
There was a problem hiding this comment. Does the
Collaborator
Author
There was a problem hiding this comment. It probably is fine to nil it on iOS too -- but AX is such a touchy area with lots of undocumented behavior in VoiceOver that I'd rather not risk any deltas to the iOS path.
Member
There was a problem hiding this comment. I’m going to unhide this one, just in case I ever want to come back to this while unforking. |
||
| [super setAccessibilityLabel:nil]; | ||
| #endif // ]TODO(macOS ISS#2323203) | ||
| return RCTRecursiveAccessibilityLabel(self); | ||
| } | ||
|
|
||
| - (NSArray <UIAccessibilityCustomAction *> *)accessibilityCustomActions | ||
| { | ||
| if (!self.accessibilityActions.count) { | ||
|
|
@@ -233,11 +251,10 @@ - (BOOL)didActivateAccessibilityCustomAction:(UIAccessibilityCustomAction *)acti | |
| } | ||
| return YES; | ||
| } | ||
|
|
||
| #if !TARGET_OS_OSX // TODO(macOS ISS#2323203) | ||
| - (NSString *)accessibilityValue | ||
| { | ||
| if ((self.accessibilityTraits & SwitchAccessibilityTrait) == SwitchAccessibilityTrait) { | ||
| for (NSString *state in self.accessibilityState) { | ||
| id val = self.accessibilityState[state]; | ||
|
|
@@ -248,17 +265,7 @@ - (NSString *)accessibilityValue | |
| return [val boolValue] ? @"1" : @"0"; | ||
| } | ||
| } | ||
| } | ||
| NSMutableArray *valueComponents = [NSMutableArray new]; | ||
| static NSDictionary<NSString *, NSString *> *roleDescriptions = nil; | ||
| static dispatch_once_t onceToken1; | ||
|
|
@@ -294,7 +301,7 @@ - (NSString *)accessibilityValue | |
| @"mixed": @"mixed", | ||
| }; | ||
| }); | ||
| NSString *roleDescription = self.accessibilityRoleInternal ? roleDescriptions[self.accessibilityRoleInternal]: nil; // TODO(OSS Candidate ISS#2710739): renamed prop so it doesn't conflict with -[NSAccessibility accessibilityRole]. | ||
| if (roleDescription) { | ||
| [valueComponents addObject:roleDescription]; | ||
| } | ||
|
|
@@ -341,6 +348,147 @@ - (NSString *)accessibilityValue | |
| } | ||
| return nil; | ||
| } | ||
| #else // [TODO(macOS ISS#2323203) | ||
| - (id)accessibilityValue { | ||
| id accessibilityValue = nil; | ||
| NSAccessibilityRole role = [self accessibilityRole]; | ||
| if (role == NSAccessibilityCheckBoxRole || | ||
| role == NSAccessibilityRadioButtonRole || | ||
| role == NSAccessibilityDisclosureTriangleRole) { | ||
| for (NSString *state in [self accessibilityState]) { | ||
| id val = [self accessibilityState][state]; | ||
| if (val != nil) { | ||
|
There was a problem hiding this comment. This and the if statement below can be combined into one |
||
| if ([state isEqualToString:@"checked"]) { | ||
| if ([val isKindOfClass:[NSNumber class]]) { | ||
| accessibilityValue = @([val boolValue]); | ||
| } else if ([val isKindOfClass:[NSString class]] && [val isEqualToString:@"mixed"]) { | ||
| accessibilityValue = @(2); // undocumented by Apple: @(2) is the accessibilityValue an NSButton has when its state is NSMixedState (-1) and causes VoiceOver to announced "mixed". | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } else if ([self accessibilityRole] == NSAccessibilityStaticTextRole) { | ||
| // On macOS if the role is static text, VoiceOver will only read the text returned by accessibilityValue. | ||
| // So return accessibilityLabel which has the logic to return either either the ivar or a computed value of all the children's text. | ||
| // If the accessibilityValueInternal "text" is present, it will override this value below. | ||
| accessibilityValue = [self accessibilityLabel]; | ||
| } | ||
|
|
||
| // handle accessibilityValue | ||
|
|
||
| id accessibilityValueInternal = [self accessibilityValueInternal]; | ||
| if (accessibilityValueInternal != nil) { | ||
| id now = accessibilityValueInternal[@"now"]; | ||
| id text = accessibilityValueInternal[@"text"]; | ||
| if (text != nil && [text isKindOfClass:[NSString class]]) { | ||
| accessibilityValue = text; | ||
| } else if (now != nil && [now isKindOfClass:[NSNumber class]]) { | ||
| accessibilityValue = now; | ||
| } | ||
| } | ||
|
|
||
| return accessibilityValue; | ||
| } | ||
|
|
||
| - (BOOL)isAccessibilitySelectorAllowed:(SEL)selector { | ||
| BOOL isAllowed = NO; | ||
| if (selector == @selector(isAccessibilityEnabled)) { | ||
| if (self.accessibilityState != nil) { | ||
| id disabled = self.accessibilityState[@"disabled"]; | ||
| if ([disabled isKindOfClass:[NSNumber class]]) { | ||
| isAllowed = YES; | ||
| } | ||
| } | ||
| } else if (selector == @selector(isAccessibilitySelected)) { | ||
| if (self.accessibilityState != nil) { | ||
| id selected = self.accessibilityState[@"selected"]; | ||
| if ([selected isKindOfClass:[NSNumber class]]) { | ||
| isAllowed = YES; | ||
| } | ||
| } | ||
| } else if (selector == @selector(isAccessibilityExpanded)) { | ||
| if (self.accessibilityState != nil) { | ||
| id expanded = self.accessibilityState[@"expanded"]; | ||
| if ([expanded isKindOfClass:[NSNumber class]]) { | ||
| isAllowed = YES; | ||
| } | ||
| } | ||
| } else if (selector == @selector(accessibilityPerformPress)) { | ||
| if (_onAccessibilityTap != nil || | ||
| (_onAccessibilityAction != nil && accessibilityActionsNameMap[@"activate"]) || | ||
| _onClick != nil) { | ||
| isAllowed = YES; | ||
| } | ||
| } else if (selector == @selector(accessibilityPerformIncrement)) { | ||
| if (_onAccessibilityAction != nil && accessibilityActionsNameMap[@"increment"]) { | ||
| isAllowed = YES; | ||
| } | ||
| } else if (selector == @selector(accessibilityPerformDecrement)) { | ||
| if (_onAccessibilityAction != nil && accessibilityActionsNameMap[@"decrement"]) { | ||
| isAllowed = YES; | ||
| } | ||
| } else { | ||
| isAllowed = YES; | ||
| } | ||
| return isAllowed; | ||
| } | ||
|
|
||
| - (BOOL)isAccessibilityEnabled { | ||
| BOOL isAccessibilityEnabled = YES; | ||
| if (self.accessibilityState != nil) { | ||
| id disabled = self.accessibilityState[@"disabled"]; | ||
| if ([disabled isKindOfClass:[NSNumber class]]) { | ||
| isAccessibilityEnabled = [disabled boolValue] ? NO : YES; | ||
| } | ||
| } | ||
| return isAccessibilityEnabled; | ||
| } | ||
|
|
||
| - (BOOL)isAccessibilitySelected { | ||
| BOOL isAccessibilitySelected = NO; | ||
| if (self.accessibilityState != nil) { | ||
| id selected = self.accessibilityState[@"selected"]; | ||
| if ([selected isKindOfClass:[NSNumber class]]) { | ||
| isAccessibilitySelected = [selected boolValue]; | ||
| } | ||
| } | ||
| return isAccessibilitySelected; | ||
| } | ||
|
|
||
| - (BOOL)isAccessibilityExpanded { | ||
| BOOL isAccessibilityExpanded = NO; | ||
| if (self.accessibilityState != nil) { | ||
| id expanded = self.accessibilityState[@"expanded"]; | ||
| if ([expanded isKindOfClass:[NSNumber class]]) { | ||
| isAccessibilityExpanded = [expanded boolValue]; | ||
| } | ||
| } | ||
| return isAccessibilityExpanded; | ||
| } | ||
|
|
||
| - (id)accessibilityMinValue { | ||
| id accessibilityMinValue = nil; | ||
| if (self.accessibilityValueInternal != nil) { | ||
| id min = self.accessibilityValueInternal[@"min"]; | ||
| if ([min isKindOfClass:[NSNumber class]]) { | ||
| accessibilityMinValue = min; | ||
| } | ||
| } | ||
| return accessibilityMinValue; | ||
| } | ||
|
Member
There was a problem hiding this comment. Just for my understanding, what else can these values be that they can be ignored?
Collaborator
Author
There was a problem hiding this comment. The accessibilityMinValue and accessibilityMaxValue are used for sliders and steppers roles. When the appropriate role, like slider, is set, VoiceOver will automatically compute percentages and announce them. i.e. if accessibilityMinValue=0, accessibilityMaxValue=100, and accessibilityValue = 50, voiceover will say "50%" |
||
|
|
||
| - (id)accessibilityMaxValue { | ||
| id accessibilityMaxValue = nil; | ||
| if (self.accessibilityValueInternal != nil) { | ||
| id max = self.accessibilityValueInternal[@"max"]; | ||
| if ([max isKindOfClass:[NSNumber class]]) { | ||
| accessibilityMaxValue = max; | ||
| } | ||
| } | ||
| return accessibilityMaxValue; | ||
| } | ||
|
|
||
| #endif // ]TODO(macOS ISS#2323203) | ||
|
|
||
| - (void)setPointerEvents:(RCTPointerEvents)pointerEvents | ||
| { | ||
|
|
@@ -445,17 +593,28 @@ - (BOOL)performAccessibilityAction:(NSString *) name | |
| return NO; | ||
| } | ||
|
|
||
| #if !TARGET_OS_OSX // TODO(macOS ISS#2323203) | ||
| - (BOOL)accessibilityActivate | ||
| #else // [TODO(macOS ISS#2323203) | ||
| - (BOOL)accessibilityPerformPress | ||
| #endif // ]TODO(macOS ISS#2323203) | ||
| { | ||
| #if TARGET_OS_OSX // [TODO(macOS ISS#2323203) | ||
| if ([self isAccessibilityEnabled] == NO) { | ||
|
There was a problem hiding this comment. This method is plagued by early returns. Would be awesome if we could fix those up instead of adding one more to it :) |
||
| return NO; | ||
| } | ||
| #endif // ]TODO(macOS ISS#2323203) | ||
| if ([self performAccessibilityAction:@"activate"]) { | ||
| return YES; | ||
| } else if (_onAccessibilityTap) { | ||
| _onAccessibilityTap(nil); | ||
| return YES; | ||
| #if TARGET_OS_OSX // [TODO(macOS ISS#2323203) | ||
| } else if (_onClick != nil) { | ||
| // macOS is not simulating a click if there is no onAccessibilityAction like it does on iOS, so we simulate it here. | ||
| _onClick(nil); | ||
| return YES; | ||
| #endif // ]TODO(macOS ISS#2323203) | ||
|
Member
There was a problem hiding this comment. Do you imagine that at some point we might be able to upstream things like this and just have it not do anything on platforms that don’t have the concept of clicks?
Collaborator
Author
There was a problem hiding this comment. This is a weird one. On iOS returning NO from
Member
There was a problem hiding this comment. Good to know, thanks for the update! (I’ll actually unresolve this thread, as it’s just good general info for posterity.)
Collaborator
Author
There was a problem hiding this comment. BTW, I think I found how mac VoiceOver can simulate a click on VO activate: if the accessibilityActivationPoint prop is set https://developer.apple.com/documentation/appkit/nsaccessibility/1535149-accessibilityactivationpoint?language=objc. Might be worth further investigation in the future, but don't want to add risk to this change by introducing another property. |
||
| } else { | ||
| return NO; | ||
| } | ||
|
|
@@ -487,15 +646,29 @@ - (BOOL)accessibilityPerformEscape | |
| } | ||
| } | ||
|
|
||
| #if !TARGET_OS_OSX // TODO(macOS ISS#2323203) | ||
| - (void)accessibilityIncrement | ||
| { | ||
| [self performAccessibilityAction:@"increment"]; | ||
| } | ||
| #else // [TODO(macOS ISS#2323203) | ||
| - (BOOL)accessibilityPerformIncrement | ||
| { | ||
| return [self performAccessibilityAction:@"increment"]; | ||
| } | ||
| #endif // ]TODO(macOS ISS#2323203) | ||
|
|
||
| #if !TARGET_OS_OSX // TODO(macOS ISS#2323203) | ||
| - (void)accessibilityDecrement | ||
| { | ||
| [self performAccessibilityAction:@"decrement"]; | ||
| } | ||
| #else // [TODO(macOS ISS#2323203) | ||
| - (BOOL)accessibilityPerformDecrement | ||
| { | ||
| return [self performAccessibilityAction:@"decrement"]; | ||
| } | ||
| #endif // ]TODO(macOS ISS#2323203) | ||
|
|
||
| - (NSString *)description | ||
| { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.