Skip to content

Conversation

@zeroedin
Copy link
Contributor

@zeroedin zeroedin commented Mar 5, 2024

What I did

  1. Implementation of InternalsController
  2. Updates label API to a single <label> with children having the data-state

Testing Instructions

  1. View the deploy preview

Notes to Reviewers

@changeset-bot
Copy link

changeset-bot bot commented Mar 5, 2024

🦋 Changeset detected

Latest commit: 12a308f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@patternfly/elements Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added functionality Functionality, typically pertaining to the JavaScript. work in progress POC / Not ready for review labels Mar 5, 2024
@netlify
Copy link

netlify bot commented Mar 5, 2024

Deploy Preview for patternfly-elements ready!

Name Link
🔨 Latest commit 0bfeb3b
😎 Deploy Preview https://deploy-preview-2702--patternfly-elements.netlify.app/

To edit notification comments on pull requests, go to your Netlify site settings.

@github-actions github-actions bot added the AT passed Automated testing has passed label Mar 5, 2024
@github-actions github-actions bot added the tests Related to testing label Mar 21, 2024
@github-actions github-actions bot added the demo Updating demo pages label Mar 21, 2024
@hellogreg
Copy link
Contributor

hellogreg commented Mar 22, 2024

So, here's the current a11y state of the switch at patternflyelements.org (PFE) and in the deploy preview (DP). It's kind of a pick-your-poison with them.

TLDR:

  • Both work great with keyboard, mouse, and touchscreen for non-users of assistive tech (though PFE needs to quell the page scroll upon spacebar press).
  • DP version works just as expected with Mac and iOS Safari/VoiceOver. PFE version has label issue (reads both the on and off versions of the label).
  • Both operate pretty well with Android Chome/Talkback, but the edge goes to PFE for announcing toggle state changes.
  • Both have issues with Windows browser and screen reader combos. Neither NVDA nor JAWS recognize the switches as form elements. The PFE version is slightly better with NVDA once coerced into working, but both still have issues.

patternflyelements.org

Without screen reader

  • Mouse: works as expected in all browsers.
  • Keyboard: pressing space activates switch (good) and scrolls page (bad).

Mac Safari/VoiceOver

  • Good: Keyboard operable, announces checkbox role and state.
  • Issue: Always announces both labels: "Message when on. Message when off."

Win Chrome/JAWS

  • Issue: Neither labeled nor unlabeled versions work by default. JAWS does not enter forms mode upon the switch receiving focus. There are ways to force JAWS into forms mode (by using the next/previous checkbox hotkeys). However, even in those cases, the switch toggles visually but does not announce on/off state.

Win Firefox/NVDA

  • Good: Keyboard operable, announces checkbox role and state when receiving focus.
  • Issue: Only announces state change when switched on (checked), and not when turned off (unchecked).

iOS Safari/VoiceOver

  • Issue: Same issue as Mac safari with the dual labels, but otherwise works well.

Android Chrome/Talkback

  • Good: Works as expected. Touch operable and announces checkbox role, state, and label.
  • Issue: Announces the SVG in the switch (which should be hidden from assistive tech) as an unlabeled image.

Current deploy preview

Without screen reader

  • Mouse: works as expected in all browsers.
  • Keyboard: works as expected in all browsers.

Mac Safari/VoiceOver

  • Good: Works as expected. Keyboard operable and announces switch role, state, and label.

Win Chrome/JAWS

  • Issue: Neither labeled nor unlabeled versions work by default. JAWS does not enter forms mode upon the switch receiving focus. There are ways to force JAWS into forms mode (by using the next/previous checkbox hotkeys). However, even in those cases, the switch toggles visually but does not announce on/off state.

Win Firefox/NVDA

  • Good: No label version is keyboard operable, announces role, and announces when switched on and off (except when it's on by default and you switch it off. In this case, it does not announce the very first switch off, but does subsequent ones.)
  • Issue: The labeled version doesn't work by default. Placing focus on the switch causes NVDA to announce the "section" role. Switch can then only be triggered after manually setting NVDA to forms mode. So NVDA is not recognizing this element as an interactive form element.

iOS Safari/VoiceOver

  • Good: Works as expected.

Android Chrome/Talkback

  • Good: Touch operable and announces checkbox role, state, and label upon focus.
  • Issue: Does not announce state change on toggle, though it does occur visually.

@hellogreg
Copy link
Contributor

Also wanted to mention that the DP version is better than PFE in Mac/iOS Safari because of some updates @zeroedin made yesterday. If needed (as in, if we're not yet ready to go with this new version of switch), maybe those same fixes could be made to the live PFE version, too--fixing the label issue, the page scroll issue, and hiding the SVG from assistive tech.

@bennypowers
Copy link
Member

@hellogreg did 5dbdb14 help?

@hellogreg
Copy link
Contributor

hellogreg commented Mar 25, 2024

Looks like it could be getting a little closer, but still has issues.

Still works fine in Safari/VO.

Chrome/Jaws:

  • The labeled switch can be activated via keyboard after initially receiving focus. However, the screen reader doesn't announce state change. It just announces the key that was pressed ("space" or "enter").
  • The unlabeled switch does not work on the demo page. Activating it either does nothing or occasionally toggles the labeled switch.
    Update: Just realized that part of this issue may be a current Chrome bug with screen readers.

Firefox/NVDA

  • The unlabeled switch works as expected, toggling and announcing state change.
  • Pressing space or enter on the labeled switch always toggles the unlabeled switch. The inverse problem of the issue that sometimes occurs with Chrome/JAWS!

@github-actions github-actions bot added the styles An issue or PR pertaining only to CSS/Sass label Mar 27, 2024
Copy link
Member

@bennypowers bennypowers left a comment

Choose a reason for hiding this comment

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

This should get a major changeset because the label API changed, no? unless i'm mistaken and we already did that

@zeroedin
Copy link
Contributor Author

This should get a major changeset because the label API changed, no? unless i'm mistaken and we already did that

You are correct we changed the label API by adding the <span> instead of multiple labels. This should get a major.

@zeroedin zeroedin marked this pull request as ready for review March 27, 2024 17:11
@github-actions github-actions bot added the doc label Mar 27, 2024
@bennypowers
Copy link
Member

@nikkimk or @hellogreg if we could get a final sign off please

@hellogreg hellogreg self-requested a review March 27, 2024 23:35
Copy link
Contributor

@hellogreg hellogreg left a comment

Choose a reason for hiding this comment

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

Lasting Greek Tragedy: Medea.

Works in all tested browser and screen reader combos (desktop and mobile).

@bennypowers bennypowers merged commit 67eb59c into main Mar 28, 2024
@bennypowers bennypowers deleted the fix/switch-internals-accessibility branch March 28, 2024 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AT passed Automated testing has passed demo Updating demo pages doc functionality Functionality, typically pertaining to the JavaScript. styles An issue or PR pertaining only to CSS/Sass tests Related to testing work in progress POC / Not ready for review

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants