[ASDisplayNode] Ensure accessibilityElementsHidden is respected before returning accessibilityElements to UIKit. by fruitcoder · Pull Request #795 · TextureGroup/Texture

fruitcoder · 2018-02-07T21:34:17Z

Check whether node has accessibilityElementsHidden set before returning accessibilityElements to UIKit

# Conflicts: # Source/Details/_ASDisplayViewAccessiblity.mm # Source/Private/ASDisplayNode+UIViewBridge.mm # Source/Private/_ASPendingState.mm

ghost · 2018-02-08T11:59:42Z

🚫 CI failed with log

appleguy

Thanks for implementing this!

appleguy · 2018-02-09T19:05:44Z

-//    XCTAssertEqualObjects(@"Awesome things will happen", node.accessibilityAttributedHint.string, @"accessibilityAttributedHint is broken %@", hasLoadedView);
-//    XCTAssertEqualObjects(@"1 of 2", node.accessibilityAttributedValue.string, @"accessibilityAttributedValue is broken %@", hasLoadedView);
-//  }
+   if (@available(iOS 11.0, *)) {


This might break some build systems on Xcode 8.3. We'll adopt this soon, but it should be done as a single diff rather than in smaller changes. Could you keep AS_AT_LEAST_IOS11 for now?

I have no idea where this comes from tbo. I know I had that when submitting my last PR but you changed it and I thought pulling from master would keep your changes. Will change this back next week!

appleguy · 2018-02-19T23:57:55Z

Thanks @fruitcoder! I'll merge this as soon as the CI lets me...

appleguy · 2018-02-21T07:13:27Z

@maicki @Adlai-Holler it appears there are ongoing reliability problems with CI. Is there any way I can be added as an admin for this project in order to be able to trigger build restarts? It's definitely the top issue affecting developer velocity for GitHub contributors right now.

If you guys can push this without waiting on CI or otherwise get it to run, it would be great to close this out.

maicki

LGTM

maicki · 2018-02-21T17:17:40Z

@garrettmoon Can you help out with the question from @appleguy

appleguy · 2018-02-26T08:57:50Z

@garrettmoon I know you're very busy right now - no worries if it takes a little while to get to this. We do have a whole lot of PRs blocked on CI though, and it would be good to at least disable the blocking aspect of it to allow dev to continue if it's not practical to fix the CI soon.

Specifically this would be only for people with authorization to merge. I think we can self-police to make sure things build during the review process, as there would be extra caution used when operating without CI.

fruitcoder · 2018-02-26T14:10:12Z

I just noticed a problem using my fork #a11yHiddenElements with my current project. It might be my setup but after I set a node's accessibilityElementsHidden to true once it won't let me focus any subnode after setting the property back to false. It's like it either cached the flag or UIKit doesn't ask for a11y elements again.

nguyenhuy · 2018-03-21T12:53:28Z

@fruitcoder Did you figure out the problem you mentioned above? Should we go ahead and merge this PR?

fruitcoder · 2018-03-22T08:17:09Z

@nguyenhuy Don‘t merge! I still haven‘t found out why the a11y elements aren‘t coming back when accessibilityElementsHidden is set back to true but I am out of clues why that is 😔 do you have any idea what could cause this?

_{Sent with GitHawk}

Adlai-Holler

Marking as request-changes so we know not to land until we figure this out.

I don't have any bandwidth right now to dig into this with you in the next few days @fruitcoder, sorry! I really wish I did 😞

@maicki has a lot of experience working with accessibility. Maybe he can look at it?

appleguy · 2018-10-21T21:45:12Z

@fruitcoder let us know if you had any other observations here. It would be great to merge if possible, though we should probably close out if there is an investigation needed that we don't have time to pursue.

fruitcoder · 2018-10-22T11:20:17Z

I just tried to run the branch again and the results are the same. After setting the accessibilityElementsHidden to false once they won't come back. This seems to be an issue with isAccessibilityElement as well. The functions that collect the elements aren't called again.
Still no idea why that is and I'm sorry but I won't have the time to look more into this :(

fillky · 2019-05-06T09:33:51Z

Having the same issue in a different project. Was playing around UIAccessibility.post(notification: .layoutChanged, argument: nil) but unfortunately still not working 🙆🏻‍♂️.

@fruitcoder

Most of this code comes from an old PR that @fruitcoder put up TextureGroup#795 2 years ago. When creating our array of accessibilityElements, we need to respect the value of `accessibilityElementsHidden`. If the value of this property changes, we need to invalidate the cached accessibility elements (unless we are in the experiment that doesn’t cache `accessibilityElements`). I created a simple test app and made sure this matched UIKit’s implementation. I also added a test case that changes the value of `accessibilityElementsHidden` and makes sure the proper accessibilityElements are returned.

rcancro · 2020-06-01T21:11:37Z

I was having similar issues with cached accessibility elements when working on accessibilityViewIsModal. Since the accessibility code has changed a bit since 2 years ago, I put up a new PR that adds accessibilityElementsHidden. #1859. If that one lands, I'll close this one.

@fruitcoder

Most of this code comes from an old PR that @fruitcoder put up #795 2 years ago. When creating our array of accessibilityElements, we need to respect the value of `accessibilityElementsHidden`. If the value of this property changes, we need to invalidate the cached accessibility elements (unless we are in the experiment that doesn’t cache `accessibilityElements`). I created a simple test app and made sure this matched UIKit’s implementation. I also added a test case that changes the value of `accessibilityElementsHidden` and makes sure the proper accessibilityElements are returned.

rcancro · 2020-06-03T23:17:44Z

#1859 landed, so hopefully accessibilityElementsHidden works now!

@fruitcoder

) Most of this code comes from an old PR that @fruitcoder put up TextureGroup#795 2 years ago. When creating our array of accessibilityElements, we need to respect the value of `accessibilityElementsHidden`. If the value of this property changes, we need to invalidate the cached accessibility elements (unless we are in the experiment that doesn’t cache `accessibilityElements`). I created a simple test app and made sure this matched UIKit’s implementation. I also added a test case that changes the value of `accessibilityElementsHidden` and makes sure the proper accessibilityElements are returned.

fruitcoder added 3 commits October 26, 2017 16:16

Use XCode 9 API, Clean Up Unused Code

4f3c018

Merge branch 'master' of https://github.com/TextureGroup/Texture

e5d0cec

# Conflicts: # Source/Details/_ASDisplayViewAccessiblity.mm # Source/Private/ASDisplayNode+UIViewBridge.mm # Source/Private/_ASPendingState.mm

Respect accessibilityElementsHidden property

a285be3

fruitcoder mentioned this pull request Feb 7, 2018

Accessibility Elements Are Accessible Even Though Parent Has accessibilityElementsHidden Set To true #694

Open

Update Changelog

f8f251f

appleguy approved these changes Feb 9, 2018

View reviewed changes

appleguy changed the title ~~A11y hidden elements~~ [ASDisplayNode] Ensure accessibilityElementsHidden is respected before returning accessibilityElements to UIKit. Feb 9, 2018

fruitcoder and others added 2 commits February 15, 2018 10:39

Ensure Xcode 8.3 compatibility

af4f27a

Merge branch 'master' into a11yHiddenElements

581e25e

maicki approved these changes Feb 21, 2018

View reviewed changes

Merge branch 'master' into a11yHiddenElements

e5ce73d

Merge branch 'master' into a11yHiddenElements

4a257f8

Adlai-Holler requested changes May 29, 2018

View reviewed changes

rcancro mentioned this pull request Jun 1, 2020

[ASDisplayNode] Implement accessibilityElementsHidden #1859

Merged

rcancro closed this Jun 3, 2020

Uh oh!

Conversation

fruitcoder commented Feb 7, 2018

Uh oh!

ghost commented Feb 8, 2018

Uh oh!

appleguy left a comment

Choose a reason for hiding this comment

Uh oh!

appleguy Feb 9, 2018

Choose a reason for hiding this comment

Uh oh!

fruitcoder Feb 10, 2018

Choose a reason for hiding this comment

Uh oh!

appleguy commented Feb 19, 2018

Uh oh!

appleguy commented Feb 21, 2018

Uh oh!

maicki left a comment

Choose a reason for hiding this comment

Uh oh!

maicki commented Feb 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

appleguy commented Feb 26, 2018

Uh oh!

fruitcoder commented Feb 26, 2018

Uh oh!

nguyenhuy commented Mar 21, 2018

Uh oh!

fruitcoder commented Mar 22, 2018

Uh oh!

Adlai-Holler left a comment

Choose a reason for hiding this comment

Uh oh!

appleguy commented Oct 21, 2018

Uh oh!

fruitcoder commented Oct 22, 2018

Uh oh!

fillky commented May 6, 2019

Uh oh!

rcancro commented Jun 1, 2020

Uh oh!

rcancro commented Jun 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

maicki commented Feb 21, 2018 •

edited

Loading