Page MenuHomePhabricator

ToggleSwitch: consider making the label required
Closed, DeclinedPublic

Assigned To
None
Authored By
AnneT
Apr 11 2023, 7:42 PM
Referenced Files
F37099511: image.png
Jun 9 2023, 8:46 PM
F37085856: Captura de pantalla 2023-05-31 a las 11.55.21.png
May 31 2023, 9:57 AM
Restricted File
May 29 2023, 10:31 AM

Description

The ToggleSwitch component has a <label> element that contains the default slot. This means that adding a label is optional. We should consider ways of making the label required.

{F37082303}

One way we could do this is by removing the slot and adding a required label prop, plus a hideLabel prop to make the <label> visually hidden. This is similar to the approach we plan to take with the Field component.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Codesearch shows that no one is using CdxToggleSwitch yet, so if we want to do this, we should do it ASAP as it's a breaking change.

ldelench_wmf moved this task from Inbox to Up Next on the Design-System-Team board.

I'd like to revisit this after spending a lot of time with the new Field and Label components and other input components like Checkbox and Radio. Some thoughts:

  • We currently do not require the label for Checkbox or Radio either - for all 3 components, the label is just the default slot and can be included or not
  • In order to require the label, we would have to change it from a slot to a prop. This changes the API, and means that it can only include plain text
  • If we change this for ToggleSwitch, it probably makes sense to change it for Checkbox and Radio too

Additionally, we decided not to make the label text required in the Field or Label component, instead including a slot for it, to ensure that text formatting could be done easily.

Given all these things, I'm wary of making such a restrictive change. There are other things we could do that are less restrictive and require fewer changes:

  • Remove all examples in the docs of ToggleSwitches without labels
  • In the docs for all 3 binary input components, stress that a semantic label must be included
  • If we need to support the use case of a binary input with no *visible* label, we could add a hideLabel prop to all 3 components to make the label visually hidden (I'm already doing this for Field)

I would propose we do these things instead of making the label a hard requirement.

cc @bmartinezcalvo @CCiufo-WMF (we can also discuss this in the design/eng sync)

@AnneT one thing to keep in mind is that the separation between label and ToggleSwitch should be 16px @spacing-100 while in the current Codex demo is 6px. We should fix it.

Captura de pantalla 2023-05-31 a las 11.55.21.png (650×1 px, 95 KB)

I'd like to revisit this after spending a lot of time with the new Field and Label components and other input components like Checkbox and Radio. Some thoughts:

  • We currently do not require the label for Checkbox or Radio either - for all 3 components, the label is just the default slot and can be included or not
  • In order to require the label, we would have to change it from a slot to a prop. This changes the API, and means that it can only include plain text
  • If we change this for ToggleSwitch, it probably makes sense to change it for Checkbox and Radio too

Additionally, we decided not to make the button text required in the Field or Label component, instead including a slot for it, to ensure that text formatting could be done easily.

Given all these things, I'm wary of making such a restrictive change. There are other things we could do that are less restrictive and require fewer changes:

  • Remove all examples in the docs of ToggleSwitches without labels
  • In the docs for all 3 binary input components, stress that a semantic label must be included
  • If we need to support the use case of a binary input with no *visible* label, we could add a hideLabel prop to all 3 components to make the label visually hidden (I'm already doing this for Field)

I would propose we do these things instead of making the label a hard requirement.

cc @bmartinezcalvo @CCiufo-WMF (we can also discuss this in the design/eng sync)

I wonder if it makes sense to make this required on toggle switches, as well as other form fields like radios and checkboxes from an Accessibility perspective:
https://www.w3.org/WAI/tutorials/forms/labels/

After looking into this some more and discussing with @AnneT, I agree that it does not make sense to add the label as a required prop to the ToggleSwitch or any of the other binary input elements. We should be doing as much as possible to ensure Codex users are conforming to accessibility standards, but turning the label into a prop would be too restrictive. There are also multiple ways to assign a semantic label to an input in HTML.

As an example, interfaces like this (Special:Preferences on mobile web) would not be possible to implement because of the use of other forms of markup (like urls in the label text) and it is semantically labelled using aria-labelledby:

image.png (792×946 px, 125 KB)

I do think there is more that we can do to guide users down the right path though. Some suggestions:

  • Provide clear guidelines and better examples of different labelling options in the Figma Library and in the Codex docs (like we are doing here for the Field component).
  • For any of the binary input elements where the label is technically "required" because it is the default slot: if we detect the slot is empty, there should be a warning generated that goes away if aria-label or aria-labelledby attributes are present. Even better would be to somehow know that a label somewhere else on the page is assigned to the input and remove the warning if that's the case.

After looking into this some more and discussing with @AnneT, I agree that it does not make sense to add the label as a required prop to the ToggleSwitch or any of the other binary input elements. We should be doing as much as possible to ensure Codex users are conforming to accessibility standards, but turning the label into a prop would be too restrictive. There are also multiple ways to assign a semantic label to an input in HTML.

As an example, interfaces like this (Special:Preferences on mobile web) would not be possible to implement because of the use of other forms of markup (like urls in the label text) and it is semantically labelled using aria-labelledby:

image.png (792×946 px, 125 KB)

I do think there is more that we can do to guide users down the right path though. Some suggestions:

  • Provide clear guidelines and better examples of different labelling options in the Figma Library and in the Codex docs (like we are doing here for the Field component).
  • For any of the binary input elements where the label is technically "required" because it is the default slot: if we detect the slot is empty, there should be a warning generated that goes away if aria-label or aria-labelledby attributes are present. Even better would be to somehow know that a label somewhere else on the page is assigned to the input and remove the warning if that's the case.

I was under the impression that this meant making label mandatory but these would be visually hidden, is that correct and doable? That's why I assumed it should be included for a11y reasons.

I think we will need to give people the option of visually hiding the label, but leave it up to them so that they can produce a ToggleSwitch with or without a visible label.

From an accessibility standpoint, it makes sense to make the label required. However, when it comes to implementation, we're concerned that we might unintentionally limit Codex users. Throughout the library we're giving people multiple ways of doing things and putting components together, and we've mostly avoided making things required to prevent situations where someone is doing something in an acceptable way that we didn't anticipate.

For example, with ToggleSwitch, maybe someone wants to just output the switch via the ToggleSwitch component, then use the Label component along with it. Or maybe they have some reason for using an aria-label instead of a <label> element.

Plus, if we make it required, the only way we can do that is by making it a prop. Props can only be strings of text, so we typically do not allow any markup in those strings. This might limit users who need to use things like sub- or superscripts, and might have effects on the translatability of labels. The better solution here is to use a slot, which is just a space in the markup where people can drop a plain string or markup. The limit here is that you cannot make a slot a hard requirement. Chris suggested we throw a warning if the slot is not filled out, which I think is a good solution - it alerts the developer that something might be wrong, but allows them to move forward if they are actually including a label in some other way.

I think we will need to give people the option of visually hiding the label, but leave it up to them so that they can produce a ToggleSwitch with or without a visible label.

From an accessibility standpoint, it makes sense to make the label required. However, when it comes to implementation, we're concerned that we might unintentionally limit Codex users. Throughout the library we're giving people multiple ways of doing things and putting components together, and we've mostly avoided making things required to prevent situations where someone is doing something in an acceptable way that we didn't anticipate.

For example, with ToggleSwitch, maybe someone wants to just output the switch via the ToggleSwitch component, then use the Label component along with it. Or maybe they have some reason for using an aria-label instead of a <label> element.

Plus, if we make it required, the only way we can do that is by making it a prop. Props can only be strings of text, so we typically do not allow any markup in those strings. This might limit users who need to use things like sub- or superscripts, and might have effects on the translatability of labels. The better solution here is to use a slot, which is just a space in the markup where people can drop a plain string or markup. The limit here is that you cannot make a slot a hard requirement. Chris suggested we throw a warning if the slot is not filled out, which I think is a good solution - it alerts the developer that something might be wrong, but allows them to move forward if they are actually including a label in some other way.

That SGTM, thanks for explaining the nuances of it all form dev perspective. Design-wise - @bmartinezcalvo should this be updated in the guidelines for ToggleSwitch? And also curious how this relates to T302320?

And also curious how this relates to T302320?

Ooo, good question - that one fell off my radar.

I think not technically requiring the label will actually free us up to implement a solution for T302320 in the future. I'm not exactly sure how we'd do it, but we will have more flexibility if we have an optional slot for the label rather than a required prop.

I think we will need to give people the option of visually hiding the label, but leave it up to them so that they can produce a ToggleSwitch with or without a visible label.

From an accessibility standpoint, it makes sense to make the label required. However, when it comes to implementation, we're concerned that we might unintentionally limit Codex users. Throughout the library we're giving people multiple ways of doing things and putting components together, and we've mostly avoided making things required to prevent situations where someone is doing something in an acceptable way that we didn't anticipate.

For example, with ToggleSwitch, maybe someone wants to just output the switch via the ToggleSwitch component, then use the Label component along with it. Or maybe they have some reason for using an aria-label instead of a <label> element.

Plus, if we make it required, the only way we can do that is by making it a prop. Props can only be strings of text, so we typically do not allow any markup in those strings. This might limit users who need to use things like sub- or superscripts, and might have effects on the translatability of labels. The better solution here is to use a slot, which is just a space in the markup where people can drop a plain string or markup. The limit here is that you cannot make a slot a hard requirement. Chris suggested we throw a warning if the slot is not filled out, which I think is a good solution - it alerts the developer that something might be wrong, but allows them to move forward if they are actually including a label in some other way.

That SGTM, thanks for explaining the nuances of it all form dev perspective. Design-wise - @bmartinezcalvo should this be updated in the guidelines for ToggleSwitch?

@RHo yes, we should update the Figma component adding the label as component's property so designers can use it as needed. Apart from this, we will need to update the spec recommendations about the use of this label. I guess we will make this change just in the Codex library. Do we want to add this as acceptance criteria in this task?

If we're in agreement that we're not going to make the label a required prop, I think we decline this task and open a new task similar to T339013: Binary inputs: Use the Label component internally for all three binary inputs that covers the following:

  • Adding guidelines in the spec that make it clear a semantic label should be provided somewhere.
  • A warning to the developer when the default (label) slot is empty that goes away if aria-label or aria-labelledby attributes are present.
  • Clearer documentation about these guidelines and the above warning in the docs for each binary input component.

If we're in agreement that we're not going to make the label a required prop, I think we decline this task and open a new task similar to T339013: Binary inputs: Use the Label component internally for all three binary inputs that covers the following:

  • Adding guidelines in the spec that make it clear a semantic label should be provided somewhere.
  • A warning to the developer when the default (label) slot is empty that goes away if aria-label or aria-labelledby attributes are present.
  • Clearer documentation about these guidelines and the above warning in the docs for each binary input component.

SGTM