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#1890][Functional - Open Bot Dialog] The focus goes on the disabled controls and functioning by space/return. #1999

Merged
merged 5 commits into from Nov 22, 2019

Conversation

@denscollo
Copy link
Collaborator

denscollo commented Nov 21, 2019

Solves #1890

Description

With these changes, the PrimaryButton and LinkButton elements will be hidden to screen readers while disabled.

In the issue report, the tester says the disabled buttons close the dialogs when pressing space, this happens because the Voiceover navigation mode goes through the elements even if they are not focusable by the keyboard, so the keyboard only follows it when it lands in a focusable element, and as a disabled button is not focusable, the keyboard focus remains in the last focusable element, in the case shown by the tester is the cancel button, so when pressing space bar or enter, the cancel button is clicked. This won't keep happening as Voiceover can't navigate to the Connect button when it is disabled

Changes made

We added an attribute of aria-hidden='true' to the PrimaryButton and LinkButton elements when they have its disabled property set to true, this way the Voiceover screen reader skips them when navigating.

Testing

In the following recording, you can see an example of a LinkButton and a PrimaryButton being hidden from Voiceover while they are hidden.

hiddenbuttons

@coveralls

This comment has been minimized.

Copy link

coveralls commented Nov 21, 2019

Coverage Status

Coverage increased (+0.04%) to 68.706% when pulling c211878 on fix/focus-disabled-buttons-voiceover into d30f363 on master.

Copy link
Contributor

tonyanziano left a comment

Im wondering whether it's better to just disable the click handler for these controls when disabled instead of completely hiding them from the user when disabled.

Consider the scenario in which the user is filling out a form, and the form has a submit button that is disabled until the required form inputs are filled out (the Open Bot dialog for example).

If the user does not know that the submit button exists, because it starts off disabled, how will the user know that the button is there and enabled once he or she is finished filling out the form? I feel like it would be a better experience if the user knew about the disabled button, but it simply did nothing when pressed.

I have asked the a11y team to get back to me about this before approving this PR.

@@ -98,7 +98,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- [1934](https://github.com/microsoft/BotFramework-Emulator/pull/1934)
- [1935](https://github.com/microsoft/BotFramework-Emulator/pull/1935)
- [1936](https://github.com/microsoft/BotFramework-Emulator/pull/1936)

- [1999](https://github.com/microsoft/BotFramework-Emulator/pull/1999)

This comment has been minimized.

Copy link
@tonyanziano

tonyanziano Nov 21, 2019

Contributor

this should go underneath the "Unreleased" header since 4.6.0 has already been released.

denscollo added 2 commits Nov 22, 2019
@denscollo

This comment has been minimized.

Copy link
Collaborator Author

denscollo commented Nov 22, 2019

Hi @tonyanziano ,

What we understood that is happening is that Voiceover (navigation with VO + arrows), navigates through every visible element, even if it is not focusable by the keyboard. Because of this, the keyboard focus follows the VO navigation only when it lands on a place the keyboard can focus, resulting on this case, that when voiceover is placed in the save button, the keyboard focus remains in the cancel button, so when the user press space, he is actually pressing the cancel button.

We noticed that this behavior occurs everywhere, for example when navigating into a Checkbox and then to the text next to it, when pressing space, the Checkbox will be the one checked.
Because of this, to have disabled buttons still being visible to the user, we suggest making it focusable by the keyboard, so it can follow the voiceover navigation and if the user press space it won’t do any action.

Also, this behavior only happens with Voiceover (with VO + arrows). Other screen readers only navigate through visible objects focusable by keyboard, and disabling an element makes this object not focusable by default.

Copy link
Contributor

tonyanziano left a comment

@denscollo thanks for the clarification.

I just tested this out in NVDA, and it seems that in Browse Mode, it also sets narrator focus on disabled items, so the behavior is very similar.

I did some research and it seems that people are torn between both sides, whether disabled items should be focusable or not.

I'm going to go ahead and accept this in the sake of time, and if the a11y team comes back with a differing opinion then we can fix it.

@tonyanziano tonyanziano merged commit 3826e2f into master Nov 22, 2019
2 checks passed
2 checks passed
Emulator-CI-PR #91836 succeeded
Details
license/cla All CLA requirements met.
Details
@tonyanziano tonyanziano deleted the fix/focus-disabled-buttons-voiceover branch Nov 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.