PatternFly-React preview: https://patternfly-react-pr-2468.surge.sh

resolved merge conflict. 😄

Thanks for contributing this, @boaz0!

When I modify the first example, and remove aria-label from the component, I see that aria-labelledby is added to the <input> as expected. I also see that id attributes are added to the <input> and also the <span> elements that provide the visible text labels, as expected. 🎉

There are just a couple of updates needed:

• When aria-labelledby is added to the <input>, it should be set to the id value that is defined on the <span> that is visible during the on state.
  • Currently, the attribute is set to the id value assigned to the <input> as shown below:
    <input id="simple-switch" aria-label="" class="pf-c-switch__input" type="checkbox" aria-labelledby="simple-switch" checked="">
  • Instead, it should render like this:
    <input id="simple-switch" aria-label="" class="pf-c-switch__input" type="checkbox" aria-labelledby="simple-switch-on" checked="">
• To align with our core examples, let's remove aria-label from the first example, so that the component is defined like this:

      <Switch
        id="simple-switch"
        label={isChecked ? 'Message when on' : 'Message when off'}
        isChecked={isChecked}
        onChange={this.handleChange}
      />
  

  And then when the html is rendered, this will include the aria-labelledby attribute like the example in core.
  • I think this would be sufficient for aligning with the core examples, but if we do decide to have complete alignment, then it's important to note that the react component examples showing the disabled switch would need to include both an on and off label (currently the disabled examples only include a string for the label that's visible).

Thanks a lot @jgiardino on your input.

• I updated this PR, can you please take a second look?
• I opened PF4 Switch label props breaking change #2600 to address your comment regarding the mismatch between core and react implementations.

@boaz0 will you be addressing issue #2600 here? A separate PR is good too.

separate PR 👍 unless you will merge it tomorrow

@tlabaj @jgiardino @redallen I updated the PR to address #2600 too.

Thanks.

Your changes have been released in:

• @patternfly/react-charts@4.7.3
• @patternfly/react-core@3.81.0
• @patternfly/react-docs@4.9.16
• @patternfly/react-inline-edit-extension@2.9.62
• demo-app-ts@2.14.6
• @patternfly/react-table@2.16.2
• @patternfly/react-topology@2.7.12
• @patternfly/react-virtualized-extension@1.1.95

Thanks for your contribution! 🎉

Thanks @tlabaj @kmcfaul @redallen ! 😄

This introduced a breaking change in a minor version (from v3.80.4 to v3.81.0). Prior to this the label prop was be shown both when on and off. Not it only shows when on and not when off.

We have to explicitly add both label and labelOff even when we don't want to change the text. Was this intended?

Opened an issue #2814