From 0a57392dbcfaad4d83845401e83a1a6c1c2f1ca6 Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Thu, 23 May 2019 10:24:47 -0700 Subject: [PATCH 1/6] Revamp Accessibility API for more flexibility --- Schemas/configuration.json | 3 +- Source/ASDisplayNode+Beta.h | 10 +- Source/ASExperimentalFeatures.h | 2 + Source/ASExperimentalFeatures.mm | 4 +- Source/ASTextNode2.mm | 133 ++++++++++- Source/Details/_ASDisplayView.mm | 3 +- Source/Details/_ASDisplayViewAccessiblity.h | 7 + Source/Details/_ASDisplayViewAccessiblity.mm | 196 ++++++++++----- Source/Private/ASDisplayNode+UIViewBridge.mm | 6 - Source/Private/ASDisplayNodeInternal.h | 2 + Tests/ASConfigurationTests.mm | 8 +- Tests/ASTextNode2Tests.mm | 225 ++++++++++++++++-- examples/ASDKgram/Sample/PhotoCellNode.m | 2 + .../ASDKgram/Sample/PhotoFeedNodeController.m | 15 ++ examples/ASDKgram/Sample/PhotoModel.m | 31 ++- .../ASDKgram/Sample/TextureConfigDelegate.m | 2 +- 16 files changed, 552 insertions(+), 97 deletions(-) diff --git a/Schemas/configuration.json b/Schemas/configuration.json index 925720f6a..79d8c7145 100644 --- a/Schemas/configuration.json +++ b/Schemas/configuration.json @@ -26,7 +26,8 @@ "exp_text_drawing", "exp_oom_bg_dealloc_disable", "exp_transaction_operation_retain_cycle", - "exp_remove_textkit_initialising_lock" + "exp_text_node_2_a11y_container", + "exp_expose_text_links_a11y" ] } } diff --git a/Source/ASDisplayNode+Beta.h b/Source/ASDisplayNode+Beta.h index 4a8f4f451..cd2c78ba7 100644 --- a/Source/ASDisplayNode+Beta.h +++ b/Source/ASDisplayNode+Beta.h @@ -84,9 +84,13 @@ typedef struct { @property (readonly) ASDisplayNodePerformanceMeasurements performanceMeasurements; /** - * @abstract Whether this node acts as an accessibility container. If set to YES, then this node's accessibility label will represent - * an aggregation of all child nodes' accessibility labels. Nodes in this node's subtree that are also accessibility containers will - * not be included in this aggregation, and will be exposed as separate accessibility elements to UIKit. + * @abstract Whether this node acts as an accessibility container. If set to YES, then this node's + * accessibility label will represent an aggregation of all child nodes' accessibility labels and + * this node's accessibility custom actions will be represented will be the aggregation of all + * child nodes that have a accessibility trait of UIAccessibilityTraitLink, + * UIAccessibilityTraitKeyboardKey or UIAccessibilityTraitButton. + * Nodes in this node's subtree that are also accessibility containers will not be included in this + * aggregation, and will be exposed as separate accessibility elements to UIKit. */ @property BOOL isAccessibilityContainer; diff --git a/Source/ASExperimentalFeatures.h b/Source/ASExperimentalFeatures.h index ef91628b3..12c3a153d 100644 --- a/Source/ASExperimentalFeatures.h +++ b/Source/ASExperimentalFeatures.h @@ -32,6 +32,8 @@ typedef NS_OPTIONS(NSUInteger, ASExperimentalFeatures) { ASExperimentalTransactionOperationRetainCycle = 1 << 11, // exp_transaction_operation_retain_cycle ASExperimentalRemoveTextKitInitialisingLock = 1 << 12, // exp_remove_textkit_initialising_lock ASExperimentalDrawingGlobal = 1 << 13, // exp_drawing_global + ASExperimentalTextNode2A11YContainer = 1 << 14, // exp_text_node_2_a11y_container + ASExperimentalExposeTextLinksForA11Y = 1 << 15, // exp_expose_text_links_a11y ASExperimentalFeatureAll = 0xFFFFFFFF }; diff --git a/Source/ASExperimentalFeatures.mm b/Source/ASExperimentalFeatures.mm index 9594a8f60..b286b3cca 100644 --- a/Source/ASExperimentalFeatures.mm +++ b/Source/ASExperimentalFeatures.mm @@ -25,7 +25,9 @@ @"exp_oom_bg_dealloc_disable", @"exp_transaction_operation_retain_cycle", @"exp_remove_textkit_initialising_lock", - @"exp_drawing_global"])); + @"exp_drawing_global", + @"exp_text_node_2_a11y_container", + @"exp_expose_text_links_a11y"])); if (flags == ASExperimentalFeatureAll) { return allNames; } diff --git a/Source/ASTextNode2.mm b/Source/ASTextNode2.mm index a8f7f248a..f3e4112c9 100644 --- a/Source/ASTextNode2.mm +++ b/Source/ASTextNode2.mm @@ -30,6 +30,23 @@ #import #import +@interface ASTextNodeAccessiblityElement : UIAccessibilityElement +@property (assign) NSRange accessibilityRange; +@end + +@implementation ASTextNodeAccessiblityElement + +- (instancetype)initWithAccessibilityContainer:(id)container +{ + self = [super initWithAccessibilityContainer:container]; + if (self) { + _accessibilityRange = NSMakeRange(NSNotFound, 0); + } + return self; +} + +@end + @interface ASTextCacheValue : NSObject { @package AS::Mutex _m; @@ -215,7 +232,7 @@ - (instancetype)init // Accessibility self.isAccessibilityElement = YES; self.accessibilityTraits = self.defaultAccessibilityTraits; - + // Placeholders // Disabled by default in ASDisplayNode, but add a few options for those who toggle // on the special placeholder behavior of ASTextNode. @@ -313,6 +330,114 @@ - (UIAccessibilityTraits)defaultAccessibilityTraits return UIAccessibilityTraitStaticText; } +/// Uses the given layout, node and container node to calculate the accessibiliyty frame for the given ASTextNodeAccessiblityElement in screen coordinates. +static void ASUpdateAccessibilityFrame(ASTextNodeAccessiblityElement *accessibilityElement, ASTextLayout *layout, ASDisplayNode * _Nullable containerNode, ASDisplayNode *node) { + containerNode = containerNode ?: ASFirstNonLayerBackedSupernodeForNode(node); + CGRect textLayoutFrame = CGRectZero; + if (accessibilityElement.accessibilityRange.location == NSNotFound) { + // If no accessibilityRange was specified (as is done for the text element), just use the + // label's range and clampt to the visible range otherwise the returned rect would be invalid. + NSRange range = NSMakeRange(0, accessibilityElement.accessibilityLabel.length); + range = NSIntersectionRange(range, layout.visibleRange); + textLayoutFrame = [layout rectForRange:[ASTextRange rangeWithRange:range]]; + } else { + textLayoutFrame = [layout rectForRange:[ASTextRange rangeWithRange:accessibilityElement.accessibilityRange]]; + } + CGRect accessibilityFrame = [node convertRect:textLayoutFrame toNode:containerNode]; + accessibilityElement.accessibilityFrame = UIAccessibilityConvertFrameToScreenCoordinates(accessibilityFrame, containerNode.view); +} + +/// Walks up the node tree and searches for the first node that is not layer backed +static ASDisplayNode *ASFirstNonLayerBackedSupernodeForNode(ASDisplayNode *node) { + ASDisplayNode *containerNode = node; + while (containerNode.isLayerBacked) { + containerNode = containerNode.supernode; + } + return containerNode; +} + +/// Overwrite accessibilityElementAtIndex: so we can update the element's accessibilityFrame when it is requested. +- (id)accessibilityElementAtIndex:(NSInteger)index +{ + ASTextNodeAccessiblityElement *accessibilityElement = self.accessibilityElements[index]; + + ASTextLayout *layout = ASTextNodeCompatibleLayoutWithContainerAndText(_textContainer, _attributedText); + ASUpdateAccessibilityFrame(accessibilityElement, layout, nil, self); + return accessibilityElement; +} + +- (NSArray *)accessibilityElements +{ + if (_accessibilityElements != nil) { + return _accessibilityElements; + } + + NSAttributedString *attributedText = _attributedText; + NSInteger attributedTextLength = attributedText.length; + if (attributedTextLength == 0) { + _accessibilityElements = @[]; + return _accessibilityElements; + } + + NSMutableArray *accessibilityElements = [[NSMutableArray alloc] init]; + + // Searc the first node that is not layer backed + ASDisplayNode *containerNode = ASFirstNonLayerBackedSupernodeForNode(self); + UIAccessibilityTraits accessibilityTraits = self.accessibilityTraits; + ASTextLayout *layout = ASTextNodeCompatibleLayoutWithContainerAndText(_textContainer, attributedText); + + // Create an accessibility element to represent the label's text. It's not necessary to specify + // a accessibilityRange here, as the entirety of the text is being represented. + ASTextNodeAccessiblityElement *accessibilityElement = [[ASTextNodeAccessiblityElement alloc] initWithAccessibilityContainer:containerNode.view]; + accessibilityElement.accessibilityTraits = accessibilityTraits; + accessibilityElement.accessibilityLabel = self.accessibilityLabel; + ASUpdateAccessibilityFrame(accessibilityElement, layout, containerNode, self); + [accessibilityElements addObject:accessibilityElement]; + + if (ASActivateExperimentalFeature(ASExperimentalExposeTextLinksForA11Y)) { + // Collect all links as accessiblity items + for (NSString *linkAttributeName in _linkAttributeNames) { + [attributedText enumerateAttribute:linkAttributeName inRange:NSMakeRange(0, attributedTextLength) options:NSAttributedStringEnumerationLongestEffectiveRangeNotRequired usingBlock:^(id _Nullable value, NSRange range, BOOL * _Nonnull stop) { + if (value == nil) { + return; + } + + ASTextNodeAccessiblityElement *accessibilityElement = [[ASTextNodeAccessiblityElement alloc] initWithAccessibilityContainer:self]; + accessibilityElement.accessibilityTraits = UIAccessibilityTraitLink;; + accessibilityElement.accessibilityLabel = [attributedText.string substringWithRange:range]; + accessibilityElement.accessibilityRange = range; + ASUpdateAccessibilityFrame(accessibilityElement, layout, containerNode, self); + [accessibilityElements addObject:accessibilityElement]; + }]; + } + } + _accessibilityElements = accessibilityElements; + return _accessibilityElements; +} + +- (void)setIsAccessibilityElement:(BOOL)isAccessibilityElement + { + if (ASActivateExperimentalFeature(ASExperimentalTextNode2A11YContainer)) { + // Instead of relying on labels accessibility, We implement UIAccessibilityContainer and + // handle accessibility with ASTextNode2 + return; + } + + [super setIsAccessibilityElement:isAccessibilityElement]; + + } + +- (BOOL)isAccessibilityElement { + if (ASActivateExperimentalFeature(ASExperimentalTextNode2A11YContainer)) { + // Instead of relying on labels accessibility, We implement UIAccessibilityContainer and + // handle accessibility with ASTextNode2 + return NO; + } + + // Use whatever the default is + return [super isAccessibilityElement]; +} + #pragma mark - Layout and Sizing - (void)setTextContainerInset:(UIEdgeInsets)textContainerInset @@ -412,16 +537,16 @@ - (void)setAttributedText:(NSAttributedString *)attributedText style.ascender = [[self class] ascenderWithAttributedString:attributedText]; style.descender = [[attributedText attribute:NSFontAttributeName atIndex:attributedText.length - 1 effectiveRange:NULL] descender]; } - + // Tell the display node superclasses that the cached layout is incorrect now [self setNeedsLayout]; - + // Force display to create renderer with new size and redisplay with new string [self setNeedsDisplay]; // Accessiblity self.accessibilityLabel = self.defaultAccessibilityLabel; - + // We update the isAccessibilityElement setting if this node is not switching between strings. if (oldAttributedText.length == 0 || length == 0) { // We're an accessibility element by default if there is a string. diff --git a/Source/Details/_ASDisplayView.mm b/Source/Details/_ASDisplayView.mm index 9ba6176f4..7f28b054e 100644 --- a/Source/Details/_ASDisplayView.mm +++ b/Source/Details/_ASDisplayView.mm @@ -43,8 +43,7 @@ @implementation _ASDisplayView unsigned inIsFirstResponder:1; } _internalFlags; - NSArray *_accessibilityElements; - CGRect _lastAccessibilityElementsFrame; + _ASDisplayViewAccessibilityFlags _accessibilityFlags; } #pragma mark - Class diff --git a/Source/Details/_ASDisplayViewAccessiblity.h b/Source/Details/_ASDisplayViewAccessiblity.h index 192b24d2c..ea0e8c00a 100644 --- a/Source/Details/_ASDisplayViewAccessiblity.h +++ b/Source/Details/_ASDisplayViewAccessiblity.h @@ -14,3 +14,10 @@ // should still work as long as accessibility is enabled, this framework provides no guarantees on // their correctness. For details, see // https://developer.apple.com/documentation/objectivec/nsobject/1615147-accessibilityelements + +struct _ASDisplayViewAccessibilityFlags { + unsigned inAccessibilityElementCount:1; + unsigned inIndexOfAccessibilityElement:1; + unsigned inAccessibilityElementAtIndex:1; + unsigned inSetAccessibilityElements:1; +}; diff --git a/Source/Details/_ASDisplayViewAccessiblity.mm b/Source/Details/_ASDisplayViewAccessiblity.mm index 202c9c9e4..96decf099 100644 --- a/Source/Details/_ASDisplayViewAccessiblity.mm +++ b/Source/Details/_ASDisplayViewAccessiblity.mm @@ -9,6 +9,7 @@ #ifndef ASDK_ACCESSIBILITY_DISABLE +#import #import #import #import @@ -57,18 +58,18 @@ static void SortAccessibilityElements(NSMutableArray *elements) @interface ASAccessibilityElement : UIAccessibilityElement -@property (nonatomic) ASDisplayNode *node; @property (nonatomic) ASDisplayNode *containerNode; +@property (nonatomic) ASDisplayNode *node; -+ (ASAccessibilityElement *)accessibilityElementWithContainer:(UIView *)container node:(ASDisplayNode *)node containerNode:(ASDisplayNode *)containerNode; ++ (ASAccessibilityElement *)accessibilityElementWithContainerNode:(ASDisplayNode *)containerNode node:(ASDisplayNode *)node; @end @implementation ASAccessibilityElement -+ (ASAccessibilityElement *)accessibilityElementWithContainer:(UIView *)container node:(ASDisplayNode *)node containerNode:(ASDisplayNode *)containerNode ++ (ASAccessibilityElement *)accessibilityElementWithContainerNode:(ASDisplayNode *)containerNode node:(ASDisplayNode *)node { - ASAccessibilityElement *accessibilityElement = [[ASAccessibilityElement alloc] initWithAccessibilityContainer:container]; + ASAccessibilityElement *accessibilityElement = [[ASAccessibilityElement alloc] initWithAccessibilityContainer:containerNode.view]; accessibilityElement.node = node; accessibilityElement.containerNode = containerNode; accessibilityElement.accessibilityIdentifier = node.accessibilityIdentifier; @@ -96,9 +97,8 @@ - (CGRect)accessibilityFrame @interface ASAccessibilityCustomAction : UIAccessibilityCustomAction -@property (nonatomic) UIView *container; -@property (nonatomic) ASDisplayNode *node; @property (nonatomic) ASDisplayNode *containerNode; +@property (nonatomic) ASDisplayNode *node; @end @@ -106,34 +106,45 @@ @implementation ASAccessibilityCustomAction - (CGRect)accessibilityFrame { - CGRect accessibilityFrame = [self.containerNode convertRect:self.node.bounds fromNode:self.node]; - return UIAccessibilityConvertFrameToScreenCoordinates(accessibilityFrame, self.container); + ASDisplayNode *containerNode = self.containerNode; + ASDisplayNode *node = self.node; + ASDisplayNodeCAssertNotNil(containerNode, @"ASAccessibilityCustomAction needs a container node."); + ASDisplayNodeCAssertNotNil(node, @"ASAccessibilityCustomAction needs a node."); + CGRect accessibilityFrame = [containerNode convertRect:node.bounds fromNode:node]; + return UIAccessibilityConvertFrameToScreenCoordinates(accessibilityFrame, containerNode.view); } @end -/// Collect all subnodes for the given node by walking down the subnode tree and calculates the screen coordinates based on the containerNode and container -static void CollectUIAccessibilityElementsForNode(ASDisplayNode *node, ASDisplayNode *containerNode, id container, NSMutableArray *elements) +/// Collect all subnodes for the given node by walking down the subnode tree and calculates the screen coordinates based on the containerNode and container. This is necessary for layer backed nodes or rasterrized subtrees as no UIView instance for this node exists. +static void CollectAccessibilityElementsForLayerBackedOrRasterizedNode(ASDisplayNode *containerNode, ASDisplayNode *node, NSMutableArray *elements) { ASDisplayNodeCAssertNotNil(elements, @"Should pass in a NSMutableArray"); - + + // Iterate any node in the tree and either collect nodes that are accessibility elements + // or leaf nodes that are accessibility containers ASDisplayNodePerformBlockOnEveryNodeBFS(node, ^(ASDisplayNode * _Nonnull currentNode) { - // For every subnode that is layer backed or it's supernode has subtree rasterization enabled - // we have to create a UIAccessibilityElement as no view for this node exists - if (currentNode != containerNode && currentNode.isAccessibilityElement) { - UIAccessibilityElement *accessibilityElement = [ASAccessibilityElement accessibilityElementWithContainer:container node:currentNode containerNode:containerNode]; - [elements addObject:accessibilityElement]; + if (currentNode != containerNode) { + if (currentNode.isAccessibilityElement) { + // For every subnode that is layer backed or it's supernode has subtree rasterization enabled + // we have to create a UIAccessibilityElement as no view for this node exists + UIAccessibilityElement *accessibilityElement = [ASAccessibilityElement accessibilityElementWithContainerNode:containerNode node:currentNode]; + [elements addObject:accessibilityElement]; + } else if (currentNode.subnodes.count == 0) { + // In leaf nodes that are layer backed and acting as accessibility container we call + // through to the accessibilityElements method. + [elements addObjectsFromArray:currentNode.accessibilityElements]; + } } }); } -static void CollectAccessibilityElementsForContainer(ASDisplayNode *container, UIView *view, NSMutableArray *elements) { - UIAccessibilityElement *accessiblityElement = [ASAccessibilityElement accessibilityElementWithContainer:view node:container containerNode:container]; +/// Called from the usual accessibility elements collection function for a container to collect all subnodes accessibilityLabels +static void AggregateSublabelsOrCustomActionsForContainerNode(ASDisplayNode *container, NSMutableArray *elements) { + UIAccessibilityElement *accessiblityElement = [ASAccessibilityElement accessibilityElementWithContainerNode:container node:container]; NSMutableArray *labeledNodes = [[NSMutableArray alloc] init]; NSMutableArray *actions = [[NSMutableArray alloc] init]; - std::queue queue; - queue.push(container); // If the container does not have an accessibility label set, or if the label is meant for custom // actions only, then aggregate its subnodes' labels. Otherwise, treat the label as an overriden @@ -142,26 +153,31 @@ static void CollectAccessibilityElementsForContainer(ASDisplayNode *container, U (container.accessibilityLabel.length == 0) || (container.accessibilityTraits & InteractiveAccessibilityTraitsMask()); + std::queue queue; + queue.push(container); ASDisplayNode *node = nil; while (!queue.empty()) { node = queue.front(); queue.pop(); + // Only handle accessibility containers if (node != container && node.isAccessibilityContainer) { - CollectAccessibilityElementsForContainer(node, view, elements); + AggregateSublabelsOrCustomActionsForContainerNode(node, elements); continue; } + // Aggregate either custom actions for specific accessibility traits or the accessibility labels + // of the node if (node.accessibilityLabel.length > 0) { if (node.accessibilityTraits & InteractiveAccessibilityTraitsMask()) { ASAccessibilityCustomAction *action = [[ASAccessibilityCustomAction alloc] initWithName:node.accessibilityLabel target:node selector:@selector(performAccessibilityCustomAction:)]; - action.node = node; action.containerNode = node.supernode; - action.container = node.supernode.view; + action.node = node; [actions addObject:action]; } else if (node == container || shouldAggregateSubnodeLabels) { - // Even though not surfaced to UIKit, create a non-interactive element for purposes of building sorted aggregated label. - ASAccessibilityElement *nonInteractiveElement = [ASAccessibilityElement accessibilityElementWithContainer:view node:node containerNode:container]; + // Even though not surfaced to UIKit, create a non-interactive element for purposes + // of building sorted aggregated label. + ASAccessibilityElement *nonInteractiveElement = [ASAccessibilityElement accessibilityElementWithContainerNode:container node:node]; [labeledNodes addObject:nonInteractiveElement]; } } @@ -194,12 +210,10 @@ static void CollectAccessibilityElementsForContainer(ASDisplayNode *container, U [elements addObject:accessiblityElement]; } -/// Collect all accessibliity elements for a given view and view node -static void CollectAccessibilityElementsForView(UIView *view, NSMutableArray *elements) +/// Collect all accessibliity elements for a given node +static void CollectAccessibilityElements(ASDisplayNode *node, NSMutableArray *elements) { ASDisplayNodeCAssertNotNil(elements, @"Should pass in a NSMutableArray"); - - ASDisplayNode *node = view.asyncdisplaykit_node; BOOL anySubNodeIsCollection = (nil != ASDisplayNodeFindFirstNode(node, ^BOOL(ASDisplayNode *nodeToCheck) { @@ -207,41 +221,44 @@ static void CollectAccessibilityElementsForView(UIView *view, NSMutableArray *el ASDynamicCast(nodeToCheck, ASTableNode) != nil; })); + // Handle an accessibility container (collects accessibility labels or custom actions) if (node.isAccessibilityContainer && !anySubNodeIsCollection) { - CollectAccessibilityElementsForContainer(node, view, elements); + AggregateSublabelsOrCustomActionsForContainerNode(node, elements); return; } - // Handle rasterize case + // Handle a rasterize node if (node.rasterizesSubtree) { - CollectUIAccessibilityElementsForNode(node, node, view, elements); + CollectAccessibilityElementsForLayerBackedOrRasterizedNode(node, node, elements); return; } - + + // Go down each subnodes and collect all accessibility elements for (ASDisplayNode *subnode in node.subnodes) { if (subnode.isAccessibilityElement) { - // An accessiblityElement can either be a UIView or a UIAccessibilityElement if (subnode.isLayerBacked) { // No view for layer backed nodes exist. It's necessary to create a UIAccessibilityElement that represents this node - UIAccessibilityElement *accessiblityElement = [ASAccessibilityElement accessibilityElementWithContainer:view node:subnode containerNode:node]; + UIAccessibilityElement *accessiblityElement = [ASAccessibilityElement accessibilityElementWithContainerNode:node node:subnode]; [elements addObject:accessiblityElement]; } else { // Accessiblity element is not layer backed just add the view as accessibility element [elements addObject:subnode.view]; } } else if (subnode.isLayerBacked) { - // Go down the hierarchy of the layer backed subnode and collect all of the UIAccessibilityElement - CollectUIAccessibilityElementsForNode(subnode, node, view, elements); - } else if ([subnode accessibilityElementCount] > 0) { - // UIView is itself a UIAccessibilityContainer just add it + // Go down the hierarchy for layer backed subnodes which are also accessibility containe + // and collect all of the UIAccessibilityElement + CollectAccessibilityElementsForLayerBackedOrRasterizedNode(node, subnode, elements); + } else if (subnode.accessibilityElementCount > 0) { + // _ASDisplayView is itself a UIAccessibilityContainer just add it, UIKit will call the accessiblity + // methods of the nodes _ASDisplayView [elements addObject:subnode.view]; } } } @interface _ASDisplayView () { - NSArray *_accessibilityElements; + _ASDisplayViewAccessibilityFlags _accessibilityFlags; } @end @@ -250,10 +267,51 @@ @implementation _ASDisplayView (UIAccessibilityContainer) #pragma mark - UIAccessibility +- (NSInteger)accessibilityElementCount +{ + ASDisplayNodeAssertMainThread(); + if (_accessibilityFlags.inAccessibilityElementCount) { + return [super accessibilityElementCount]; + } + _accessibilityFlags.inAccessibilityElementCount = YES; + NSInteger accessibilityElementCount = [self.asyncdisplaykit_node accessibilityElementCount]; + _accessibilityFlags.inAccessibilityElementCount = NO; + return accessibilityElementCount; +} + +- (NSInteger)indexOfAccessibilityElement:(id)element +{ + ASDisplayNodeAssertMainThread(); + if (_accessibilityFlags.inIndexOfAccessibilityElement) { + return [super indexOfAccessibilityElement:element]; + } + _accessibilityFlags.inIndexOfAccessibilityElement = YES; + NSInteger indexOfAccessibilityElement = [self.asyncdisplaykit_node indexOfAccessibilityElement:element]; + _accessibilityFlags.inIndexOfAccessibilityElement = NO; + return indexOfAccessibilityElement; +} + +- (id)accessibilityElementAtIndex:(NSInteger)index +{ + ASDisplayNodeAssertMainThread(); + if (_accessibilityFlags.inAccessibilityElementAtIndex) { + return [super accessibilityElementAtIndex:index]; + } + _accessibilityFlags.inAccessibilityElementAtIndex = YES; + id accessibilityElement = [self.asyncdisplaykit_node accessibilityElementAtIndex:index]; + _accessibilityFlags.inAccessibilityElementAtIndex = NO; + return accessibilityElement; +} + - (void)setAccessibilityElements:(NSArray *)accessibilityElements { ASDisplayNodeAssertMainThread(); - _accessibilityElements = accessibilityElements; + if (_accessibilityFlags.inSetAccessibilityElements) { + return [super setAccessibilityElements:accessibilityElements]; + } + _accessibilityFlags.inSetAccessibilityElements = YES; + [self.asyncdisplaykit_node setAccessibilityElements:accessibilityElements]; + _accessibilityFlags.inSetAccessibilityElements = NO; } - (NSArray *)accessibilityElements @@ -265,53 +323,81 @@ - (NSArray *)accessibilityElements return @[]; } - if (_accessibilityElements == nil) { - _accessibilityElements = [viewNode accessibilityElements]; - } - return _accessibilityElements; + return [viewNode accessibilityElements]; } @end @implementation ASDisplayNode (AccessibilityInternal) +- (NSInteger)accessibilityElementCount +{ + return [_view accessibilityElementCount]; +} + +- (NSInteger)indexOfAccessibilityElement:(id)element +{ + return [_view indexOfAccessibilityElement:element]; +} + +- (id)accessibilityElementAtIndex:(NSInteger)index +{ + return [_view accessibilityElementAtIndex:index]; +} + +- (void)setAccessibilityElements:(NSArray *)accessibilityElements +{ + _accessibilityElements = accessibilityElements; + [_view setAccessibilityElements:accessibilityElements]; +} + - (NSArray *)accessibilityElements { if (!self.isNodeLoaded) { ASDisplayNodeFailAssert(@"Cannot access accessibilityElements since node is not loaded"); return @[]; } - NSMutableArray *accessibilityElements = [[NSMutableArray alloc] init]; - CollectAccessibilityElementsForView(self.view, accessibilityElements); - SortAccessibilityElements(accessibilityElements); - return accessibilityElements; + + if (_accessibilityElements == nil) { + NSMutableArray *accessibilityElements = [[NSMutableArray alloc] init]; + CollectAccessibilityElements(self, accessibilityElements); + SortAccessibilityElements(accessibilityElements); + _accessibilityElements = accessibilityElements; + } + return _accessibilityElements; } @end @implementation _ASDisplayView (UIAccessibilityAction) -- (BOOL)accessibilityActivate { +- (BOOL)accessibilityActivate +{ return [self.asyncdisplaykit_node accessibilityActivate]; } -- (void)accessibilityIncrement { +- (void)accessibilityIncrement +{ [self.asyncdisplaykit_node accessibilityIncrement]; } -- (void)accessibilityDecrement { +- (void)accessibilityDecrement +{ [self.asyncdisplaykit_node accessibilityDecrement]; } -- (BOOL)accessibilityScroll:(UIAccessibilityScrollDirection)direction { +- (BOOL)accessibilityScroll:(UIAccessibilityScrollDirection)direction +{ return [self.asyncdisplaykit_node accessibilityScroll:direction]; } -- (BOOL)accessibilityPerformEscape { +- (BOOL)accessibilityPerformEscape +{ return [self.asyncdisplaykit_node accessibilityPerformEscape]; } -- (BOOL)accessibilityPerformMagicTap { +- (BOOL)accessibilityPerformMagicTap +{ return [self.asyncdisplaykit_node accessibilityPerformMagicTap]; } diff --git a/Source/Private/ASDisplayNode+UIViewBridge.mm b/Source/Private/ASDisplayNode+UIViewBridge.mm index ec8b4a685..6eb7f0b58 100644 --- a/Source/Private/ASDisplayNode+UIViewBridge.mm +++ b/Source/Private/ASDisplayNode+UIViewBridge.mm @@ -1325,12 +1325,6 @@ - (UIBezierPath *)accessibilityPath return _getAccessibilityFromViewOrProperty(_accessibilityPath, accessibilityPath); } -- (NSInteger)accessibilityElementCount -{ - _bridge_prologue_read; - return _getFromViewOnly(accessibilityElementCount); -} - @end diff --git a/Source/Private/ASDisplayNodeInternal.h b/Source/Private/ASDisplayNodeInternal.h index 55536eefd..47b134804 100644 --- a/Source/Private/ASDisplayNodeInternal.h +++ b/Source/Private/ASDisplayNodeInternal.h @@ -148,6 +148,8 @@ static constexpr CACornerMask kASCACornerAllCorners = ASDisplayNodePerformanceMeasurementOptions _measurementOptions; ASDisplayNodeMethodOverrides _methodOverrides; + NSArray *_accessibilityElements; + @protected ASDisplayNode * __weak _supernode; NSMutableArray *_subnodes; diff --git a/Tests/ASConfigurationTests.mm b/Tests/ASConfigurationTests.mm index 150360ea9..1c120b558 100644 --- a/Tests/ASConfigurationTests.mm +++ b/Tests/ASConfigurationTests.mm @@ -31,7 +31,9 @@ ASExperimentalOOMBackgroundDeallocDisable, ASExperimentalTransactionOperationRetainCycle, ASExperimentalRemoveTextKitInitialisingLock, - ASExperimentalDrawingGlobal + ASExperimentalDrawingGlobal, + ASExperimentalTextNode2A11YContainer, + ASExperimentalExposeTextLinksForA11Y }; @interface ASConfigurationTests : ASTestCase @@ -57,7 +59,9 @@ + (NSArray *)names { @"exp_oom_bg_dealloc_disable", @"exp_transaction_operation_retain_cycle", @"exp_remove_textkit_initialising_lock", - @"exp_drawing_global" + @"exp_drawing_global", + @"exp_text_node_2_a11y_container", + @"exp_expose_text_links_a11y" ]; } diff --git a/Tests/ASTextNode2Tests.mm b/Tests/ASTextNode2Tests.mm index 39d3b7aa6..bd00f3891 100644 --- a/Tests/ASTextNode2Tests.mm +++ b/Tests/ASTextNode2Tests.mm @@ -28,6 +28,11 @@ @implementation ASTextNode2Tests - (void)setUp { [super setUp]; + + // Reset configuration on every setup + ASConfiguration *config = [[ASConfiguration alloc] initWithDictionary:nil]; + [ASConfigurationManager test_resetWithConfiguration:config]; + _textNode = [[ASTextNode2 alloc] init]; UIFontDescriptor *desc = [UIFontDescriptor fontDescriptorWithName:@"Didot" size:18]; @@ -64,6 +69,13 @@ - (void)setUp _textNode.attributedText = _attributedText; } +- (void)setUpEnablingExperiments +{ + ASConfiguration *config = [[ASConfiguration alloc] initWithDictionary:nil]; + config.experimentalFeatures = ASExperimentalTextNode2A11YContainer | ASExperimentalExposeTextLinksForA11Y; + [ASConfigurationManager test_resetWithConfiguration:config]; +} + - (void)testTruncation { XCTAssertTrue([(ASTextNode *)_textNode shouldTruncateForConstrainedSize:ASSizeRangeMake(CGSizeMake(100, 100))], @"Text Node should truncate"); @@ -72,7 +84,7 @@ - (void)testTruncation XCTAssertTrue(_textNode.isTruncated, @"Text Node should be truncated"); } -- (void)testAccessibility +- (void)testBasicAccessibility { XCTAssertTrue(_textNode.isAccessibilityElement, @"Should be an accessibility element"); XCTAssertTrue(_textNode.accessibilityTraits == UIAccessibilityTraitStaticText, @@ -91,20 +103,205 @@ - (void)testAccessibility _textNode.defaultAccessibilityLabel, _attributedText.string); } -- (void)testRespectingAccessibilitySetting +- (void)testBasicAccessibilityWithExperiments +{ + [self setUpEnablingExperiments]; + + XCTAssertFalse(_textNode.isAccessibilityElement, @"Is not an accessiblity element as it's a UIAccessibilityContainer"); + XCTAssertTrue(_textNode.accessibilityTraits == UIAccessibilityTraitStaticText, + @"Should have static text accessibility trait, instead has %llu", + _textNode.accessibilityTraits); + XCTAssertTrue(_textNode.defaultAccessibilityTraits == UIAccessibilityTraitStaticText, + @"Default accessibility traits should return static text accessibility trait, " + @"instead returns %llu", + _textNode.defaultAccessibilityTraits); + + XCTAssertTrue([_textNode.accessibilityLabel isEqualToString:_attributedText.string], + @"Accessibility label is incorrectly set to \n%@\n when it should be \n%@\n", + _textNode.accessibilityLabel, _attributedText.string); + XCTAssertTrue([_textNode.defaultAccessibilityLabel isEqualToString:_attributedText.string], + @"Default accessibility label incorrectly returns \n%@\n when it should be \n%@\n", + _textNode.defaultAccessibilityLabel, _attributedText.string); + + XCTAssertTrue(_textNode.accessibilityElements.count == 1, @"Accessibility elements should exist"); + XCTAssertTrue([[_textNode.accessibilityElements[0] accessibilityLabel] isEqualToString:_attributedText.string], + @"First accessibility element incorrectly returns \n%@\n when it should be \n%@\n", + [_textNode.accessibilityElements[0] accessibilityLabel], _textNode.accessibilityLabel); + XCTAssertTrue([[_textNode.accessibilityElements[0] accessibilityLabel] isEqualToString:_attributedText.string], + @"First accessibility element incorrectly returns \n%@\n when it should be \n%@\n", + [_textNode.accessibilityElements[0] accessibilityLabel], _textNode.accessibilityLabel); +} + +- (void)testAccessibilityLayerBackedContainerAndTextNode2 +{ + ASDisplayNode *container = [[ASDisplayNode alloc] init]; + container.frame = CGRectMake(50, 50, 200, 600); + container.backgroundColor = [UIColor grayColor]; + + ASDisplayNode *layerBackedContainer = [[ASDisplayNode alloc] init]; + layerBackedContainer.layerBacked = YES; + layerBackedContainer.frame = CGRectMake(50, 50, 200, 600); + layerBackedContainer.backgroundColor = [UIColor grayColor]; + [container addSubnode:layerBackedContainer]; + + ASTextNode2 *text = [[ASTextNode2 alloc] init]; + text.layerBacked = YES; + text.attributedText = [[NSAttributedString alloc] initWithString:@"hello"]; + text.frame = CGRectMake(50, 100, 200, 200); + [layerBackedContainer addSubnode:text]; + + ASTextNode2 *text2 = [[ASTextNode2 alloc] init]; + text2.layerBacked = YES; + text2.attributedText = [[NSAttributedString alloc] initWithString:@"world"]; + text2.frame = CGRectMake(50, 100, 200, 200); + [layerBackedContainer addSubnode:text2]; + + NSArray *elements = container.view.accessibilityElements; + XCTAssertTrue(elements.count == 2); + XCTAssertTrue([[elements[0] accessibilityLabel] isEqualToString:@"hello"]); + XCTAssertTrue([[elements[1] accessibilityLabel] isEqualToString:@"world"]); +} + +- (void)testBasicAccessibilityWithExperimentsWithExperiments +{ + [self setUpEnablingExperiments]; + [self testAccessibilityLayerBackedContainerAndTextNode2]; +} + +- (void)testAccessibilityLayerBackedTextNode2WithExperiments +{ + [self setUpEnablingExperiments]; + + ASDisplayNode *container = [[ASDisplayNode alloc] init]; + container.frame = CGRectMake(50, 50, 200, 600); + container.backgroundColor = [UIColor grayColor]; + + ASTextNode2 *text = [[ASTextNode2 alloc] init]; + text.layerBacked = YES; + text.attributedText = [[NSAttributedString alloc] initWithString:@"hello"]; + text.frame = CGRectMake(50, 100, 200, 200); + [container addSubnode:text]; + + // Trigger calculation of layouts on both nodes manually otherwise the internal + // text container will not have any size and the accessibility elements are not layed out + // properly + (void)[text layoutThatFits:ASSizeRangeMake(CGSizeZero, container.frame.size)]; + (void)[container layoutThatFits:ASSizeRangeMake(CGSizeZero, container.frame.size)]; + [container layoutIfNeeded]; + [container.layer displayIfNeeded]; + + NSArray *elements = container.view.accessibilityElements; + XCTAssertTrue(elements.count == 1); + + UIAccessibilityElement *firstElement = elements.firstObject; + XCTAssertTrue([firstElement.accessibilityLabel isEqualToString:@"hello"]); + XCTAssertTrue(CGRectEqualToRect(CGRectMake(50, 102, 26, 13), CGRectIntegral(firstElement.accessibilityFrame))); +} + +- (void)testThatASTextNode2SubnodeAccessibilityLabelAggregationWorks +{ + // Setup nodes + ASDisplayNode *node = [[ASDisplayNode alloc] init]; + ASTextNode2 *innerNode1 = [[ASTextNode2 alloc] init]; + ASTextNode2 *innerNode2 = [[ASTextNode2 alloc] init]; + + // Initialize nodes with relevant accessibility data + node.isAccessibilityContainer = YES; + innerNode1.attributedText = [[NSAttributedString alloc] initWithString:@"hello"]; + innerNode2.attributedText = [[NSAttributedString alloc] initWithString:@"world"]; + + // Attach the subnodes to the parent node, then ensure their accessibility labels have been' + // aggregated to the parent's accessibility label + [node addSubnode:innerNode1]; + [node addSubnode:innerNode2]; + XCTAssertEqualObjects([node.view.accessibilityElements.firstObject accessibilityLabel], + @"hello, world", @"Subnode accessibility label aggregation broken %@", + [node.view.accessibilityElements.firstObject accessibilityLabel]); +} + +- (void)testThatASTextNode2SubnodeAccessibilityLabelAggregationWorksWithExperiments +{ + [self setUpEnablingExperiments]; + [self testThatASTextNode2SubnodeAccessibilityLabelAggregationWorks]; +} + +- (void)testThatLayeredBackedASTextNode2SubnodeAccessibilityLabelAggregationWorks +{ + // Setup nodes + ASDisplayNode *node = [[ASDisplayNode alloc] init]; + ASTextNode2 *innerNode1 = [[ASTextNode2 alloc] init]; + innerNode1.layerBacked = YES; + ASTextNode2 *innerNode2 = [[ASTextNode2 alloc] init]; + innerNode2.layerBacked = YES; + + // Initialize nodes with relevant accessibility data + node.isAccessibilityContainer = YES; + innerNode1.attributedText = [[NSAttributedString alloc] initWithString:@"hello"]; + innerNode2.attributedText = [[NSAttributedString alloc] initWithString:@"world"]; + + // Attach the subnodes to the parent node, then ensure their accessibility labels have been' + // aggregated to the parent's accessibility label + [node addSubnode:innerNode1]; + [node addSubnode:innerNode2]; + XCTAssertEqualObjects([node.view.accessibilityElements.firstObject accessibilityLabel], + @"hello, world", @"Subnode accessibility label aggregation broken %@", + [node.view.accessibilityElements.firstObject accessibilityLabel]); + +} + +- (void)testThatLayeredBackedASTextNode2SubnodeAccessibilityLabelAggregationWorksWithExperiments +{ + [self setUpEnablingExperiments]; + [self testThatLayeredBackedASTextNode2SubnodeAccessibilityLabelAggregationWorks]; +} + +- (void)testThatASTextNode2SubnodeCustomActionsAreWorking +{ + ASDisplayNode *node = [[ASDisplayNode alloc] init]; + ASTextNode2 *innerNode1 = [[ASTextNode2 alloc] init]; + innerNode1.accessibilityTraits = UIAccessibilityTraitButton; + ASTextNode2 *innerNode2 = [[ASTextNode2 alloc] init]; + innerNode2.accessibilityTraits = UIAccessibilityTraitButton; + + // Initialize nodes with relevant accessibility data + node.isAccessibilityContainer = YES; + innerNode1.attributedText = [[NSAttributedString alloc] initWithString:@"hello"]; + innerNode2.attributedText = [[NSAttributedString alloc] initWithString:@"world"]; + + // Attach the subnodes to the parent node, then ensure their accessibility labels have been' + // aggregated to the parent's accessibility label + [node addSubnode:innerNode1]; + [node addSubnode:innerNode2]; + + NSArray *accessibilityElements = node.view.accessibilityElements; + XCTAssertTrue(accessibilityElements.count == 1, @"Container node should have one accessibility element for custom actions"); + + NSArray *accessibilityCustomActions = accessibilityElements.firstObject.accessibilityCustomActions; + XCTAssertTrue(accessibilityCustomActions.count == 2, @"Text nodes should be exposed as a11y custom actions."); +} + +- (void)testThatASTextNode2SubnodeCustomActionsAreWorkingWithExperiments +{ + [self setUpEnablingExperiments]; + [self testThatASTextNode2SubnodeCustomActionsAreWorking]; +} + +- (void)testAccessibilityExposeA11YLinksWithExperiments { - ASTextNode2 *textNode = [[ASTextNode2 alloc] init]; - textNode.attributedText = _attributedText; - textNode.isAccessibilityElement = NO; - - textNode.attributedText = [[NSAttributedString alloc] initWithString:@"new string"]; - XCTAssertFalse(textNode.isAccessibilityElement); - - // Ensure removing string on an accessible text node updates the setting. - ASTextNode2 *accessibleTextNode = [ASTextNode2 new]; - accessibleTextNode.attributedText = _attributedText; - accessibleTextNode.attributedText = nil; - XCTAssertFalse(accessibleTextNode.isAccessibilityElement); + [self setUpEnablingExperiments]; + + NSString *link = @"https://texturegroup.com"; + NSMutableAttributedString *attributedText = [[NSMutableAttributedString alloc] initWithString:[NSString stringWithFormat:@"Texture Website: %@", link]]; + NSRange linkRange = [attributedText.string rangeOfString:link]; + [attributedText addAttribute:NSLinkAttributeName value:link range:linkRange]; + + _textNode.attributedText = attributedText; + + NSArray *accessibilityElements = _textNode.accessibilityElements; + XCTAssertTrue(accessibilityElements.count == 2, @"Link should be exposed as accessibility element"); + + XCTAssertTrue([[accessibilityElements[0] accessibilityLabel] isEqualToString:attributedText.string], @"First accessibility element should be the full text"); + XCTAssertTrue([[accessibilityElements[1] accessibilityLabel] isEqualToString:link], @"Second accessibility element should be the link"); } @end diff --git a/examples/ASDKgram/Sample/PhotoCellNode.m b/examples/ASDKgram/Sample/PhotoCellNode.m index 5353da65b..d86914af4 100644 --- a/examples/ASDKgram/Sample/PhotoCellNode.m +++ b/examples/ASDKgram/Sample/PhotoCellNode.m @@ -84,6 +84,8 @@ - (instancetype)initWithPhotoObject:(PhotoModel *)photo; self.automaticallyManagesSubnodes = YES; [self setupYogaLayoutIfNeeded]; + + self.accessibilityIdentifier = @"PhotoCellNode"; #if DEBUG_PHOTOCELL_LAYOUT _userAvatarImageNode.backgroundColor = [UIColor greenColor]; diff --git a/examples/ASDKgram/Sample/PhotoFeedNodeController.m b/examples/ASDKgram/Sample/PhotoFeedNodeController.m index 8abc80bb5..bcdadede5 100644 --- a/examples/ASDKgram/Sample/PhotoFeedNodeController.m +++ b/examples/ASDKgram/Sample/PhotoFeedNodeController.m @@ -93,6 +93,21 @@ - (ASCellNodeBlock)tableNode:(ASTableNode *)tableNode nodeBlockForRowAtIndexPath return ASCellNodeBlock; } +static void InvalidateAccessibleElementsNode(ASDisplayNode *node) { + for (ASDisplayNode *subnode in node.subnodes) { + subnode.accessibilityElements = nil; + InvalidateAccessibleElementsNode(subnode); + } +} + +- (void)scrollViewDidScroll:(UIScrollView *)scrollView +{ + for (ASCellNode *visibleNode in self.tableNode.visibleNodes) { + visibleNode.accessibilityElements = nil; + InvalidateAccessibleElementsNode(visibleNode); + } +} + #pragma mark - ASTableDelegate methods // Receive a message that the tableView is near the end of its data set and more data should be fetched if necessary. diff --git a/examples/ASDKgram/Sample/PhotoModel.m b/examples/ASDKgram/Sample/PhotoModel.m index 0662e71ae..6186ac313 100644 --- a/examples/ASDKgram/Sample/PhotoModel.m +++ b/examples/ASDKgram/Sample/PhotoModel.m @@ -47,12 +47,27 @@ - (instancetype)initWithUnsplashPhoto:(NSDictionary *)photoDictionary - (NSAttributedString *)descriptionAttributedStringWithFontSize:(CGFloat)size { - NSString *string = [NSString stringWithFormat:@"%@ %@", self.ownerUserProfile.username, self.descriptionText]; - NSAttributedString *attrString = [NSAttributedString attributedStringWithString:string - fontSize:size - color:[UIColor darkGrayColor] - firstWordColor:[UIColor darkBlueColor]]; - return attrString; + if ((id)self.descriptionText == [NSNull null] || self.descriptionText.length == 0) { + return [[NSAttributedString alloc] init]; + } + + NSString *descriptionText = self.descriptionText; + NSMutableAttributedString *attributedString = [[NSAttributedString attributedStringWithString:descriptionText fontSize:size color:[UIColor darkGrayColor] firstWordColor:nil] mutableCopy]; + + // Scan through description text for links + static NSDataDetector *dataDector = nil; + static dispatch_once_t onceToken; + dispatch_once(&onceToken, ^{ + dataDector = [NSDataDetector dataDetectorWithTypes:NSTextCheckingTypeLink error:nil]; + }); + + [dataDector enumerateMatchesInString:descriptionText options:kNilOptions range:NSMakeRange(0, descriptionText.length) usingBlock:^(NSTextCheckingResult * _Nullable result, NSMatchingFlags flags, BOOL * _Nonnull stop) { + NSRange range = result.range; + [attributedString addAttribute:NSLinkAttributeName value:[descriptionText substringWithRange:range] range:range]; + [attributedString addAttribute:NSForegroundColorAttributeName value:[UIColor darkBlueColor] range:range]; + }]; + + return [attributedString copy]; } - (NSAttributedString *)uploadDateAttributedStringWithFontSize:(CGFloat)size @@ -62,7 +77,7 @@ - (NSAttributedString *)uploadDateAttributedStringWithFontSize:(CGFloat)size - (NSAttributedString *)likesAttributedStringWithFontSize:(CGFloat)size { - NSString *likesString = [NSString stringWithFormat:@"♥︎ %lu likes", (unsigned long)_likesCount]; + NSString *likesString = [NSString stringWithFormat:@"♥︎ %lu likes", (unsigned long)self.likesCount]; return [NSAttributedString attributedStringWithString:likesString fontSize:size color:[UIColor darkBlueColor] firstWordColor:nil]; } @@ -74,7 +89,7 @@ - (NSAttributedString *)locationAttributedStringWithFontSize:(CGFloat)size - (NSString *)description { - return [NSString stringWithFormat:@"%@ - %@", _photoID, _descriptionText]; + return [NSString stringWithFormat:@"%@ - %@", self.photoID, self.descriptionText]; } - (id)diffIdentifier diff --git a/examples/ASDKgram/Sample/TextureConfigDelegate.m b/examples/ASDKgram/Sample/TextureConfigDelegate.m index e2fabef4a..c6fd5a487 100644 --- a/examples/ASDKgram/Sample/TextureConfigDelegate.m +++ b/examples/ASDKgram/Sample/TextureConfigDelegate.m @@ -17,7 +17,7 @@ @implementation ASConfiguration (UserProvided) + (ASConfiguration *)textureConfiguration { ASConfiguration *config = [[ASConfiguration alloc] init]; - config.experimentalFeatures = ASExperimentalTextNode; + config.experimentalFeatures = ASExperimentalTextNode | ASExperimentalTextNode2A11YContainer | ASExperimentalExposeTextLinksForA11Y; config.delegate = [[TextureConfigDelegate alloc] init]; return config; } From 9f6394f11b2c8b772d6e81c27dc3bce39ae97a9c Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Fri, 31 May 2019 10:27:59 -0700 Subject: [PATCH 2/6] Fix ASTextNode2 accessibilityElementCount --- Source/ASTextNode2.mm | 25 +++++++++++++++------- Tests/ASTextNode2Tests.mm | 44 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 8 deletions(-) diff --git a/Source/ASTextNode2.mm b/Source/ASTextNode2.mm index f3e4112c9..208b51fa9 100644 --- a/Source/ASTextNode2.mm +++ b/Source/ASTextNode2.mm @@ -356,6 +356,15 @@ static void ASUpdateAccessibilityFrame(ASTextNodeAccessiblityElement *accessibil return containerNode; } +- (NSInteger)accessibilityElementCount +{ + if (ASActivateExperimentalFeature(ASExperimentalTextNode2A11YContainer)) { + return self.accessibilityElements.count; + } + + return [super accessibilityElementCount]; +} + /// Overwrite accessibilityElementAtIndex: so we can update the element's accessibilityFrame when it is requested. - (id)accessibilityElementAtIndex:(NSInteger)index { @@ -416,16 +425,16 @@ - (NSArray *)accessibilityElements } - (void)setIsAccessibilityElement:(BOOL)isAccessibilityElement - { - if (ASActivateExperimentalFeature(ASExperimentalTextNode2A11YContainer)) { - // Instead of relying on labels accessibility, We implement UIAccessibilityContainer and - // handle accessibility with ASTextNode2 - return; - } +{ + if (ASActivateExperimentalFeature(ASExperimentalTextNode2A11YContainer)) { + // Instead of relying on labels accessibility, We implement UIAccessibilityContainer and + // handle accessibility with ASTextNode2 + return; + } - [super setIsAccessibilityElement:isAccessibilityElement]; + [super setIsAccessibilityElement:isAccessibilityElement]; - } +} - (BOOL)isAccessibilityElement { if (ASActivateExperimentalFeature(ASExperimentalTextNode2A11YContainer)) { diff --git a/Tests/ASTextNode2Tests.mm b/Tests/ASTextNode2Tests.mm index bd00f3891..701e9022e 100644 --- a/Tests/ASTextNode2Tests.mm +++ b/Tests/ASTextNode2Tests.mm @@ -304,4 +304,48 @@ - (void)testAccessibilityExposeA11YLinksWithExperiments XCTAssertTrue([[accessibilityElements[1] accessibilityLabel] isEqualToString:link], @"Second accessibility element should be the link"); } +- (void)testAccessibilityNonLayerbackedNodesOperationInNonContainer +{ + ASDisplayNode *contianer = [[ASDisplayNode alloc] init]; + contianer.frame = CGRectMake(50, 50, 200, 600); + contianer.backgroundColor = [UIColor grayColor]; + // Do any additional setup after loading the view, typically from a nib. + ASTextNode2 *text1 = [[ASTextNode2 alloc] init]; + text1.attributedText = [[NSAttributedString alloc] initWithString:@"hello"]; + text1.frame = CGRectMake(50, 100, 200, 200); + [contianer addSubnode:text1]; + [contianer layoutIfNeeded]; + [contianer.layer displayIfNeeded]; + NSArray *elements = contianer.view.accessibilityElements; + XCTAssertTrue(elements.count == 1); + XCTAssertTrue([[elements.firstObject accessibilityLabel] isEqualToString:@"hello"]); + ASTextNode2 *text2 = [[ASTextNode2 alloc] init]; + text2.attributedText = [[NSAttributedString alloc] initWithString:@"world"]; + text2.frame = CGRectMake(50, 300, 200, 200); + [contianer addSubnode:text2]; + [contianer layoutIfNeeded]; + [contianer.layer displayIfNeeded]; + NSArray *updatedElements = contianer.view.accessibilityElements; + XCTAssertTrue(updatedElements.count == 2); + XCTAssertTrue([[updatedElements.firstObject accessibilityLabel] isEqualToString:@"hello"]); + XCTAssertTrue([[updatedElements.lastObject accessibilityLabel] isEqualToString:@"world"]); + ASTextNode2 *text3 = [[ASTextNode2 alloc] init]; + text3.attributedText = [[NSAttributedString alloc] initWithString:@"!!!!"]; + text3.frame = CGRectMake(50, 400, 200, 100); + [text2 addSubnode:text3]; + [contianer layoutIfNeeded]; + [contianer.layer displayIfNeeded]; + NSArray *updatedElements2 = contianer.view.accessibilityElements; + //text3 won't be read out cause it's overshadowed by text2 + XCTAssertTrue(updatedElements2.count == 2); + XCTAssertTrue([[updatedElements2.firstObject accessibilityLabel] isEqualToString:@"hello"]); + XCTAssertTrue([[updatedElements2.lastObject accessibilityLabel] isEqualToString:@"world"]); +} + +- (void)testAccessibilityNonLayerbackedNodesOperationInNonContainerWithExperiment +{ + [self setUpEnablingExperiments]; + [self testAccessibilityNonLayerbackedNodesOperationInNonContainer]; +} + @end From 6e36749e5c16fb11912580fb70a621431ec62f0e Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Fri, 31 May 2019 10:42:37 -0700 Subject: [PATCH 3/6] Small improvements --- Source/ASTextNode2.mm | 5 ++--- Source/Details/_ASDisplayViewAccessiblity.mm | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Source/ASTextNode2.mm b/Source/ASTextNode2.mm index 208b51fa9..6ea1f5aae 100644 --- a/Source/ASTextNode2.mm +++ b/Source/ASTextNode2.mm @@ -390,15 +390,14 @@ - (NSArray *)accessibilityElements NSMutableArray *accessibilityElements = [[NSMutableArray alloc] init]; - // Searc the first node that is not layer backed + // Search the first node that is not layer backed ASDisplayNode *containerNode = ASFirstNonLayerBackedSupernodeForNode(self); - UIAccessibilityTraits accessibilityTraits = self.accessibilityTraits; ASTextLayout *layout = ASTextNodeCompatibleLayoutWithContainerAndText(_textContainer, attributedText); // Create an accessibility element to represent the label's text. It's not necessary to specify // a accessibilityRange here, as the entirety of the text is being represented. ASTextNodeAccessiblityElement *accessibilityElement = [[ASTextNodeAccessiblityElement alloc] initWithAccessibilityContainer:containerNode.view]; - accessibilityElement.accessibilityTraits = accessibilityTraits; + accessibilityElement.accessibilityTraits = self.accessibilityTraits; accessibilityElement.accessibilityLabel = self.accessibilityLabel; ASUpdateAccessibilityFrame(accessibilityElement, layout, containerNode, self); [accessibilityElements addObject:accessibilityElement]; diff --git a/Source/Details/_ASDisplayViewAccessiblity.mm b/Source/Details/_ASDisplayViewAccessiblity.mm index 96decf099..890c6d03e 100644 --- a/Source/Details/_ASDisplayViewAccessiblity.mm +++ b/Source/Details/_ASDisplayViewAccessiblity.mm @@ -246,7 +246,7 @@ static void CollectAccessibilityElements(ASDisplayNode *node, NSMutableArray *el [elements addObject:subnode.view]; } } else if (subnode.isLayerBacked) { - // Go down the hierarchy for layer backed subnodes which are also accessibility containe + // Go down the hierarchy for layer backed subnodes which are also accessibility container // and collect all of the UIAccessibilityElement CollectAccessibilityElementsForLayerBackedOrRasterizedNode(node, subnode, elements); } else if (subnode.accessibilityElementCount > 0) { From 481cb47463bb4a5e38864f967d2a1303098bbd7d Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Fri, 31 May 2019 14:14:15 -0700 Subject: [PATCH 4/6] Copy ore a11y attributes over --- Source/ASTextNode2.mm | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/Source/ASTextNode2.mm b/Source/ASTextNode2.mm index 6ea1f5aae..06a87627e 100644 --- a/Source/ASTextNode2.mm +++ b/Source/ASTextNode2.mm @@ -397,8 +397,16 @@ - (NSArray *)accessibilityElements // Create an accessibility element to represent the label's text. It's not necessary to specify // a accessibilityRange here, as the entirety of the text is being represented. ASTextNodeAccessiblityElement *accessibilityElement = [[ASTextNodeAccessiblityElement alloc] initWithAccessibilityContainer:containerNode.view]; - accessibilityElement.accessibilityTraits = self.accessibilityTraits; + accessibilityElement.accessibilityIdentifier = self.accessibilityIdentifier; accessibilityElement.accessibilityLabel = self.accessibilityLabel; + accessibilityElement.accessibilityHint = self.accessibilityHint; + accessibilityElement.accessibilityValue = self.accessibilityValue; + accessibilityElement.accessibilityTraits = self.accessibilityTraits; + if (AS_AVAILABLE_IOS_TVOS(11, 11)) { + accessibilityElement.accessibilityAttributedLabel = self.accessibilityAttributedLabel; + accessibilityElement.accessibilityAttributedHint = self.accessibilityAttributedHint; + accessibilityElement.accessibilityAttributedValue = self.accessibilityAttributedValue; + } ASUpdateAccessibilityFrame(accessibilityElement, layout, containerNode, self); [accessibilityElements addObject:accessibilityElement]; @@ -414,6 +422,9 @@ - (NSArray *)accessibilityElements accessibilityElement.accessibilityTraits = UIAccessibilityTraitLink;; accessibilityElement.accessibilityLabel = [attributedText.string substringWithRange:range]; accessibilityElement.accessibilityRange = range; + if (AS_AVAILABLE_IOS_TVOS(11, 11)) { + accessibilityElement.accessibilityAttributedLabel = [attributedText attributedSubstringFromRange:range]; + } ASUpdateAccessibilityFrame(accessibilityElement, layout, containerNode, self); [accessibilityElements addObject:accessibilityElement]; }]; From a3e508acf9c0cbec0ef4d490743c77743c41e6c6 Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Fri, 31 May 2019 14:14:29 -0700 Subject: [PATCH 5/6] More tests --- Tests/ASTextNode2Tests.mm | 51 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/Tests/ASTextNode2Tests.mm b/Tests/ASTextNode2Tests.mm index 701e9022e..d09617e87 100644 --- a/Tests/ASTextNode2Tests.mm +++ b/Tests/ASTextNode2Tests.mm @@ -15,6 +15,7 @@ #import #import "ASTestCase.h" +#import "ASDisplayNodeTestsHelper.h" @interface ASTextNode2Tests : XCTestCase @@ -348,4 +349,54 @@ - (void)testAccessibilityNonLayerbackedNodesOperationInNonContainerWithExperimen [self testAccessibilityNonLayerbackedNodesOperationInNonContainer]; } +- (void)testAccessibilityNonLayerbackedNodesOperationInNonContainerSecond +{ + ASDisplayNode *container = [[ASDisplayNode alloc] init]; + UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 320, 560)]; + [window addSubnode:container]; + [window makeKeyAndVisible]; + + container.frame = CGRectMake(50, 50, 200, 600); + container.backgroundColor = [UIColor grayColor]; + // Do any additional setup after loading the view, typically from a nib. + ASTextNode2 *text1 = [[ASTextNode2 alloc] init]; + text1.attributedText = [[NSAttributedString alloc] initWithString:@"hello"]; + text1.frame = CGRectMake(50, 100, 200, 200); + [container addSubnode:text1]; + [container layoutIfNeeded]; + [container.layer displayIfNeeded]; + NSArray *elements = container.view.accessibilityElements; + XCTAssertTrue(elements.count == 1); + XCTAssertTrue([[elements.firstObject accessibilityLabel] isEqualToString:@"hello"]); + ASTextNode2 *text2 = [[ASTextNode2 alloc] init]; + text2.attributedText = [[NSAttributedString alloc] initWithString:@"world"]; + text2.frame = CGRectMake(50, 300, 200, 200); + [container addSubnode:text2]; + [container layoutIfNeeded]; + [container.layer displayIfNeeded]; + ASCATransactionQueueWait(nil); + NSArray *updatedElements = container.view.accessibilityElements; + XCTAssertTrue(updatedElements.count == 2); + XCTAssertTrue([[updatedElements.firstObject accessibilityLabel] isEqualToString:@"hello"]); + XCTAssertTrue([[updatedElements.lastObject accessibilityLabel] isEqualToString:@"world"]); + ASTextNode2 *text3 = [[ASTextNode2 alloc] init]; + text3.attributedText = [[NSAttributedString alloc] initWithString:@"!!!!"]; + text3.frame = CGRectMake(50, 400, 200, 100); + [text2 addSubnode:text3]; + [container layoutIfNeeded]; + [container.layer displayIfNeeded]; + ASCATransactionQueueWait(nil); + NSArray *updatedElements2 = container.view.accessibilityElements; + //text3 won't be read out cause it's overshadowed by text2 + XCTAssertTrue(updatedElements2.count == 2); + XCTAssertTrue([[updatedElements2.firstObject accessibilityLabel] isEqualToString:@"hello"]); + XCTAssertTrue([[updatedElements2.lastObject accessibilityLabel] isEqualToString:@"world"]); +} + +- (void)testAccessibilityNonLayerbackedNodesOperationInNonContainerSecondWithExperiment +{ + [self setUpEnablingExperiments]; + [self testAccessibilityNonLayerbackedNodesOperationInNonContainerSecond]; +} + @end From adb84232b6d42baf923f4683d4725efe258cf4df Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Fri, 31 May 2019 15:39:30 -0700 Subject: [PATCH 6/6] Try something different --- Source/ASTextNode2.mm | 27 ++++++++++----- Source/Details/_ASDisplayView.mm | 1 + Source/Details/_ASDisplayViewAccessiblity.mm | 13 +++++-- Tests/ASDisplayViewAccessibilityTests.mm | 36 ++++++++++++-------- 4 files changed, 52 insertions(+), 25 deletions(-) diff --git a/Source/ASTextNode2.mm b/Source/ASTextNode2.mm index 06a87627e..3df80e8da 100644 --- a/Source/ASTextNode2.mm +++ b/Source/ASTextNode2.mm @@ -230,7 +230,7 @@ - (instancetype)init self.linkAttributeNames = DefaultLinkAttributeNames(); // Accessibility - self.isAccessibilityElement = YES; + self.isAccessibilityElement = !ASActivateExperimentalFeature(ASExperimentalTextNode2A11YContainer); self.accessibilityTraits = self.defaultAccessibilityTraits; // Placeholders @@ -358,16 +358,21 @@ static void ASUpdateAccessibilityFrame(ASTextNodeAccessiblityElement *accessibil - (NSInteger)accessibilityElementCount { - if (ASActivateExperimentalFeature(ASExperimentalTextNode2A11YContainer)) { - return self.accessibilityElements.count; + if ( + !ASActivateExperimentalFeature(ASExperimentalTextNode2A11YContainer)) { + return [super accessibilityElementCount]; } - return [super accessibilityElementCount]; + return self.accessibilityElements.count; } /// Overwrite accessibilityElementAtIndex: so we can update the element's accessibilityFrame when it is requested. - (id)accessibilityElementAtIndex:(NSInteger)index { + if (!ASActivateExperimentalFeature(ASExperimentalTextNode2A11YContainer)) { + return [super accessibilityElementAtIndex:index]; + } + ASTextNodeAccessiblityElement *accessibilityElement = self.accessibilityElements[index]; ASTextLayout *layout = ASTextNodeCompatibleLayoutWithContainerAndText(_textContainer, _attributedText); @@ -377,6 +382,10 @@ - (id)accessibilityElementAtIndex:(NSInteger)index - (NSArray *)accessibilityElements { + if (!ASActivateExperimentalFeature(ASExperimentalTextNode2A11YContainer)) { + return [super accessibilityElements]; + } + if (_accessibilityElements != nil) { return _accessibilityElements; } @@ -566,10 +575,12 @@ - (void)setAttributedText:(NSAttributedString *)attributedText // Accessiblity self.accessibilityLabel = self.defaultAccessibilityLabel; - // We update the isAccessibilityElement setting if this node is not switching between strings. - if (oldAttributedText.length == 0 || length == 0) { - // We're an accessibility element by default if there is a string. - self.isAccessibilityElement = (length != 0); + if (!ASActivateExperimentalFeature(ASExperimentalTextNode2A11YContainer)) { + // We update the isAccessibilityElement setting if this node is not switching between strings. + if (oldAttributedText.length == 0 || length == 0) { + // We're an accessibility element by default if there is a string. + self.isAccessibilityElement = (length != 0); + } } #if AS_TEXTNODE2_RECORD_ATTRIBUTED_STRINGS diff --git a/Source/Details/_ASDisplayView.mm b/Source/Details/_ASDisplayView.mm index 7f28b054e..1f20ced9f 100644 --- a/Source/Details/_ASDisplayView.mm +++ b/Source/Details/_ASDisplayView.mm @@ -43,6 +43,7 @@ @implementation _ASDisplayView unsigned inIsFirstResponder:1; } _internalFlags; + NSArray *_accessibilityElements; _ASDisplayViewAccessibilityFlags _accessibilityFlags; } diff --git a/Source/Details/_ASDisplayViewAccessiblity.mm b/Source/Details/_ASDisplayViewAccessiblity.mm index 890c6d03e..392370545 100644 --- a/Source/Details/_ASDisplayViewAccessiblity.mm +++ b/Source/Details/_ASDisplayViewAccessiblity.mm @@ -258,6 +258,7 @@ static void CollectAccessibilityElements(ASDisplayNode *node, NSMutableArray *el } @interface _ASDisplayView () { + NSArray *_accessibilityElements; _ASDisplayViewAccessibilityFlags _accessibilityFlags; } @@ -311,6 +312,7 @@ - (void)setAccessibilityElements:(NSArray *)accessibilityElements } _accessibilityFlags.inSetAccessibilityElements = YES; [self.asyncdisplaykit_node setAccessibilityElements:accessibilityElements]; + _accessibilityElements = accessibilityElements; _accessibilityFlags.inSetAccessibilityElements = NO; } @@ -323,7 +325,12 @@ - (NSArray *)accessibilityElements return @[]; } - return [viewNode accessibilityElements]; +// return [viewNode accessibilityElements]; + + if (_accessibilityElements == nil) { + _accessibilityElements = [viewNode accessibilityElements]; + } + return _accessibilityElements; } @end @@ -358,12 +365,12 @@ - (NSArray *)accessibilityElements return @[]; } - if (_accessibilityElements == nil) { +// if (_accessibilityElements == nil) { NSMutableArray *accessibilityElements = [[NSMutableArray alloc] init]; CollectAccessibilityElements(self, accessibilityElements); SortAccessibilityElements(accessibilityElements); _accessibilityElements = accessibilityElements; - } +// } return _accessibilityElements; } diff --git a/Tests/ASDisplayViewAccessibilityTests.mm b/Tests/ASDisplayViewAccessibilityTests.mm index 7d4512cdf..f4d2bc2f3 100644 --- a/Tests/ASDisplayViewAccessibilityTests.mm +++ b/Tests/ASDisplayViewAccessibilityTests.mm @@ -19,6 +19,8 @@ #import #import +#import "ASDisplayNodeTestsHelper.h" + @interface ASDisplayViewAccessibilityTests : XCTestCase @end @@ -90,26 +92,31 @@ - (void)testThatContainerAccessibilityLabelOverrideStopsAggregation - (void)testAccessibilityNonLayerbackedNodesOperationInNonContainer { - ASDisplayNode *contianer = [[ASDisplayNode alloc] init]; - contianer.frame = CGRectMake(50, 50, 200, 600); - contianer.backgroundColor = [UIColor grayColor]; + ASDisplayNode *container = [[ASDisplayNode alloc] init]; + UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 320, 560)]; + [window addSubnode:container]; + [window makeKeyAndVisible]; + + container.frame = CGRectMake(50, 50, 200, 600); + container.backgroundColor = [UIColor grayColor]; // Do any additional setup after loading the view, typically from a nib. ASTextNode *text1 = [[ASTextNode alloc] init]; text1.attributedText = [[NSAttributedString alloc] initWithString:@"hello"]; text1.frame = CGRectMake(50, 100, 200, 200); - [contianer addSubnode:text1]; - [contianer layoutIfNeeded]; - [contianer.layer displayIfNeeded]; - NSArray *elements = contianer.view.accessibilityElements; + [container addSubnode:text1]; + [container layoutIfNeeded]; + [container.layer displayIfNeeded]; + NSArray *elements = container.view.accessibilityElements; XCTAssertTrue(elements.count == 1); XCTAssertTrue([[elements.firstObject accessibilityLabel] isEqualToString:@"hello"]); ASTextNode *text2 = [[ASTextNode alloc] init]; text2.attributedText = [[NSAttributedString alloc] initWithString:@"world"]; text2.frame = CGRectMake(50, 300, 200, 200); - [contianer addSubnode:text2]; - [contianer layoutIfNeeded]; - [contianer.layer displayIfNeeded]; - NSArray *updatedElements = contianer.view.accessibilityElements; + [container addSubnode:text2]; + [container layoutIfNeeded]; + [container.layer displayIfNeeded]; + ASCATransactionQueueWait(nil); + NSArray *updatedElements = container.view.accessibilityElements; XCTAssertTrue(updatedElements.count == 2); XCTAssertTrue([[updatedElements.firstObject accessibilityLabel] isEqualToString:@"hello"]); XCTAssertTrue([[updatedElements.lastObject accessibilityLabel] isEqualToString:@"world"]); @@ -117,9 +124,10 @@ - (void)testAccessibilityNonLayerbackedNodesOperationInNonContainer text3.attributedText = [[NSAttributedString alloc] initWithString:@"!!!!"]; text3.frame = CGRectMake(50, 400, 200, 100); [text2 addSubnode:text3]; - [contianer layoutIfNeeded]; - [contianer.layer displayIfNeeded]; - NSArray *updatedElements2 = contianer.view.accessibilityElements; + [container layoutIfNeeded]; + [container.layer displayIfNeeded]; + ASCATransactionQueueWait(nil); + NSArray *updatedElements2 = container.view.accessibilityElements; //text3 won't be read out cause it's overshadowed by text2 XCTAssertTrue(updatedElements2.count == 2); XCTAssertTrue([[updatedElements2.firstObject accessibilityLabel] isEqualToString:@"hello"]);