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

A11y fix: remove clickable hint from Talkback description when no click listener is set on Balloon #406

Merged

Conversation

matt-clarke-wooliesx
Copy link

🎯 Goal

The goal of this PR is to improve the correctness of screen reader hints for Balloons that do not have a click listener set (via Balloon::setOnBalloonClickListener).

Currently, Talkback will incorrectly say "double-tap to activate" for these Balloons. This change removes that part of the Talkback description without affecting the rest of it.

🛠 Implementation details

Currently setOnBalloonClickListener() always sets a "wrapper/parent" click listener on the Balloon, even when the click listener argument is null. Talkback incorrectly infers from this that the Balloon is clickable, despite it possibly being a no-op click listener.

This change only sets the parent click listener when clicking would likely have some noticeable effect. More specifically, if the onBalloonClickListener argument is non-null or if builder.dismissWhenClicked is true.

android:focusable="true" was set on the balloon wrapper FrameLayout in order to maintain the existing Talkback behavior of reading out the text contents of the Balloon.

✍️ Explain examples

In the example below, tapping the highlighted balloon with Talkback on would currently read out:
"skydoves. Love coffee, music, magic tricks and writing poems. Double-tap to activate."

After this change:
"skydoves. Love coffee, music, magic tricks and writing poems."

Preparing a pull request for review

Ensure your change is properly formatted by running:

$ ./gradlew spotlessApply

Please let me know if you have any requests for improvement for this to be acceptable to merge.

Copy link
Owner

@skydoves skydoves left a comment

Choose a reason for hiding this comment

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

Hey @matt-clarke-wooliesx,
This looks perfect! Thanks for your contribution 😄

@skydoves skydoves merged commit 0cb0392 into skydoves:main Jan 14, 2023
@matt-clarke-wooliesx
Copy link
Author

Thank you @skydoves !

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.

3 participants