Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch toggle accessibility #906

Closed
afercia opened this issue May 25, 2017 · 7 comments · Fixed by #909
Closed

Switch toggle accessibility #906

afercia opened this issue May 25, 2017 · 7 comments · Fixed by #909
Assignees
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).

Comments

@afercia
Copy link
Contributor

afercia commented May 25, 2017

The "switch toggle", used for example for the "Pending review" setting, would need some improvements. This is the current markup output:

<label class="editor-post-status__row">
	<span>Pending review</span>
	<div class="components-form-toggle">
		<input type="checkbox" class="components-form-toggle__input" id="toggle-2" value="false">
		<label class="components-form-toggle__label" for="toggle-2"></label>
	</div>
</label>

Invalid HTML, two errors:

  • div elements can't be placed inside label elements
  • nested labels are not allowed

I understand this was meant to make the text "Pending review" text change the checkbox state when clicked, unfortunately the FormToggle component already outputs a label (and it's empty).

From an accessibility perspective, screen readers ignore the outer label and take into consideration the inner one. Since it's empty, there's nothing to announce:

screen shot 2017-05-25 at 19 10 09

For sighed users, the state change is indicated just with a color change:

screen shot 2017-05-24 at 09 28 00

screen shot 2017-05-24 at 09 27 53

Yes, the "knob" position changes but in absence of color, or with color perception impairments, users wouldn't know which of the two positions is which. When exactly is "off" or "on"?

I've recently worked on a switch control on another project that is taking inspiration from Google Material design guidelines where these kind of custom controls are well documented. For reference, in a first version they recommended to use visible "on" and "off" text, but then that was deprecated. In the current version, they recommend:

The option that the switch controls, as well as the state it’s in, should be made clear from the corresponding inline label.

That's a bit controversial, as a real label element shouldn't dynamically change its content. For this reasons, our decision was to keep using visible "on" and "off" text, hidden to assistive technologies with aria-hidden="true" because the state change is already announced by screen readers. That was needed for the visual part though. I'm not saying this solution should necessarily be the one Gutenberg will use 🙂 I'd recommend to consider visual improvements too, though.

Aside:
one of the reasons why I'm concerned about the educational value of React, it's because it just happens (and happened also to me a lot of times) to forget the actual HTML output. This code:

<label className="editor-post-status__row">
	<span>{ __( 'Pending review' ) }</span>
	<FormToggle
		checked={ status === 'pending' }
		onChange={ onToggle }
	/>
</label>

doesn't tell me at first sight that I'm actually going to produce invalid HTML. It's wonderfully abstracted and re-usable but so easy to be abused. What when, a few months or years from now, developers that haven't built this component will try to use it? They're not required to actually know what HTML it outputs, they are just supposed to use it otherwise what's the point in re-usability? And if they have to check each time what a component actually does, what's the point in abstracting it? Just thoughts from an old-school guy 😉

@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label May 25, 2017
@youknowriad
Copy link
Contributor

Good points here.

  • The inner label can be easily dropped if no label is provided.
  • We could switch the div to a span if no label is provided too, assuming the label is outside the toggle in this case.

Aside:

one of the reasons why I'm concerned about the educational value of React, it's because it just happens (and happened also to me a lot of times) to forget the actual HTML output.

This is true but not specific to any framework. you have the same issue in VueJS, AngularJS, CycleJS, Preact, .... you name it.

@afercia
Copy link
Contributor Author

afercia commented May 25, 2017

Sure, it relates to any framework. But you're using this one ¯_(ツ)_/¯

Re: labels, as per the WP a11y coding standards, all new code in WordPress must use explicitly associated labels (not wrapping, using for/id attributes).

@aduth
Copy link
Member

aduth commented May 25, 2017

They're not required to actually know what HTML it outputs, they are just supposed to use it otherwise what's the point in re-usability? And if they have to check each time what a component actually does, what's the point in abstracting it?

What you lodge here as a concern I'd instead regard as a beauty of React's composition. Yes, the consumer of the component should ideally not be concerned with its generated output, or at least not exposed to the same risk of "doing it wrong". Consider the alternative: In reinventing the wheel each time we implement a toggle with label, each and every developer (and all future maintainers) become susceptible to these and many other traps regarding correct usage of labels. By isolating this common behavior in the abstraction of a component, we can be certain that not only is the implementation consistent throughout the application, but that when issues do occur, they can be fixed in a single location and future maintainers are thereby forever shielded from encountering the same problem. This is the very purpose of an abstraction: concealing complexity behind an idealized interface.

@afercia
Copy link
Contributor Author

afercia commented May 25, 2017

Still, we're failing to do a very simple, basic, thing: output valid HTML. Abstraction needs education, strict guidelines, and good documentation to achieve the ideal development vision you're describing 🙂 Instead, in a non-ideal world like the one we live in, this kind of misuse happens often. Apparently, even very experienced and skilled developers can be fooled by a high level of abstraction, as just happened in this specific case.
Also, better naming conventions can help. The component name here doesn't really reflect what it does.

As I said, my concern is about the educational value of frameworks. We should also aim to educate new generations of developers and users, plugins and theme authors. Considering the average WordPress audience is generally not super technical, I'm afraid abstracted components will be largely abused and will produce tons of spaghetti-HTML. (and I can say spaghetti because I'm from Italy 😉 )

@afercia
Copy link
Contributor Author

afercia commented May 26, 2017

Worth noting in some of the mockups these switch toggle do have a visible text On/Off:

admin ui inspector

In this case, "Featured Image", for example, should be the real label while On/Off should be just a visible "hint", hidden to assistive technologies.

@aduth
Copy link
Member

aduth commented May 26, 2017

[...] Abstraction needs education, strict guidelines, and good documentation to achieve the ideal development vision you're describing [...]

I don't disagree. And looking upon your specific example with fresh eyes, I think you raise a valid point; that the concealed nature of abstractions do increase the risk of composing invalid structures because we choose to shield ourselves from the complexity of how they are formed. Then how do we use this information to help guide us forward? I agree that your suggestion of improved naming to indicate the contained label could help. Perhaps also avoiding elements which have specific nesting requirements, or using their shallow forms (like label without children), or documenting caveats. Tooling could help surface issues at compile time or in the browser when those invalid structures are present on the page; it shouldn't always have to be the human identifying the problem. But is the path forward to avoid composition? I don't think so, for the reason we should distinguish its micro and macro impact. I'd agree in specific cases like these, the negatives of abstractions and their obscurity are most obvious. To the broader picture though, the points in my previous comment about consistency, predictability, and encapsulation I find to far outweigh them.

I don't mean to perpetuate the argument, but I did want to acknowledge that your concern is a very fair one that we ought to consider carefully.

@aduth
Copy link
Member

aduth commented May 26, 2017

Yes, the "knob" position changes but in absence of color, or with color perception impairments, users wouldn't know which of the two positions is which. When exactly is "off" or "on"?

This is an interesting point maybe @jasmussen could weigh in on. At least with my iOS phone using an abundance of these sorts of toggles, I've become accustomed to the meaning of each state. Are there other ways we could make this more visually clear without necessarily an adjacent text label?

Edit: Maybe a tooltip shown when the toggle is focused or hovered, and when it changes?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes).
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants