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.
TextureGroup · Aug 24, 2018 · 1d31fc2 · 1d31fc2
1 parent 31cb65b
commit 1d31fc2
Show file tree

Hide file tree

Showing 7 changed files with 72 additions and 36 deletions.
diff --git 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 = "<group>"; };
 		E5E2D72F1EA780DF005C24C6 /* ASCollectionGalleryLayoutDelegate.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; lineEnding = 0; path = ASCollectionGalleryLayoutDelegate.mm; sourceTree = "<group>"; 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 = "<group>"; };
 		F711994D1D20C21100568860 /* ASDisplayNodeExtrasTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASDisplayNodeExtrasTests.m; sourceTree = "<group>"; };
 		FA4FAF14200A850200E735BD /* ASControlNode+Private.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "ASControlNode+Private.h"; sourceTree = "<group>"; };
 		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 = "<group>"; };
@@ -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
@@ -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
@@ -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
@@ -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
@@ -18,6 +18,8 @@
 #import <Foundation/Foundation.h>
 #import <AsyncDisplayKit/_ASDisplayView.h>
 
-@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
@@ -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
@@ -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 <XCTest/XCTest.h>
+
+#import <AsyncDisplayKit/ASDisplayNode.h>
+#import <AsyncDisplayKit/ASDisplayNode+Beta.h>
+
+@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