From f9df765fb5fef6ddf598e21bc9b56963e8b7a5b0 Mon Sep 17 00:00:00 2001 From: ricky cancro Date: Wed, 22 Apr 2020 12:59:51 -0700 Subject: [PATCH 1/3] [ASDisplayNode] Allow explicit setting of accessibilityElements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since `NSObject` conforms to the informal accessibility protocol, `ASDisplayNode` has an inherited property for `accessibilityElements`. However, setting this property has no effect since `ASDisplayNode` overrides the `accessibilityElements` getter. Added a small change to the getter to check if the `accessibilityElements` property on `ASDisplayNode` has been explicitly set, and if so return that. I also added a comment around `_ASDisplayView`’s `setAccessibilityElements:` method to clear up some (of my own) confusion. --- Source/Details/_ASDisplayViewAccessiblity.mm | 12 ++++ Tests/ASDisplayViewAccessibilityTests.mm | 58 ++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/Source/Details/_ASDisplayViewAccessiblity.mm b/Source/Details/_ASDisplayViewAccessiblity.mm index 858fd5ddc..577467ede 100644 --- a/Source/Details/_ASDisplayViewAccessiblity.mm +++ b/Source/Details/_ASDisplayViewAccessiblity.mm @@ -259,6 +259,10 @@ @implementation _ASDisplayView (UIAccessibilityContainer) - (void)setAccessibilityElements:(NSArray *)accessibilityElements { ASDisplayNodeAssertMainThread(); + // While it looks very strange to ignore the accessibilyElements param and set _accessibilityElements to nil, it is actually on purpose. + // _ASDisplayView's accessibilityElements method will always defer to the node for accessibilityElements when _accessibilityElements is + // nil. Calling setAccessibilityElements on _ASDisplayView is basically us clearing the cache and forcing _ASDisplayView to ask the node + // for its accessibilityElements the next time they are requested. _accessibilityElements = nil; } @@ -283,6 +287,14 @@ @implementation ASDisplayNode (AccessibilityInternal) - (NSArray *)accessibilityElements { + // NSObject implements the informal accessibility protocol. This means that all ASDisplayNodes already have an accessibilityElements + // property. If an ASDisplayNode subclass has explicitly set the property, let's use that instead of traversing the node tree to try + // to create the elements automatically + NSArray *elements = [super accessibilityElements]; + if (elements.count) { + return elements; + } + if (!self.isNodeLoaded) { ASDisplayNodeFailAssert(@"Cannot access accessibilityElements since node is not loaded"); return @[]; diff --git a/Tests/ASDisplayViewAccessibilityTests.mm b/Tests/ASDisplayViewAccessibilityTests.mm index 15d14c29a..f2476a00d 100644 --- a/Tests/ASDisplayViewAccessibilityTests.mm +++ b/Tests/ASDisplayViewAccessibilityTests.mm @@ -12,6 +12,8 @@ #import +#import +#import #import #import #import @@ -193,4 +195,60 @@ - (void)testActionForwarding { OCMVerifyAll(mockNode); } +#pragma mark - +#pragma mark AccessibilityElements + +// dummy action for a button +- (void)fakeSelector:(id)sender { } + +- (void)testThatAccessibilityElementsWorks { + ASDisplayNode *containerNode = [[ASDisplayNode alloc] init]; + containerNode.frame = CGRectMake(0, 0, 100, 200); + + ASTextNode *label = [[ASTextNode alloc] init]; + label.attributedText = [[NSAttributedString alloc] initWithString:@"test label"]; + label.frame = CGRectMake(0, 0, 100, 20); + + ASButtonNode *button = [[ASButtonNode alloc] init]; + [button setTitle:@"tap me" withFont:[UIFont systemFontOfSize:17] withColor:nil forState:UIControlStateNormal]; + [button addTarget:self action:@selector(fakeSelector:) forControlEvents:ASControlNodeEventTouchUpInside]; + button.frame = CGRectMake(0, 25, 100, 20); + + [containerNode addSubnode:label]; + [containerNode addSubnode:button]; + + // force load + __unused UIView *view = containerNode.view; + + NSArray *elements = [containerNode accessibilityElements]; + XCTAssertTrue(elements.count == 2); + XCTAssertEqual([elements.firstObject asyncdisplaykit_node], label); + XCTAssertEqual([elements.lastObject asyncdisplaykit_node], button); +} + +- (void)testThatAccessibilityElementsOverrideWorks { + ASDisplayNode *containerNode = [[ASDisplayNode alloc] init]; + containerNode.frame = CGRectMake(0, 0, 100, 200); + + ASTextNode *label = [[ASTextNode alloc] init]; + label.attributedText = [[NSAttributedString alloc] initWithString:@"test label"]; + label.frame = CGRectMake(0, 0, 100, 20); + + ASButtonNode *button = [[ASButtonNode alloc] init]; + [button setTitle:@"tap me" withFont:[UIFont systemFontOfSize:17] withColor:nil forState:UIControlStateNormal]; + [button addTarget:self action:@selector(fakeSelector:) forControlEvents:ASControlNodeEventTouchUpInside]; + button.frame = CGRectMake(0, 25, 100, 20); + + [containerNode addSubnode:label]; + [containerNode addSubnode:button]; + containerNode.accessibilityElements = @[ label ]; + + // force load + __unused UIView *view = containerNode.view; + + NSArray *elements = [containerNode accessibilityElements]; + XCTAssertTrue(elements.count == 1); + XCTAssertEqual(elements.firstObject, label); +} + @end From bbfb9acd7ccd453a9a8b4d54c6f4d7f72cac005c Mon Sep 17 00:00:00 2001 From: ricky cancro Date: Wed, 22 Apr 2020 16:17:10 -0700 Subject: [PATCH 2/3] comment tweak --- Source/Details/_ASDisplayViewAccessiblity.mm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Details/_ASDisplayViewAccessiblity.mm b/Source/Details/_ASDisplayViewAccessiblity.mm index 577467ede..fa90666dd 100644 --- a/Source/Details/_ASDisplayViewAccessiblity.mm +++ b/Source/Details/_ASDisplayViewAccessiblity.mm @@ -261,7 +261,7 @@ - (void)setAccessibilityElements:(NSArray *)accessibilityElements ASDisplayNodeAssertMainThread(); // While it looks very strange to ignore the accessibilyElements param and set _accessibilityElements to nil, it is actually on purpose. // _ASDisplayView's accessibilityElements method will always defer to the node for accessibilityElements when _accessibilityElements is - // nil. Calling setAccessibilityElements on _ASDisplayView is basically us clearing the cache and forcing _ASDisplayView to ask the node + // nil. Calling setAccessibilityElements on _ASDisplayView is basically clearing the cache and forcing _ASDisplayView to ask the node // for its accessibilityElements the next time they are requested. _accessibilityElements = nil; } From 4ad54a84e12beab82e8156968135ff3a77c4e05f Mon Sep 17 00:00:00 2001 From: ricky cancro Date: Thu, 23 Apr 2020 11:03:05 -0700 Subject: [PATCH 3/3] Fix flakey test --- Tests/ASViewControllerTests.mm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Tests/ASViewControllerTests.mm b/Tests/ASViewControllerTests.mm index ede0ab144..929ee67b3 100644 --- a/Tests/ASViewControllerTests.mm +++ b/Tests/ASViewControllerTests.mm @@ -77,8 +77,9 @@ - (void)testThatViewControllerFrameIsRightAfterCustomTransitionWithNonextendedEd nav.delegate = navDelegate; window.rootViewController = nav; [window makeKeyAndVisible]; - [[NSRunLoop mainRunLoop] runMode:NSDefaultRunLoopMode beforeDate:[NSDate date]]; + [[NSRunLoop mainRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:2]]; [nav pushViewController:vc animated:YES]; + [[NSRunLoop mainRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:2]]; [self waitForExpectationsWithTimeout:2 handler:nil];