patternfly / patternfly-elements

PatternFly Elements. A set of community-created web components based on PatternFly design.
https://patternflyelements.org/
MIT License
375 stars 85 forks source link

[bug] `<pf-switch />` - Switch lacks a static accessible label #2754

Open adamjohnson opened 2 months ago

adamjohnson commented 2 months ago

Description of the issue

<pf-switch /> lacks an accessible static label. The current label uses the state of the switch as the label.

Impacted component(s)

Steps to reproduce

Basic Demo

  1. Go to the <pf-switch /> "Basic" demo.
  2. When clicking the switch, the message changes from "Message when on" to "Message when off". These messages incorrectly act as the label for the switch.

Without Label Demo

  1. Go to the "Without Label" demo.
  2. This demo lacks an accessible label and accessible states for its controls.

Other demos

Similarly, the "Checked with label" and "Disabled" follow these trends and need fixed.

Expected behavior

All <pf-switch /> elements should include an accessible label for the control.

  1. They should also have an aria-describedby attribute that targets the controls toggled state options.
  2. If a <label> isn't used, the switch needs an accessible-label property/attribute.

Proposed fixes:

<label for="with-label">Switch B</label>
<pf-switch id="with-label" aria-describedby="b" checked>
  <div id="b">
    <span data-state="on">Message when on</span>
    <span data-state="off" hidden>Message when off</span>
  </div>
</pf-switch>
<pf-switch aria-describedby="a" accessible-label="Switch A" checked>
  <div id="a">
    <span data-state="on">Message when on</span>
    <span data-state="off" hidden>Message when off</span>
  </div>
</pf-switch>

Related to https://github.com/RedHat-UX/red-hat-design-system/pull/1513#issuecomment-2062119853.

bennypowers commented 2 months ago

Since switch is a FACE, we can just associate it with a label, and we'll be good to go.

We already do this, just that when the state changes, we check for specially marked children of that label and hide or show then depending on their attributes

Fixing this issue should simplify the implementation of switch as well, because we could simply provide a pair of slots (with complimentary attributes) for the descriptions

This is all dependent on the upstream design

<label for=a>label!</label>
<pf-switch id=a>
  <span slot="off">Off!</span>
  <span slot="on">On!</span>
</pf-switch>