Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ASDisplayNode] Implement accessibilityViewIsModal #1858

Merged
merged 5 commits into from
Jun 1, 2020
Merged

[ASDisplayNode] Implement accessibilityViewIsModal #1858

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
353ec60
[ASDisplayNode] Implement accessibilityViewIsModal
rcancro May 28, 2020
54ca0cf
add some experiment checks
rcancro May 28, 2020
e12766c
fix tests and address jon’s comment
rcancro May 29, 2020
4d2fe0f
Fix tests
rcancro May 29, 2020
0dbc606
remove debug code
rcancro May 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions Source/Details/_ASDisplayViewAccessiblity.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,21 @@ static void CollectAccessibilityElements(ASDisplayNode *node, NSMutableArray *el
return;
}

for (ASDisplayNode *subnode in node.subnodes) {
// see if one of the subnodes is modal. If it is, then we only need to collect accessibilityElements from that
// node. If more than one subnode is modal, UIKit uses the last view in subviews as the modal view (it appears to
// be based on the index in the subviews array, not the location on screen). Let's do the same.
ASDisplayNode *modalSubnode = nil;
for (ASDisplayNode *subnode in node.subnodes.reverseObjectEnumerator) {
if (subnode.accessibilityViewIsModal) {
modalSubnode = subnode;
break;
}
}

// If we have a modal subnode, just use that. Otherwise, use all subnodes
NSArray *subnodes = modalSubnode ? @[ modalSubnode ] : node.subnodes;

for (ASDisplayNode *subnode in subnodes) {
// If a node is hidden or has an alpha of 0.0 we should not include it
if (subnode.hidden || subnode.alpha == 0.0) {
continue;
Expand All @@ -280,7 +294,7 @@ static void CollectAccessibilityElements(ASDisplayNode *node, NSMutableArray *el
} 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) {
} else if (subnode.accessibilityElements.count > 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does anyone have any thoughts around this change? When I made an example app with accessibilityViewIsModal set to YES accessibilityElementCount was 1 when I turned on voice over. However, in the tests it was always 0.

I'm not really sure where/how accessibilityElementCount is being changed in my test app. It isn't a property on NSObject, but a method that should be overwritten (which we aren't doing). This comment also exists in the header of _ASDisplayViewAccessiblity.h, so I'm not sure why we are favoring accessibilityElementCount over accessibilityElements:

// 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.

Copy link
Contributor

@vovasty vovasty May 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as far as I understand we should rely on accessibilityElementCount, but
according to documentation it returns 0 by default. May be default implementation?

- (NSInteger)accessibilityElementCount {
  return [[self accessibilityElements] count];
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just played with this some more, and it doesn't work. I'll focus on a way to make the tests pass and revert this change.

// UIView is itself a UIAccessibilityContainer just add it
[elements addObject:subnode.view];
}
Expand Down
31 changes: 30 additions & 1 deletion Source/Private/ASDisplayNode+UIViewBridge.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1137,6 +1137,22 @@ - (BOOL)_locked_insetsLayoutMarginsFromSafeArea

@implementation ASDisplayNode (UIViewBridgeAccessibility)

// Walks up the view tree to nil out all the cached accsesibilityElements. This is required when changing
// accessibility properties like accessibilityViewIsModal.
- (void)invalidateAccessibilityElements
{
// If we are not caching accessibilityElements we don't need to do anything here.
if (ASActivateExperimentalFeature(ASExperimentalDoNotCacheAccessibilityElements)) {
return;
}

// we want to check if we are on the main thread first, since _loaded checks the layer and can only be done on main
if (ASDisplayNodeThreadIsMain() && _loaded(self)) {
self.view.accessibilityElements = nil;
[self.supernode invalidateAccessibilityElements];
}
}

- (BOOL)isAccessibilityElement
{
_bridge_prologue_read;
Expand Down Expand Up @@ -1306,9 +1322,16 @@ - (BOOL)accessibilityViewIsModal
- (void)setAccessibilityViewIsModal:(BOOL)accessibilityViewIsModal
{
_bridge_prologue_write;
BOOL oldAccessibilityViewIsModal = _getFromViewOnly(accessibilityViewIsModal);
_setAccessibilityToViewAndProperty(_flags.accessibilityViewIsModal, accessibilityViewIsModal, accessibilityViewIsModal, accessibilityViewIsModal);

// if we made a change, we need to clear the view's accessibilityElements cache.
if (!ASActivateExperimentalFeature(ASExperimentalDoNotCacheAccessibilityElements) && self.isNodeLoaded && oldAccessibilityViewIsModal != accessibilityViewIsModal) {
[self invalidateAccessibilityElements];
}
}


- (BOOL)shouldGroupAccessibilityChildren
{
_bridge_prologue_read;
Expand Down Expand Up @@ -1398,7 +1421,13 @@ - (UIBezierPath *)accessibilityPath
- (NSInteger)accessibilityElementCount
{
_bridge_prologue_read;
return _getFromViewOnly(accessibilityElementCount);
NSInteger count = 0;
if _loaded(self) {
count = _view.accessibilityElementCount;
} else {
count = ASDisplayNodeGetPendingState(self).accessibilityElementCount;
}
return count;
}

@end
Expand Down
103 changes: 97 additions & 6 deletions Tests/ASDisplayViewAccessibilityTests.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ - (void)testThatAccessibilityElementsWorks {
// force load
__unused UIView *view = containerNode.view;

NSArray *elements = [containerNode accessibilityElements];
NSArray *elements = [containerNode.view accessibilityElements];
XCTAssertTrue(elements.count == 2);
XCTAssertEqual([elements.firstObject asyncdisplaykit_node], label);
XCTAssertEqual([elements.lastObject asyncdisplaykit_node], button);
Expand All @@ -273,7 +273,7 @@ - (void)testThatAccessibilityElementsOverrideWorks {
// force load
__unused UIView *view = containerNode.view;

NSArray *elements = [containerNode accessibilityElements];
NSArray *elements = [containerNode.view accessibilityElements];
XCTAssertTrue(elements.count == 1);
XCTAssertEqual(elements.firstObject, label);
}
Expand Down Expand Up @@ -301,7 +301,7 @@ - (void)testHiddenAccessibilityElements {
// force load
__unused UIView *view = containerNode.view;

NSArray *elements = [containerNode accessibilityElements];
NSArray *elements = [containerNode.view accessibilityElements];
XCTAssertTrue(elements.count == 1);
XCTAssertEqual(elements.firstObject, label.view);
}
Expand All @@ -328,7 +328,7 @@ - (void)testTransparentAccessibilityElements {
// force load
__unused UIView *view = containerNode.view;

NSArray *elements = [containerNode accessibilityElements];
NSArray *elements = [containerNode.view accessibilityElements];
XCTAssertTrue(elements.count == 1);
XCTAssertEqual(elements.firstObject, label.view);
}
Expand Down Expand Up @@ -376,7 +376,7 @@ - (void)testAccessibilityElementsNotInAppWindow {
[node addSubnode:offScreenNodeX];
[node addSubnode:offScreenNode];

NSArray *elements = [node accessibilityElements];
NSArray *elements = [node.view accessibilityElements];
XCTAssertTrue(elements.count == 3);
XCTAssertTrue([elements containsObject:label.view]);
XCTAssertTrue([elements containsObject:partiallyOnScreenNodeX.view]);
Expand Down Expand Up @@ -427,7 +427,7 @@ - (void)testAccessibilityElementsNotInAppWindowButInScrollView {
[node addSubnode:offScreenNodeX];
[node addSubnode:offScreenNode];

NSArray *elements = [node accessibilityElements];
NSArray *elements = [node.view accessibilityElements];
XCTAssertTrue(elements.count == 6);
XCTAssertTrue([elements containsObject:label.view]);
XCTAssertTrue([elements containsObject:partiallyOnScreenNodeX.view]);
Expand Down Expand Up @@ -488,5 +488,96 @@ - (void)testCustomAccessibilitySort {
XCTAssertEqual(elements[3], node4);
}

- (void)testSubnodeIsModal {

UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 320, 568)];
ASDisplayNode *node = [[ASDisplayNode alloc] init];
node.automaticallyManagesSubnodes = YES;

ASViewController *vc = [[ASViewController alloc] initWithNode:node];
window.rootViewController = vc;
[window makeKeyAndVisible];
[window layoutIfNeeded];

ASTextNode *label1 = [[ASTextNode alloc] init];
label1.attributedText = [[NSAttributedString alloc] initWithString:@"label1"];
label1.frame = CGRectMake(10, 80, 300, 20);
[node addSubnode:label1];

ASTextNode *label2 = [[ASTextNode alloc] init];
label2.attributedText = [[NSAttributedString alloc] initWithString:@"label2"];
label2.frame = CGRectMake(10, CGRectGetMaxY(label1.frame) + 8, 300, 20);
[node addSubnode:label2];

ASDisplayNode *modalNode = [[ASDisplayNode alloc] init];
modalNode.frame = CGRectInset(CGRectUnion(label1.frame, label2.frame), -8, -8);
[node addSubnode:modalNode];

ASTextNode *label3 = [[ASTextNode alloc] init];
label3.attributedText = [[NSAttributedString alloc] initWithString:@"label6"];
label3.frame = CGRectMake(8, 4, 200, 20);
[modalNode addSubnode:label3];
modalNode.accessibilityViewIsModal = YES;

NSArray *elements = [node.view accessibilityElements];
XCTAssertTrue(elements.count == 1);
XCTAssertTrue([elements containsObject:modalNode.view]);
}

- (void)testMultipleSubnodesAreModal {

UIWindow *window = [[UIWindow alloc] initWithFrame:CGRectMake(0, 0, 320, 568)];
ASDisplayNode *node = [[ASDisplayNode alloc] init];
node.automaticallyManagesSubnodes = YES;

ASViewController *vc = [[ASViewController alloc] initWithNode:node];
window.rootViewController = vc;
[window makeKeyAndVisible];
[window layoutIfNeeded];

ASTextNode *label1 = [[ASTextNode alloc] init];
label1.attributedText = [[NSAttributedString alloc] initWithString:@"label1"];
label1.frame = CGRectMake(10, 80, 300, 20);
[node addSubnode:label1];

ASTextNode *label2 = [[ASTextNode alloc] init];
label2.attributedText = [[NSAttributedString alloc] initWithString:@"label2"];
label2.frame = CGRectMake(10, CGRectGetMaxY(label1.frame) + 8, 300, 20);
[node addSubnode:label2];

ASDisplayNode *modalNode1 = [[ASDisplayNode alloc] init];
modalNode1.frame = CGRectInset(CGRectUnion(label1.frame, label2.frame), -8, -8);

ASTextNode *label3 = [[ASTextNode alloc] init];
label3.attributedText = [[NSAttributedString alloc] initWithString:@"label6"];
label3.frame = CGRectMake(8, 4, 200, 20);
[modalNode1 addSubnode:label3];
modalNode1.accessibilityViewIsModal = YES;

ASDisplayNode *modalNode2 = [[ASDisplayNode alloc] init];
modalNode2.frame = CGRectOffset(modalNode1.frame, 0, modalNode1.frame.size.height + 10);

ASTextNode *label4 = [[ASTextNode alloc] init];
label4.attributedText = [[NSAttributedString alloc] initWithString:@"label6"];
label4.frame = CGRectMake(8, 4, 200, 20);
[modalNode2 addSubnode:label4];
modalNode2.accessibilityViewIsModal = YES;

// add modalNode1 last, and assert that it is the one that appears in accessibilityElements
// (UIKit uses the last modal subview in subviews as the modal element).
[node addSubnode:modalNode2];
[node addSubnode:modalNode1];

NSArray *elements = [node.view accessibilityElements];
XCTAssertTrue(elements.count == 1);
XCTAssertTrue([elements containsObject:modalNode1.view]);

// let's change which node is modal and make sure the elements get updated.
modalNode1.accessibilityViewIsModal = NO;
elements = [node.view accessibilityElements];
XCTAssertTrue(elements.count == 1);
XCTAssertTrue([elements containsObject:modalNode2.view]);
}


@end