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

[ISSUE#1909][MAS1.3.1][Screen Reader-Connect the bot] Voiceover announces the same name on the two control with different role #2012

Merged
merged 6 commits into from
Dec 2, 2019

Conversation

denscollo
Copy link
Contributor

Solves #1909

Description

Fixes how Voiceover announces the same name "Restart Conversation" for the "Restart conversation" button and "Restart conversation" dropdown.

Changes made

We added a new property to the SplitButton component named auxiliaryLabel. This label will add to the "Restart conversation" legend an extra "sub menu" legend to difference the button from the dropdown when the emulator runs under macOS.
This way, Voiceover will read the SplitButton with two different legends as NVDA does on Windows.

Testing

In the following recording, you can see how Voiceover reads the button and the dropdown with two different labels:

splitButton

@coveralls
Copy link

coveralls commented Nov 26, 2019

Coverage Status

Coverage increased (+0.05%) to 68.795% when pulling aa62412 on fix/same-name-for-different-controls into 3ecf25f on master.

Copy link
Contributor

@tonyanziano tonyanziano left a comment

Choose a reason for hiding this comment

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

Some minor changes. Otherwise, I tested on Windows and Mac and it looks good! :)

CHANGELOG.md Outdated Show resolved Hide resolved
@@ -95,7 +103,7 @@ export class SplitButton extends React.Component<SplitButtonProps, SplitButtonSt
</button>
<div className={styles.separator} aria-hidden={'true'} />
<button
aria-label={defaultLabel}
aria-label={auxiliaryLabel}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's a good idea to make this default to the defaultLabel here in case auxiliaryLabel was not passed in.

aria-label={ auxiliaryLabel | defaultLabel }

packages/app/client/src/ui/editor/emulator/emulator.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@tonyanziano tonyanziano left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@tonyanziano tonyanziano merged commit 7019832 into master Dec 2, 2019
@tonyanziano tonyanziano deleted the fix/same-name-for-different-controls branch December 2, 2019 20:11
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.

4 participants