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

Add WAI-ARIA Roles, States, and Properties to ToggleSwitch #13259

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

rak-phillip
Copy link
Member

@rak-phillip rak-phillip commented Jan 31, 2025

Summary

This adds additional WAI-ARIA Roles, States, and Properties to ToggleSwitch. Noted missing properties and keyboard interactions:

  • role
  • aria-label
  • @keydown.enter - defined as optional, but I opted to include it here

Fixes #12817

Technical notes summary

aria-label is set to onLabel because this should be the primary when working with the switch.

The following patterns specified for the switch pattern in the APG1 have been implemented. Patterns not implemented are not relevant to this PR.

Keyboard Interaction

  • ✅ Space: When focus is on the switch, changes the state of the switch.
  • ✅ Enter (Optional): When focus is on the switch, changes the state of the switch.

WAI-ARIA Roles, States, and Properties

  • ✅ The switch has role switch.
  • ✅ The switch has an accessible label provided by one of the following:
    • ◻️ Visible text content contained within the element with role switch.
    • ◻️ A visible label referenced by the value of aria-labelledby set on the element with role switch.
    • ✅ aria-label set on the element with role switch.
  • ◻️ When on, the switch element has state aria-checked set to true.
  • ◻️ When off, the switch element has state aria-checked set to false.
  • ✅ If the switch element is an HTML input[type="checkbox"], it uses the HTML checked attribute instead of the aria-checked property.
  • ◻️ If a set of switches is presented as a logical group with a visible label, either:
    • ◻️ The switches are included in an element with role group that has the property aria-labelledby set to the ID of the element containing the group label.
    • ◻️ The set is contained in an HTML fieldset and the label for the set is contained in an HTML legend element.
  • ◻️ If the presentation includes additional descriptive static text relevant to a switch or switch group, the switch or switch group has the property aria-describedby set to the ID of the element containing the description.

Areas or cases that should be tested

ToggleSwitch is used in the following components:

  • shell/components/form/Labels.vue
  • shell/edit/provisioning.cattle.io.cluster/index.vue
  • shell/pages/c/_cluster/auth/user.retention/index.vue

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

Footnotes

  1. https://www.w3.org/WAI/ARIA/apg/patterns/switch/

@rak-phillip rak-phillip requested a review from aalves08 January 31, 2025 19:02
@rak-phillip rak-phillip added this to the v2.11.0 milestone Jan 31, 2025
@rak-phillip rak-phillip self-assigned this Jan 31, 2025
Copy link
Member

@aalves08 aalves08 left a comment

Choose a reason for hiding this comment

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

I've taken it for a spin and I checked the presence of the missing elements. Can we fix the focus state for it while we are at it? we don't know if we are focused on the right element or not...

If we can do that on this PR it's one less UI element for us to worry about in the future. The rest LGTM 🙏

Screen.Recording.2025-02-05.at.18.04.37.mov

@rak-phillip
Copy link
Member Author

Can we fix the focus state for it while we are at it? we don't know if we are focused on the right element or not...

@aalves08 good point - I added the focus ring to the element. I originally considered it to be out of scope wrt the issue, but it's not too big of an ask to get this in.

image

@rak-phillip rak-phillip requested a review from aalves08 February 5, 2025 19:18
Copy link
Member

@aalves08 aalves08 left a comment

Choose a reason for hiding this comment

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

LGTM mate! 💪

Screenshot 2025-02-06 at 09 51 17

FYI @IsaSih , fixes #13296

@rak-phillip rak-phillip merged commit 879059c into rancher:master Feb 6, 2025
34 checks passed
@rak-phillip rak-phillip deleted the task/12817-a11y-switch branch February 6, 2025 14:57
@rak-phillip rak-phillip linked an issue Feb 6, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants