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

fix(ui-options): subgroup titles in Options are now announnced by Tal… #1714

Conversation

ToMESSKa
Copy link
Contributor

@ToMESSKa ToMESSKa commented Oct 4, 2024

…kBack and iOS VoiceOver

Closes: INSTUI-4235

ISSUE:
Subgroup titles are not read in Options with Android Talkback and iOS Voiceover. It works perfectly on Mac Voiceover and on Windows.

TEST PLAN:

  • open the Options page and go to the second example (teh one with the subgroups)
  • try reading the sub-group titles using Android TalkBack and iOS VoiceOver
  • If possible, also check on Mac and Windows to see if it still works

@ToMESSKa ToMESSKa self-assigned this Oct 4, 2024
Copy link

github-actions bot commented Oct 4, 2024

PR Preview Action v1.4.8
Preview removed because the pull request was closed.
2024-11-05 12:37 UTC

@ToMESSKa ToMESSKa force-pushed the INSTUI-4235_a_11_y_subgroup_titles_are_not_read_with_talkback_in_options branch from ada13fd to 953fcb8 Compare October 4, 2024 13:15
Copy link
Collaborator

@matyasf matyasf left a comment

Choose a reason for hiding this comment

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

Looks good, just a hint, be careful when using DOM globals like navigator, a lot of them are undefined in Node, resulting in errors when using SSR. I've tested this one with our regression testing app and it works OK because Node returns Node.js/22, but this code might break in some SSR edge cases like using Bun (havent tested it) or just this value is used in a mobile browser

Copy link
Collaborator

@matyasf matyasf left a comment

Choose a reason for hiding this comment

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

Can you please use

* A utility module using the [ua-parser-js](https://www.npmjs.com/package/ua-parser-js) browser
instead of custom device detection code? Thanks!

return (
<span
id={this._labelId}
role="presentation"
aria-hidden="true"
aria-hidden={isAndroidOrIOS ? 'false' : 'true'}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please leave a comment detailing why this is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rewrote the code with ua-parser-js and left a comment too

@ToMESSKa ToMESSKa force-pushed the INSTUI-4235_a_11_y_subgroup_titles_are_not_read_with_talkback_in_options branch 2 times, most recently from a0e7284 to 2cca1ce Compare October 8, 2024 14:06
@ToMESSKa ToMESSKa requested a review from matyasf October 9, 2024 07:40
…d by TalkBack and iOS VoiceOver

Closes: INSTUI-4235
@ToMESSKa ToMESSKa force-pushed the INSTUI-4235_a_11_y_subgroup_titles_are_not_read_with_talkback_in_options branch from 2cca1ce to 80feb80 Compare October 9, 2024 09:56
@ToMESSKa ToMESSKa changed the title fix(ui-options): subgroup titles in Options are not announnced by Tal… fix(ui-options): subgroup titles in Options are now announnced by Tal… Oct 11, 2024
@balzss balzss merged commit ebdf8f0 into master Nov 5, 2024
11 checks passed
@balzss balzss deleted the INSTUI-4235_a_11_y_subgroup_titles_are_not_read_with_talkback_in_options branch November 5, 2024 12:37
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