From 6866204b155c0432ba2e07f1f914e74785811988 Mon Sep 17 00:00:00 2001 From: Jia Wern Lim <42153084+jiawernlim@users.noreply.github.com> Date: Fri, 24 Aug 2018 11:41:27 -0700 Subject: [PATCH] Refactored `accessibleElements` to `accessibilityElements` (#1069) * Refactored `accessibleElements` to `accessibilityElements`, and removed the re-definition of the property. With this refactor, the field can now be used as a single access point into the accessibility elements of a view. Also, removing the re-definition of the property in _ASDisplayViewAccessibility.h enables us to make use of the field and its associated helper methods directly from the `UIAccessibilityContainer` API rather than rolling our own implementation. * Added tests for the accessors to ASDisplayView.accessibilityElements. * Commented out tests for older a11y accessors & added relevant warnings. Also added assertions that the getter and setter for the accessibilityElements property are used only on the main thread. --- AsyncDisplayKit.xcodeproj/project.pbxproj | 5 +++ CHANGELOG.md | 1 + Source/ASDisplayNode+Yoga.mm | 2 +- Source/Details/_ASDisplayView.mm | 8 ++-- Source/Details/_ASDisplayViewAccessiblity.h | 8 ++-- Source/Details/_ASDisplayViewAccessiblity.mm | 42 +++++++------------- Tests/ASDisplayViewAccessibilityTests.mm | 42 ++++++++++++++++++++ 7 files changed, 72 insertions(+), 36 deletions(-) create mode 100644 Tests/ASDisplayViewAccessibilityTests.mm diff --git a/AsyncDisplayKit.xcodeproj/project.pbxproj b/AsyncDisplayKit.xcodeproj/project.pbxproj index 30f3f6e94..e916d4bf5 100644 --- a/AsyncDisplayKit.xcodeproj/project.pbxproj +++ b/AsyncDisplayKit.xcodeproj/project.pbxproj @@ -497,6 +497,7 @@ E5E281761E71C845006B67C2 /* ASCollectionLayoutState.mm in Sources */ = {isa = PBXBuildFile; fileRef = E5E281751E71C845006B67C2 /* ASCollectionLayoutState.mm */; }; E5E2D72E1EA780C4005C24C6 /* ASCollectionGalleryLayoutDelegate.h in Headers */ = {isa = PBXBuildFile; fileRef = E5E2D72D1EA780C4005C24C6 /* ASCollectionGalleryLayoutDelegate.h */; settings = {ATTRIBUTES = (Public, ); }; }; E5E2D7301EA780DF005C24C6 /* ASCollectionGalleryLayoutDelegate.mm in Sources */ = {isa = PBXBuildFile; fileRef = E5E2D72F1EA780DF005C24C6 /* ASCollectionGalleryLayoutDelegate.mm */; }; + F3F698D2211CAD4600800CB1 /* ASDisplayViewAccessibilityTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = F3F698D1211CAD4600800CB1 /* ASDisplayViewAccessibilityTests.mm */; }; F711994E1D20C21100568860 /* ASDisplayNodeExtrasTests.m in Sources */ = {isa = PBXBuildFile; fileRef = F711994D1D20C21100568860 /* ASDisplayNodeExtrasTests.m */; }; FA4FAF15200A850200E735BD /* ASControlNode+Private.h in Headers */ = {isa = PBXBuildFile; fileRef = FA4FAF14200A850200E735BD /* ASControlNode+Private.h */; }; /* End PBXBuildFile section */ @@ -1028,6 +1029,7 @@ E5E2D72D1EA780C4005C24C6 /* ASCollectionGalleryLayoutDelegate.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASCollectionGalleryLayoutDelegate.h; sourceTree = ""; }; E5E2D72F1EA780DF005C24C6 /* ASCollectionGalleryLayoutDelegate.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; lineEnding = 0; path = ASCollectionGalleryLayoutDelegate.mm; sourceTree = ""; xcLanguageSpecificationIdentifier = xcode.lang.objcpp; }; EFA731F0396842FF8AB635EE /* libPods-AsyncDisplayKitTests.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = "libPods-AsyncDisplayKitTests.a"; sourceTree = BUILT_PRODUCTS_DIR; }; + F3F698D1211CAD4600800CB1 /* ASDisplayViewAccessibilityTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = ASDisplayViewAccessibilityTests.mm; sourceTree = ""; }; F711994D1D20C21100568860 /* ASDisplayNodeExtrasTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASDisplayNodeExtrasTests.m; sourceTree = ""; }; FA4FAF14200A850200E735BD /* ASControlNode+Private.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "ASControlNode+Private.h"; sourceTree = ""; }; FB07EABBCF28656C6297BC2D /* Pods-AsyncDisplayKitTests.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-AsyncDisplayKitTests.debug.xcconfig"; path = "Pods/Target Support Files/Pods-AsyncDisplayKitTests/Pods-AsyncDisplayKitTests.debug.xcconfig"; sourceTree = ""; }; @@ -1255,6 +1257,8 @@ 058D09C5195D04C000B7D73C /* Tests */ = { isa = PBXGroup; children = ( + F3F698D1211CAD4600800CB1 /* ASDisplayViewAccessibilityTests.mm */, + CC35CEC520DD87280006448D /* ASCollectionsTests.m */, DBC452DD1C5C6A6A00B16017 /* ArrayDiffingTests.m */, AC026B571BD3F61800BBC17E /* ASAbsoluteLayoutSpecSnapshotTests.m */, 696FCB301D6E46050093471E /* ASBackgroundLayoutSpecSnapshotTests.mm */, @@ -2299,6 +2303,7 @@ 4E9127691F64157600499623 /* ASRunLoopQueueTests.m in Sources */, CC4981B31D1A02BE004E13CC /* ASTableViewThrashTests.m in Sources */, CC54A81E1D7008B300296A24 /* ASDispatchTests.m in Sources */, + F3F698D2211CAD4600800CB1 /* ASDisplayViewAccessibilityTests.mm in Sources */, CCE4F9B31F0D60AC00062E4E /* ASIntegerMapTests.m in Sources */, 058D0A3B195D057000B7D73C /* ASDisplayNodeTestsHelper.m in Sources */, 83A7D95E1D446A6E00BF333E /* ASWeakMapTests.m in Sources */, diff --git a/CHANGELOG.md b/CHANGELOG.md index 01ecb97ca..dfd341c4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ - Add an experimental feature that reuses CTFramesetter objects in ASTextNode2 to improve performance. [Adlai Holler](https://github.com/Adlai-Holler) - Add NS_DESIGNATED_INITIALIZER to ASViewController initWithNode: [Michael Schneider](https://github.com/maicki) [#1054](https://github.com/TextureGroup/Texture/pull/1054) - Optimize text stack by removing unneeded copying. [Adlai Holler](https://github.com/Adlai-Holler) +- Renamed `accessibleElements` to `accessibilityElements` and removed the re-definition of the property in ASDisplayView. [Jia Wern Lim](https://github.com/jiawernlim) - Remove double scaling of lineHeightMultiple & paragraphSpacing attributes in ASTextKitFontSizeAdjuster. [Eric Jensen](https://github.com/ejensen) ## 2.7 diff --git a/Source/ASDisplayNode+Yoga.mm b/Source/ASDisplayNode+Yoga.mm index ea9739de8..dc2f27cb7 100644 --- a/Source/ASDisplayNode+Yoga.mm +++ b/Source/ASDisplayNode+Yoga.mm @@ -298,7 +298,7 @@ - (void)calculateLayoutFromYogaRoot:(ASSizeRange)rootConstrainedSize // Reset accessible elements, since layout may have changed. ASPerformBlockOnMainThread(^{ - [(_ASDisplayView *)self.view setAccessibleElements:nil]; + [(_ASDisplayView *)self.view setAccessibilityElements:nil]; }); ASDisplayNodePerformBlockOnEveryYogaChild(self, ^(ASDisplayNode * _Nonnull node) { diff --git a/Source/Details/_ASDisplayView.mm b/Source/Details/_ASDisplayView.mm index 21fdd17f1..92907152a 100644 --- a/Source/Details/_ASDisplayView.mm +++ b/Source/Details/_ASDisplayView.mm @@ -99,8 +99,8 @@ @implementation _ASDisplayView BOOL _inHitTest; BOOL _inPointInside; - NSArray *_accessibleElements; - CGRect _lastAccessibleElementsFrame; + NSArray *_accessibilityElements; + CGRect _lastAccessibilityElementsFrame; _ASDisplayViewMethodOverrides _methodOverrides; } @@ -303,7 +303,7 @@ - (void)addSubview:(UIView *)view [super addSubview:view]; #ifndef ASDK_ACCESSIBILITY_DISABLE - self.accessibleElements = nil; + self.accessibilityElements = nil; #endif } @@ -312,7 +312,7 @@ - (void)willRemoveSubview:(UIView *)subview [super willRemoveSubview:subview]; #ifndef ASDK_ACCESSIBILITY_DISABLE - self.accessibleElements = nil; + self.accessibilityElements = nil; #endif } diff --git a/Source/Details/_ASDisplayViewAccessiblity.h b/Source/Details/_ASDisplayViewAccessiblity.h index 5edd82e23..d69794328 100644 --- a/Source/Details/_ASDisplayViewAccessiblity.h +++ b/Source/Details/_ASDisplayViewAccessiblity.h @@ -18,6 +18,8 @@ #import #import -@interface _ASDisplayView (UIAccessibilityContainer) -@property (copy, nonatomic) NSArray *accessibleElements; -@end +// WARNING: When dealing with accessibility elements, please use the `accessibilityElements` +// property instead of the older methods e.g. `accessibilityElementCount()`. While the older methods +// 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 diff --git a/Source/Details/_ASDisplayViewAccessiblity.mm b/Source/Details/_ASDisplayViewAccessiblity.mm index 1f336ac52..ff812e4ea 100644 --- a/Source/Details/_ASDisplayViewAccessiblity.mm +++ b/Source/Details/_ASDisplayViewAccessiblity.mm @@ -243,7 +243,7 @@ static void CollectAccessibilityElementsForView(_ASDisplayView *view, NSMutableA } @interface _ASDisplayView () { - NSArray *_accessibleElements; + NSArray *_accessibilityElements; } @end @@ -252,43 +252,29 @@ @implementation _ASDisplayView (UIAccessibilityContainer) #pragma mark - UIAccessibility -- (void)setAccessibleElements:(NSArray *)accessibleElements +- (void)setAccessibilityElements:(NSArray *)accessibilityElements { - _accessibleElements = nil; + ASDisplayNodeAssertMainThread(); + _accessibilityElements = nil; } -- (NSArray *)accessibleElements +- (NSArray *)accessibilityElements { + ASDisplayNodeAssertMainThread(); + ASDisplayNode *viewNode = self.asyncdisplaykit_node; if (viewNode == nil) { return @[]; } - - if (_accessibleElements != nil) { - return _accessibleElements; + + if (_accessibilityElements == nil) { + NSMutableArray *accessibilityElements = [[NSMutableArray alloc] init]; + CollectAccessibilityElementsForView(self, accessibilityElements); + SortAccessibilityElements(accessibilityElements); + _accessibilityElements = accessibilityElements; } - NSMutableArray *accessibleElements = [[NSMutableArray alloc] init]; - CollectAccessibilityElementsForView(self, accessibleElements); - SortAccessibilityElements(accessibleElements); - _accessibleElements = accessibleElements; - - return _accessibleElements; -} - -- (NSInteger)accessibilityElementCount -{ - return self.accessibleElements.count; -} - -- (id)accessibilityElementAtIndex:(NSInteger)index -{ - return self.accessibleElements[index]; -} - -- (NSInteger)indexOfAccessibilityElement:(id)element -{ - return [self.accessibleElements indexOfObjectIdenticalTo:element]; + return _accessibilityElements; } @end diff --git a/Tests/ASDisplayViewAccessibilityTests.mm b/Tests/ASDisplayViewAccessibilityTests.mm new file mode 100644 index 000000000..e35d1fa0b --- /dev/null +++ b/Tests/ASDisplayViewAccessibilityTests.mm @@ -0,0 +1,42 @@ +// +// ASDisplayViewAccessibilityTests.mm +// Texture +// +// Copyright (c) 2018-present, Pinterest, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// + +#import + +#import +#import + +@interface ASDisplayViewAccessibilityTests : XCTestCase +@end + +@implementation ASDisplayViewAccessibilityTests + +- (void)testAccessibilityElementsAccessors +{ + // Setup nodes with accessibility info + ASDisplayNode *node = nil; + ASDisplayNode *subnode = nil; + node = [[ASDisplayNode alloc] init]; + subnode = [[ASDisplayNode alloc] init]; + NSString *label = @"foo"; + subnode.isAccessibilityElement = YES; + subnode.accessibilityLabel = label; + [node addSubnode:subnode]; + XCTAssertEqualObjects([node.view.accessibilityElements.firstObject accessibilityLabel], label); + // NOTE: The following tests will fail unless accessibility is enabled, e.g. by turning the + // accessibility inspector on. See https://github.com/TextureGroup/Texture/pull/1069 for details. + /*XCTAssertEqualObjects([[node.view accessibilityElementAtIndex:0] accessibilityLabel], label); + XCTAssertEqual(node.view.accessibilityElementCount, 1); + XCTAssertEqual([node.view indexOfAccessibilityElement:node.view.accessibilityElements.firstObject], 0);*/ +} + +@end