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(checkbox): use a native input to fix a11y issues with axe and screen readers #22402

Merged
merged 44 commits into from Nov 12, 2020

Conversation

brandyscarney
Copy link
Member

@brandyscarney brandyscarney commented Oct 28, 2020

Dev build: 5.5.0-dev.202011111928.7c95d89 🚨 This dev build includes #22477 for testing of both at the same time

⚠️ Depends on #22352 ⚠️

Fixes #21644
Fixes #20517
Fixes #17796

If this ends up working well for Checkbox, this can also be used for the following issues:

How this updates a11y and screen readers

Axe

No longer throws errors for aria attributes or buttons without content

NVDA

Tested in Edge and Chrome, reads as:

check box  checked    Default
check box  not checked    Primary
check box  checked    Tertiary
check box  checked    Success
check box  checked    Warning
check box  checked    Dark
check box  checked    Danger
check box  checked    Light
check box  checked    Medium

Checkbox must be in a form to use space to select it with NVDA running, otherwise you need to press Ins + space to enter forms mode, then you can use space to select.

VoiceOver

On Mac:

  • Chrome: properly reads uncheck or check, space bar works, incorrectly announces as checkbox inside of a checkbox (this is a compromise we have to make in order for it to work properly in Safari)
  • Safari: properly reads uncheck or check, space bar works, correctly announces as being on a checkbox

On iOS:

Properly selects checkbox item, double tap will uncheck or check checkbox, skips over disabled checkboxes entirely

Talkback

@github-actions github-actions bot added the package: core @ionic/core package label Oct 28, 2020
@github-actions github-actions bot added the package: react @ionic/react package label Oct 30, 2020
@brandyscarney brandyscarney marked this pull request as ready for review October 30, 2020 21:08
@github-actions github-actions bot added the package: angular @ionic/angular package label Nov 2, 2020
@kensodemann
Copy link
Member

Testing this on my iPhone with VoiceOver on the checkbox does not toggle on double-tap.

@github-actions github-actions bot removed the package: angular @ionic/angular package label Nov 2, 2020
@brandyscarney brandyscarney changed the title fix(checkbox): changing button to label fix(checkbox): use a native input to fix a11y issues with axe and screen readers Nov 3, 2020
Copy link
Contributor

@liamdebeasi liamdebeasi left a comment

Choose a reason for hiding this comment

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

Tested on macOS 10.15.6 and found the following issues:

  1. Pressing "Enter" does not toggle the checkbox like it currently does on master. This is probably because using button gave us that interaction for free.

Tested with VoiceOver on iOS 14 and found no issues.

Tested with VoiceOver on macOS 10.15.6 and found the following issues:

  1. When selecting a checkbox on macOS, VoiceOver highlights a corner of the checkbox. On master it highlights the entire item:
branch master
Mask Group-1 Mask Group
  1. VoiceOver reads "You are currently on a checkbox inside of a checkbox" where it previously read "You are currently on a button inside of a checkbox" when selecting a checkbox. I don't know which one is correct but saying "checkbox inside of a checkbox" sounds confusing.
branch master
Mask Group2 Mask Group2-1

Tested with TalkBack on Android 7.0 and found the following issue:

  1. After double tapping to check the checkbox, Chrome no longer highlights the entire item:
branch master
screen2 screen3

Initially, Chrome only highlights the checkbox and not the entire item, but this is consistent across the branch and master.

Tested with NVDA on Windows 10 and found the following issues:

  1. Tabbing to a checkbox and pressing "Enter" dismisses the modal. This is likely the same underlying issue as the first issue listed in this comment.

  2. NVDA always reads the checkbox state as "not checked" even if it is checked.

NVDA does not seem to read the checkbox state when toggling the checkbox with Space, but that seems to be consistent across the branch and master.

@github-actions github-actions bot removed the package: react @ionic/react package label Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: core @ionic/core package
Projects
None yet
3 participants