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

feat(Accessibility): A11y support #1

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

lynguyen883
Copy link

No description provided.

@lynguyen883 lynguyen883 changed the title Feat/a11y support feat(Accessibility): A11y support Jun 21, 2021
@lynguyen883 lynguyen883 self-assigned this Jun 21, 2021
Copy link
Author

@lynguyen883 lynguyen883 left a comment

Choose a reason for hiding this comment

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

This needs a bit more work, looking into it now.

accessible={this.props.accessible ? this.props.accessible : true}
accessibilityLabel={this.props.accessibilityLabel ? this.props.accessibilityLabel : undefined}
accessibilityHint={this.props.accessibilityHint ? this.props.accessibilityHint : undefined}
accessibilityRole={this.props.accessibilityRole ? this.props.accessibilityRole : "alert"}
Copy link
Author

Choose a reason for hiding this comment

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

This doesn't make TalkBack on Android announce it even when the role is alert. Needs deeper investigation here.

Interestingly, the toast itself also doesn't announce the label or hint, but the invisible container does.

Copy link
Author

@lynguyen883 lynguyen883 Jun 22, 2021

Choose a reason for hiding this comment

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

Figured it out, needed an accessibilityLiveRegion statement since RN's accessibility delegate just creates a regular View and defers to live regions to handle announcements.

This still breaks a bit on Android so this isn't ready yet. Issues left:

  • announces the toast on dismissal during the animation, most likely due to rerendering it seems;
  • accessibility container hangs around forever unless visibility state is reset;
  • focus outline is a bit off from container contents due to the default styling in place.

These require slightly larger changes, but overall looks straight-forward. However, these changes could potentially be breaking, and if we want this merged upstream some additional work will be needed after-the-fact to make it fully backwards-compatible.

Copy link
Author

Choose a reason for hiding this comment

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

Expanding on this more, it seems like the most likely (and only) breaking change we'd have to introduce is how the View containing the actual touchable component in ToastContainer is styled such that it only ever expands to fit content size and the content itself drives how large it is. This is because that View will be the main component that a11y tools will interact with wrt e.g. closing the toast, reading out toast messages, etc and it currently has a screen reader focus outline that breaks out of the boundaries of the actual visible toast element.

The rest are internal-only changes that won't affect end-user usage of this, but this particular styling change may affect other users of the package. It may also end up that we can actually get it to announce things properly if we do larger sweeping changes internally to the component hierarchy that would negate this style change, but for now this is the path of least resistance to getting this out the door in time.

lib/ToastContainer.js Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants