Make sure interactive controls are always labeled

[afercia]

Description

Any interactive control (inputs, textarea, select, checkboxes, radio buttons, buttons, etc.) should always be labeled. This is a basic accessibility requirement as interactive controls need to inform users what they're about.

Turns out that many (all?) the base components for interactive controls do not enforce any check for some proper labeling nor hey make their lbel prop a required prop. This opens the door to developers misuse, as it is possible to:

• either pass an empty label prop
• or omit the label prop entirely

In both cases, the control will be unlabeled. Worth reminding that this is not only about the visual text associated with a control (which usually is a <label> element). It's is about the actual accessible name of a control: in case of missing or empty associated <label> element, missing aria-label / aria-labelledby or any other labeling mechanism, the control accessible name is empty.

Being open to misuse, my concern is that these components may potentially contribute to lower the accessibility level of the entire internet, given WordPress is used on millions and millions of web sites.

For no reason, ever, an interactive control should be unlabeled. All these components should enforce proper labeling.

As en example of developers misuse, here's a screenshot from a widely used WordPress plugin for e-commerce:

• A CheckboxControl control is passed an empty label prop, or the prop is omitted entirely.
• The CheckboxControl still renders a <label> element, but it's empty.
• The checkbox is unlabeled.
• Developers added some visible text close to the checkbox, but this text is only a paragraph. Visually, it looks like a label but it's not.

This is very far from ideal. Gutenberg should not allow for unlabeled controls. We can't rely on developers awareness and knowledge. Proper labeling must be enforced by the components themselves. Right now, passing an empty label prop or omitting it entirely doesn't even trigger a warning or error.

Cc @ciampo @mirka

Step-by-step reproduction instructions

• Add some interactive controls somewhere, omitting the label prop:

<CheckboxControl />
<TextControl autoComplete="off" value=""} />

• Observe a checkbox and an input field are rendered with no errors or warning.
• Observe the two controls are unlabeled.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[afercia]

Aside: there's also some inconsistency in the actual HTML output when passing an empty label prop or omitting the prop entirely:

CheckboxControl
It still renders a <label> element, but it's empty.

TextControl
Only renders the input, no <label> element.

This is just to note some code inconsistency. Regardless, controls should always be labeled.

[mirka]

This made me realize that we sometimes don't even mark the label prop as required in documentation/TypeScript, which should be the bare minimum. This part should be a relatively easy and obvious fix.

The next step would be to evaluate whether to add runtime checks to emit console warnings in dev mode. Maybe start with some of the common culprits that tend to go unlabeled.

I also have a feeling that a lot of unlabeled usages in the wild may be a symptom of our components not being customizable enough. For example, a consumer wants a more custom design for their checkbox label. So this may also warrant a little more deeper investigation.

[ciampo]

A runtime warning could work, although we'd need to check for all ways for an input to be labelled (corresponding label associated via for attribute, rendered inside a label element, aria-label attribute, aria-labelledby attribute...)

I'm not sure about adding the label prop mandatory — as Lena suggested, consumers of the component may prefer to go "custom", and would probably be able to label an input correctly without using the label prop (for example, by rendering a component inside a <label> element?)

[afercia]

I just spotted one more case where the editor renders interactive controls (in this case checkboxes) that are completely unlabelled.

Unlabelled controls are, honestly, not acceptable. I'm not using this adjective lightly. It's just not acceptable.

The fact they keep being introduced in the editor proves that developers education is not a solution.

I'll refrain from further considerations, my intent here is not blaming anyone. Let's just acknowledge the fact that many contributors to this project (developers and reviewers) don't know how to properly label interactive controls and have a little knowledge of the most basic HTML. This is a fact, not a personal judgment, and it is proved by the several occurrences of unlabelled controls that have been introduced in the editor across the history of this project.

The point is: how can we remediate to this lack of knowledge?

I'm not sure about adding the label prop mandatory — as Lena suggested, consumers of the component may prefer to go "custom"

Honestly I don't mind about consumers of the component who may want to 'go custom'. Functional customization should not be allowed. The priority here is to make sure that in 100% of the cases all controls are labelled.

and would probably be able to label an input correctly without using the label prop (for example, by rendering a component inside a <label> element?)

@ciampo I'd love to have your optimism about developers being able to correctly label elements but unfortunately that's not what happens in the reality, not even in this project.

The prove of this is in the recently added 'Fonts' modal dialog, where there are buttons and checkboxes that are completely unlabelled because the label prop is missing. See here, here, and here.

Example screenshot:

Yes, these checkboxes do have a correctly associated <label> element but no... the label is completely empty. I'm very uncomfortable with this level of lack of knowledge and accuracy and I think this is not acceptable.

I'd like to be very clear that I'm not willing to blame the authors of this specific implementation. This is just one more case were unlabelled controls have been merged into the codebase. It's a general lack of knowledge in this project that we need to address, in a way or the other. Once again, this proves that these components are open to misuse. I'd consider this a bug and we should fix it soon.

@andrewhayward @joedolson @alexstine can we please set some higher priority to this issue? Thanks.

[alexstine]

