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.

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

Make ASEditableTextNode accessible to VoiceOver #1162

Merged
merged 13 commits into from
Nov 10, 2018

Conversation

ay8s
Copy link
Collaborator

@ay8s ay8s commented Oct 4, 2018

Should resolve issues seen in #1135 where VoiceOver is unable to access the text view and correctly read out the value while inputting text.

@ay8s ay8s requested a review from nguyenhuy October 4, 2018 02:43
@ay8s ay8s requested a review from maicki October 17, 2018 00:16
@maicki
Copy link
Contributor

maicki commented Oct 18, 2018

@ay8s Could we try implement the accessibility methods within ASEditableTextNode:

// Return 1 here for the textView
- (NSInteger)accessibilityElementCount
{
  return 1;
}

- (id)accessibilityElementAtIndex:(NSInteger)index
{
  return self.textView;
}

- (NSInteger)indexOfAccessibilityElement:(id)element
{
  return 1;
}

Not sure if this is the right approach yet, but we would make the ASEditableTextNode an accessibility container with this.

@ay8s
Copy link
Collaborator Author

ay8s commented Oct 18, 2018

@maicki Tried a couple of approaches with this.

Making the ASEditableTextNode a container with isAccessibilityElement true, doesn't get it picked up as we don't collect the UIAccessibilityElements for non layerBacked nodes and only add the subnodes view to the array.

With isAccessibilityElement set to false we get the same outcome where only the subnodes view is added to the array.

https://github.com/TextureGroup/Texture/blob/master/Source/Details/_ASDisplayViewAccessiblity.mm#L215-L234

@maicki
Copy link
Contributor

maicki commented Oct 29, 2018

If this lands: #1188 we may can just return the textView within -[ASEditableTextNode accessibilityElements] ASEditableTextNode and therefore avoid having to check explicitly in _ASDisplayViewAccessiblity. cc @ay8s

@maicki
Copy link
Contributor

maicki commented Oct 30, 2018

@ay8s #1188 just landed on master. Could you pull latest and check if my suggestion above would work? Thanks!

@ay8s
Copy link
Collaborator Author

ay8s commented Oct 30, 2018

On it @maicki

@ay8s
Copy link
Collaborator Author

ay8s commented Oct 30, 2018

Working now @maicki. 👍

Copy link
Contributor

@maicki maicki left a comment

Choose a reason for hiding this comment

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

@ay8s Thanks again for working on this. After the requested change let's get it in and sorry for the late response.

CHANGELOG.md Outdated Show resolved Hide resolved
Source/ASEditableTextNode.mm Show resolved Hide resolved
Copy link
Contributor

@maicki maicki left a comment

Choose a reason for hiding this comment

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

@ay8s Thanks for working on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants