Form controls are neither grouped nor named

[karlgroves]

Form controls are neither grouped nor named

• Severity: Medium
• Affected Populations:
  • Blind
  • Cognitively Impaired
• Platform(s):
  • All / Universal
• Components affected:
  • Block Panel
  • Document Panel

Issue description

In the Settings dropdown (both Document and Block panels), loose text is
visually placed next to groups of controls, making the controls'
context visually clear, but not programmatically clear.

Additionally, one specific button (the "Public" button which is styled
as a link) has two <label> elements attached to it, the second
overriding the first and giving non-visual users less context.

Issue Code

    /* Publish options */


    <div class="components-panel__row edit-post-post-visibility">


       <span>Visibility</span>


    <div>


    <button type="button" aria-expanded="false" class="components-button edit-post-post-visibility__toggle is-link">Public</button>


    ...


    <div class="components-panel__row edit-post-post-schedule">


        <label for="edit-post-post-schedule__toggle-13" id="edit-post-post-schedule__heading-13">Publish</label>


        <div>


            <label class="edit-post-post-schedule__label" for="edit-post-post-schedule__toggle-13">Jan 31, 2019 8:34 am Click to change</label>


            <button type="button" id="edit-post-post-schedule__toggle-13" aria-expanded="false" aria-live="polite" class="components-button edit-post-post-schedule__toggle is-link">Jan 31, 2019 8:34 am</button>


        </div>


    </div>


    ...


    /* example section areas */


    <div class="components-base-control editor-color-palette-control">


        <div class="components-base-control__field">


            <span class="components-base-control__label">Background Color</span>


            ...


            <button type="button" aria-label="Color: Pale pink" ...></button>


            ...


            <button type="button" class="...">Clear</button>


        </div>


        ...


    </div>


    ...


    <div class="block-library-image__dimensions">


        <p class="block-library-image__dimensions__row">Image Dimensions</p>


        <div class="...">


            <label ...>Width</label>


            <input type="number" ...>


            ...


            <label ...>Height</label>


            <input type="number" ...>


            ...


            <div aria-label="Image Size" class="components-button-group" role="group">


                <button type="button" ...">25%</button>


                <button type="button" ...>50%</button>


                <button type="button" ...>75%</button>


                <button type="button" ...>100%</button>


            </div>


            <button type="button" class="...">Reset</button>


        </div>


    </div>

Remediation Guidance

For most of the selection areas, turning the wrapping element into a
fieldset and the element with the group name text to a legend is
sufficient.

For the "Visibility" and "Publish" sections, the Recommended Code
shows turning the visible text elements near the buttons into headings
(level h3) and adding additional context to the publish date button
as hidden screen reader text.

Another option available is to addid attributes to the <span>
and <button> elements, then referencing both of them to label the
button using aria-labelledby.

Recommended Code

    /* Publish options */


    <div class="components-panel__row edit-post-post-visibility">


       <h3>Visibility</h3>


    <div>


    <button type="button" aria-expanded="false" class="components-button edit-post-post-visibility__toggle is-link">Public</button>


    ...


    <div class="components-panel__row edit-post-post-schedule">


        <h3 id="edit-post-post-schedule__heading-13">Publish</h3>


        <div>


            <button type="button" id="edit-post-post-schedule__toggle-13" aria-expanded="false" ...">Jan 31, 2019 8:34 am


                <span class="screen-reader-text">Change date</span>


            </button>


        </div>


    </div>


    ...


    /* example section areas */


    <div class="components-base-control editor-color-palette-control">


        <fieldset class="components-base-control__field">


            <legend class="components-base-control__label">Background Color</legend>


            ...<button type="button" aria-label="Color: Pale pink" ...></button>


            ...


            <button type="button" class="...">Clear</button>


        </fieldset>


        ...


    </div>


    ...


    <fieldset class="block-library-image__dimensions">


        <legend class="block-library-image__dimensions__row">Image Dimensions</legend>


        <div class="...">


            <label ...>Width</label>


            <input type="number" ...>


            ...


            <label ...>Height</label>


            <input type="number" ...>


            ...


            <div aria-label="Image Size" class="components-button-group" role="group">


                <button type="button" ...">25%</button>


                <button type="button" ...>50%</button>


                <button type="button" ...>75%</button>


                <button type="button" ...>100%</button>


            </div>


            <button type="button" class="...">Reset</button>


        </div>


    </fieldset>

Relevant standards

• 1.3.1 Info and Relationships
  (Level A)
• 3.3.2 Labels or Instructions
  (Level A)
• 4.1.2 Name, Role, Value
  (Level A)

Note: This issue may be a duplicate with other existing accessibility-related bugs in this project. This issue comes from the Gutenberg accessibility audit, performed by Tenon and funded by WP Campus. This issue is GUT-36 in Tenon's report

[afercia]

Re:

Additionally, one specific button (the "Public" button which is styled
as a link) has two elements attached to it, the second
overriding the first and giving non-visual users less context.

This specific part is already tracked in #11747 and a PR is ready for review: #15381.

[melchoyce]

Example screenshot from the full report:

[aduth]

This was discussed in today's #core-editor triage session (link requires registration):

https://wordpress.slack.com/archives/C02QB2JS7/p1557754759136400

Focus of the discussion was on form fields, not the issue with redundant label tracked already at #11747.

It was discussed whether this could be programmatically enforced as a best-practice from the abstraction of a component (existing, new, or as a prop in configuring an existing component).

Two common components were identified:

• BaseControl
• PanelRow

BaseControl is the likelier candidate for describing combinations of form fields.

Question: Is it fair to say the groupings should only be implemented when multiple inputs exist for the same "label" group?

Assuming the answer to the above is "Yes":

• It would not be appropriate to change all rendered elements of BaseControl to use fieldset in place of div
• Alternatives:
  • Programmatically determine children length as indicator that it should be rendered as a grouping
  • Create a separate component for describing a group of controls, e.g. BaseControlGroup or BaseControl.Group
    • See prior art of BaseControl.VisualLabel for subcomponents of BaseControl
  • Provide a prop to BaseControl to control its rendered output, e.g. group={ true }, fieldTagName="fieldset"

[afercia]

Note: I kept the "Needs Accessibility Feedback" on this issue because I'd like to propose to discuss it during next accessibility meeting. Some of the raised concerns make sense to me e.g.: not all the rendered elements would need to be grouped in a fieldset + legend.

[afercia]

Discussed during today's accessibility bug scrub. As a general considerations, there were concerns on how to address this programmatically. Most of the times the need for a fieldset and legend really depends on the content.

Example from https://accessibility.blog.gov.uk/2016/07/22/using-the-fieldset-and-legend-elements/

Do you need a passport?

• radio button: Yes
• radio button: No

In this case, a fieldset + legend are clearly necessary and benefit all users.

In other cases, see for example the screenshot below, grouping form fields in a fieldset + legend wouldn't be so appropriate. It could also make things worse, as in adding redundant or incorrect information:

In this example, users already know they're in the “Latest Posts Settings”. They have selected the Latest Posts Settings block and navigated to its settings. Some screen readers repeat the legend for each form control. Hearing “Latest Posts Settings” repeated for each form control within this group wouldn't help at all.

[aduth]

Considering the specific examples "Font Size", "Background Color", "Text Color" (the latter two are the same underlying component), they are implemented as:

• FontSizePicker
• ColorPaletteControl

Each uses BaseControl ([1], [2]) with multiple inputs passed as children. While it might not solve all problems in the application, perhaps it may still benefit to resolve instances where the component is used by improving its implementation to be more aware of its own use.

Some common factors which could be used for programmatic determination to consider rendering as a fieldset + legend instead. This operates on an assumption that, for a single rendering of a "control", a div + label is appropriate when it includes a single input field, and a fieldset + legend pair is appropriate if it includes multiple. This also requires that "a control" is well-defined, as in some counter-examples we may consider as separate controls.

• BaseControl.VisualLabel is used, since generally each individual input should have its own labels
• Multiple children (aside from BaseControl.VisualLabel) are passed, instead of the typical single input child
• Those children could be statically analyzed (to an extent) to determine whether multiple input elements are passed

Or, this could be made explicit, as in considered in my previous comment with a group prop or BaseControlGroup or BaseControl.Group component.

In reflecting on the introduction of BaseControl.VisualLabel in #14179, it prompts the question: Do all of the cases where we would reach for this visual label appropriately fall under the use case of a fieldset + legend ? For the video block "Poster Image" addressed there, and indeed above for "Font Size", "Background Color", "Text Color", it seems to me as though it would be appropriate. This could be evidence to suggest that VisualLabel may have been more appropriately implemented as something more like <BaseControl.Group label="Poster Image"> (<fieldset><legend>Poster Image</legend>[Select Poster Image] [Remove Poster Image]</fieldset>).

[afercia]

Some common factors which could be used for programmatic determination to consider rendering as a fieldset + legend instead. This operates on an assumption that, for a single rendering of a "control", a div + label is appropriate when it includes a single input field, and a fieldset + legend pair is appropriate if it includes multiple

I do realize that, from a developer perspective, programmatically determining the way components are rendered and their behavior is desirable. However, this shouldn't happen because of development convenience and at the expenses of users experience.

It really depends on the content. I doubt there's a good way to programmatically address this issue based on the amount of controls.

Adding semantics just for the sake of... having more semantic elements doesn't always help 🙂 It's really about how semantics is communicated by assistive technologies and ultimately perceived by users who use that software. We shouldn't try do add semantics when it can potentially make things worse.

Please allow me to explain how screen readers announce fieldset + legend before I get to some examples. Sorry if this is going to be a bit long.

In accessibility terms, a legend gives a fieldset its accessible name. The fieldset represents a group of logically related form controls. Its accessible name is announced as the name of the controls group.

Not surprisingly, there are inconsistencies across browser/screen reader combos 🙂 Theoretically, the legend should be announced only when entering and exiting the fieldset. However, some combos announce the legend for all the form controls within the fieldset, which adds a lot of noise.

Add to that it also depends on the navigation mode. Some combos announce the legend only when tabbing through form controls (ie. when screen readers are in "forms mode". Instead, they don't when navigating content with the arrows (in "browse mode"). Some screen readers don't even announce "grouping" or "group".

That said, let's use your Poster Image example

<fieldset>
	<legend>Poster Image</legend>

	[Select Poster Image]
	[Remove Poster Image]
</fieldset>

When tabbing, this would be announced (more or less) in the following way. Please consider this varies across different combos.

In the best case:

Poster Image grouping 
Select Poster Image {type and state of the control}
Remove Poster Image {type and state of the control}

In the worst case (with repetition of the legend):

Select Poster Image, {type of control}, Poster Image, group 
Remove Poster Image, {type of control}, Poster Image

In both cases the point is: what is the actual value of hearing the legend "Poster Image" when the controls labels already refer to "Poster Image"? That's a useless repetition. It doesn't add value and there's really nothing else fieldset + legend do.

My point is that it really depends on how the legend and labels text is crafted. There's no way to programmatically determine the quality of the text developers will use for these components.

Consider this other example:

If the text "General" was a fieldset legend, what the added value would be? The checkboxes labels are already clear:

• Enable Pre-publish Checks
• Enable Tips

As a user, hearing "General" in addition to that wouldn't help me at all. We could argue that "General" is not a good term to start with 🙂 but regardless, it's again about the actual combination of text used for the fieldset legend and the controls labels.

Instead, in this other example:

I'd definitely agree that "Document Panels" should be a fieldset legend. Otherwise, when tabbing through the checkboxes, I would hear only:

- Permalink
- Categories
- Tags
- Featured Image
- Excerpt
- Discussion

and have no clue what they refer to. E.g. what is this "Permalink" checkbox, what it does? No clue. Hearing "Document Panels" as name of the group would help me understand what the checkboxes do.

Aside: we could argue that the text "Document Panels" could be improved: there's no verb, what is the underlying action? It should be something like "Enable Document Panels".

Ultimately, it's more about language and communication. Thinking at the fieldset legend + the form controls in terms of question and answers may help.

I don't think this can be addressed programmatically. It's about education and crafting good combinations of text. I'd be more in favor of allowing developers to make conscious, educated, choices by the means of a group prop or similar.

Then, the correct usage of fieldset + legend should be well explained in the documentation, outlining when and when not to use them (https://accessibility.blog.gov.uk/2016/07/22/using-the-fieldset-and-legend-elements/).

[aduth]

In both cases the point is: what is the actual value of hearing the legend "Poster Image" when the controls labels already refer to "Poster Image"? That's a useless repetition. It doesn't add value and there's really nothing else fieldset + legend do.

Could it not be viewed as an issue of labels, where the text of "Select Poster Image" ought to be simplified anyways as being redundant (both visually in text and as announced by accessible technology) with its grouping "Poster Image"?

Otherwise, a reading of the referenced resource could reinforce the assumptions of "single" (radio as exception) vs. "several" as the differentiator in how to apply fieldset and legend, from its "When [not] to use a fieldset and legend" section. Or at least, the recommendations could use further clarification as to identifying what these exceptions should be.

The other examples cited are not what I had in mind as being in scope to the considerations of my prior comment, as they are neither today nor presumably ought to be implemented as a single BaseControl.

If the text "General" was a fieldset legend, what the added value would be? The checkboxes labels are already clear:

Is the concern of added value here the fact more in the fact that "General" is a meaningless categorization? And if so, is that not again (to your point) an issue of the label, and not of its specific usage?

I do realize that, from a developer perspective, programmatically determining the way components are rendered and their behavior is desirable. However, this shouldn't happen because of development convenience and at the expenses of users experience.

My goal is not convenience, but in seeking ways for tools to complement and serve to promote education, to prevent these mistakes from being allowed to be made. If it's not possible do so here, then fair enough. But I'd not want to be discouraged in exploring where these opportunities might exist.

[afercia]

Could it not be viewed as an issue of labels, where the text of "Select Poster Image" ought to be simplified anyways

Sure, in fact my point is really about the combination of the legend text and the labels text. In the poster image example, the following would make more sense:

legend: Poster Image
    label: Select
    label: Remove

See test.

From the referenced resource I'd rather take the point of considering this in terms of question and answers, where the legend gives context to the (short) labels.

However, my concern is that all this is more related to the actual text developers will use. How can we ensure developers will get it right? In my experience, when there's room for abusing some code intended usage, it will be abused. 🙂 Trying to programmatically determine this relies on assumptions that we can't guarantee will be true.

Also, how we can guarantee developers will group together controls that are logically related? What if they group unrelated controls and a fieldset + legend are rendered anyways?

I'm sorry but my intent is not discouraging explorations. However, I already know, because I saw it happening many, many, times, that strong assumptions on intended usage will inevitably lead to incorrect usage and a degraded experience for users.

[sabernhardt]

If/when a design change proposed last week in issue #470 is implemented, that would be better than this. However, here is a quick fix option for the Status and Visibility section, in case it's good enough for the next maintenance release. The label tags are removed in favor of Tenon's h3 heading recommendation (removing margin and bold styles) and non-visual text is added to the button (using text strings that are already translated) for a little context.

If someone wants to make a pull request based on this, I made these edits to the following 3 files in a testing environment.

\packages\edit-post\src\components\sidebar\post-visibility\index.js
starting on line 11:

			<PanelRow className="edit-post-post-visibility">
				<h3>{ __( 'Visibility' ) }</h3>
				{ ! canEdit && <span><PostVisibilityLabel /></span> }
				{ canEdit && (
					<Dropdown
						position="bottom left"
						contentClassName="edit-post-post-visibility__dialog"
						renderToggle={ ( { isOpen, onToggle } ) => (
							<Button
								type="button"
								aria-expanded={ isOpen }
								className="edit-post-post-visibility__toggle"
								onClick={ onToggle }
								isLink
							>
								<span className="screen-reader-text">{ __( 'Visibility:' ) } </span><PostVisibilityLabel /><span className="screen-reader-text"> { __( 'Click to change' ) }</span>
							</Button>
						) }
						renderContent={ () => <PostVisibilityForm /> }
					/>
				) }
			</PanelRow>

\packages\edit-post\src\components\sidebar\post-schedule\index.js
starting on line 12:

			<PanelRow className="edit-post-post-schedule">
				<h3>{ __( 'Publish' ) }</h3>
				<Dropdown
					position="bottom left"
					contentClassName="edit-post-post-schedule__dialog"
					renderToggle={ ( { onToggle, isOpen } ) => (
						<>
							<Button
								id={ `edit-post-post-schedule__toggle-${ instanceId }` }
								type="button"
								className="edit-post-post-schedule__toggle"
								onClick={ onToggle }
								aria-expanded={ isOpen }
								aria-live="polite"
								isLink
							>
								<span className="screen-reader-text">{ __( 'Publish:' ) } </span><PostScheduleLabel /><span className="screen-reader-text"> { __( 'Click to change' ) }</span>
							</Button>
						</>
					) }
					renderContent={ () => <PostScheduleForm /> }
				/>
			</PanelRow>

\packages\edit-post\src\components\sidebar\style.scss
I added this, with the indent, starting at line 76 (after h2,h3):

	.edit-post-post-status h3 {
		margin: 0;
		font-weight: 400;
	}

[tellthemachines]

Regarding modifying BaseControl for use as a wrapper for groups, whether with fieldset or other semantics, my concern is that BaseControl is already being used as a wrapper for discrete input components such as radio-control and select-control, which will make it messy to use as a further wrapping layer for groups because we'll end up with multiple nested BaseControls.
I tend to agree with @afercia that enforcing semantics underneath the hood can easily lead to unsemantic markup, but we could try to enforce semantics in a more visible (and pedagogical?) way by creating a Fieldset component and clearly documenting where it should be used.

It would be nice to have sub-issues for addressing each specific set of controls this issue relates to. Are we waiting to decide on a global approach to then split up into tasks? I'd love to work on this but this is a pretty scary ticket to pick up as a new dev on the project! 😅