I think the whole problem here is we are giving developers too much choice. There should have never been a case where we allow components to render a custom label. Label is required for each component or you can implement your own component. We should not be helping developers create inaccessible solutions. That's kind of the whole point of a components library, to avoid anti-patterns but it seems accessibility isn't always a protected piece of the puzzle. The same is even true for the Autocomplete component, there is no accessible labeling at all. The closest is a property that accepts a React fragment, but none of them are just plain strings. You could pass anything into a React fragment and that's what lead us to creating that function to pull a string out of a React object.

I don't want to blame anyone either, especially the dedicated components team, but there comes a point when you really have to wonder, are we going to force accessibility or keep creating ways for people to ignore it?

Thanks.

[afercia]

That's kind of the whole point of a components library, to avoid anti-patterns

💯 I totally second this point.

[ciampo]

Unlabelled controls are, honestly, not acceptable.

And we can all agree on this point.

Functional customization should not be allowed.
[...]
I think the whole problem here is we are giving developers too much choice. There should have never been a case where we allow components to render a custom label.

Having a certain degree of flexibility in how a component can be used can have its value, too (as @mirka also mentioned above).

The risk of not allowing folks to add custom labels is that folks will stop using these components when they don't fit their use case, and will resort to simply using another solution (likely as inaccessible, or even worse).

It's a general lack of knowledge in this project that we need to address, in a way or the other. Once again, this proves that these components are open to misuse. I'd consider this a bug and we should fix it soon.

As explained above, we can certainly make some changes, like marking all label props as required. But I don't personally believe that adding runtime checks is a good idea, as also recently discussed in #56231 (reply in thread).

I also believe that components will always be open to misuse, in one way or another (for example, folks could disable TypeScript and ESlint errors, or simply ignore the console errors).

IMO the best tools that we have to enforce this kind of a11y patterns are tests.

Curious to hear if @andrewhayward has any suggestions on this.

That's kind of the whole point of a components library, to avoid anti-patterns

I kindly disagree. It is definitely one aspect, but IMO not the whole point.

...but it seems accessibility isn't always a protected piece of the puzzle.

Let's try to keep a collaborative approach, and remember that we are all working on this project because we share the common goal of improving the WordPress editor.

Accessibility is an important piece of a complex puzzle together with many other aspects. Every decision that we take usually involves many compromises.

[andrewhayward]

From an accessibility perspective, I'm very much in the camp that we should require labels for all interactive controls. This subject is widely covered by research and specifications, and is a vital part of understanding user interfaces.

• SC 2.4.6: Headings and Labels
• SC 2.5.3: Label in Name
• SC 3.3.2: Labels or Instructions

From a developer perspective, as @mirka suggested, this would be easiest to implement by marking label as required in documentation and type definitions. Of course, it's harder to ensure that such values aren't empty, and impossible to check that the label actually makes sense! As such, it might also be good to provide alternatives for other ways of labelling, such as using aria-labelledby, or potentially letting someone use their own <label>.

interface InputBase {
	// ...
};

interface InputWithExplicitLabel extends InputBase {
	label: string;
	hideLabel?: boolean;
};

interface InputWithImplicitLabel extends InputBase {
	label: RefObject<HTMLLabelElement>;
}

interface InputWithLabelledBy extends InputBase {
	labelledBy: RefObject<HTMLElement>;
};

type Input = InputWithExplicitLabel | InputWithImplicitLabel | InputWithLabelledBy;

type TextInput = Input & {
	// ...
};

type SelectInput = Input & {
	// ...
}

// etc

Providing a clear and easy way for some controls to hide their label might be a reasonable middle ground here. While I'm not a fan of this approach, as every input should ideally have a visible label, I appreciate that it's better to have a label than not, particularly if it means developers don't instead jump to custom solutions.

[mirka]

I also have a feeling that a lot of unlabeled usages in the wild may be a symptom of our components not being customizable enough. For example, a consumer wants a more custom design for their checkbox label.

I believe the newest example that @afercia found is definitely a symptom of this — we failed to provide sufficient documentation/APIs to make it easy enough to implement that custom checkbox layout in an accessible way.

Before considering any kind of runtime checks, I think we can start there. Make it easier to do the right thing. That starts with TypeScript, documentation, and ensuring that there are APIs/examples to easily ensure correct labeling in custom implementations.

[annezazu]

Hey folks! I've removed the high priority label from this issue as part of a broader sweep of the label. For context, the high priority label needs to be used for top issues that need quick attention. In this case, this is bringing to the surface a very important part of the project but the current solution, from what I'm reading, is more in the realm of the following efforts which is more of an ongoing, consistency issue rather than a need for quick mobilization to fix a top concern:

Make it easier to do the right thing. That starts with TypeScript, documentation, and ensuring that there are APIs/examples to easily ensure correct labeling in custom implementations.

If this feels off, please just let me know! I'm doing this sweep to make the high priority label as actionable and impactful as possible and want to get this right.

[alexstine]

I think it is still high priority. Every day that this goes unaddressed could mean regressions in our project or others using the components package incorrectly in plugins. For us, we can fix. For 3rd-party developers, it's out of our hands.

[joedolson]

Do we have a label that can mean "this is very important" without meaning "this requires immediate action"? Because this is absolutely a high priority issue, but I agree that it doesn't align with the type of urgency you're describing.

But our labeling should have some mechanism to identify longer-term topics that have high priority.

[mirka]

I think it's time we start adding some eslint rules for this. Starting with RangeControl at #63874