Make WordPress Core

Opened 9 years ago

Last modified 7 weeks ago

#31354 accepted enhancement

Compound settings confusing with screen readers (checkboxes)

Reported by: cheffheid's profile Cheffheid Owned by: joedolson's profile joedolson
Milestone: Future Release Priority: normal
Severity: normal Version: 4.1
Component: Administration Keywords: settings-api core-fields needs-refresh changes-requested
Focuses: accessibility Cc:

Description

There are a number of settings in the admin that are made out of multiple variables. That is, they combine a checkbox/radio button toggle and a value.

When you can see it, it makes sense because it forms a logical sentence and everything works out great.

When you rely on a screen reader, it doesn't really come across as such and there's no link between the different inputs in the setting.

For example:

http://i.imgur.com/0r0lDSn.png

As you tab through the different fields, NVDA (with default settings) will announce:

  • Break comments into page with - checkbox (not) checked.
  • top level comments per page and the - edit fifty.
  • page displayed by default - combo box last collapsed.

We need to come up with a pattern that can be applied to split up these toggle/value inputs and clear up confusion this may create.

It would be nice if the inputs for the actual values stay disabled until the appropriate toggle is enabled as well (similar to how the home/blog static page selectors work).

This issue concerns the following fields:

On /wp-admin/options-discussion.php:

  • Automatically close comments on articles older than X days
  • Enable threaded (nested) comments X levels deep
  • Break comments into pages with X top level comments per page and the X page displayed by default.
  • Comments should be displayed with the X comments at the top of each page

On /wp-admin/network/settings.php

  • Limit total size of files uploaded to X MB

Could use input from accessibility and UI teams on how to best tackle this. :)

Attachments (3)

31354.patch (7.7 KB) - added by anthakkar08 9 years ago.
Patch for the issue
31354.2.patch (4.8 KB) - added by anthakkar08 9 years ago.
patch based on https://core.svn.wordpress.org/trunk/ @ 31465
31354.3.patch (4.8 KB) - added by DrewAPicture 9 years ago.
regenerated from root

Download all attachments as: .zip

Change History (24)

@anthakkar08
9 years ago

Patch for the issue

#1 @anthakkar08
9 years ago

  • Keywords has-patch needs-testing added

Created a patch for this this requires to be tested with both Normal and Multisite installation so please check this out and do share the feedback

This ticket was mentioned in Slack in #accessibility by cheffheid. View the logs.


9 years ago

#3 @anthakkar08
9 years ago

Previous Patch was creating Some conflict so have updated the Patch with the Latest Trunk copy of https://core.svn.wordpress.org/trunk/ @ 31465

#4 follow-up: @Cheffheid
9 years ago

@anthakkar08 Thank you sir! Could you create the patch from the root folder though (Where the src folder and wp-config.php file resides as well)? I'm having some issues applying it on my install.

@DrewAPicture
9 years ago

regenerated from root

#5 in reply to: ↑ 4 @DrewAPicture
9 years ago

Replying to Cheffheid:

@anthakkar08 Thank you sir! Could you create the patch from the root folder though (Where the src folder and wp-config.php file resides as well)? I'm having some issues applying it on my install.

31354.3.patch is .2 regenerated from the project root.

If you can't get a patch to apply via the grunt patch command, you can always attempt to do it manually by downloading the patch to the sub-directory in question, in this case /wp-admin, and applying it there with the patch command e.g.

$ patch -p0 < 31354.2.patch

#6 @Cheffheid
9 years ago

Thank you sir! I'm one of those noobs that uses TortoiseSVN instead of grunt to apply patches, because Windows, and it kept suggesting me the parent folder of where-ever I was trying to apply it. And then failed to apply it, even on /wp-admin.

In any case, the patch is a good start and the subfields seem to enable/disable appropriately based on the checkbox state.

Would still like a good solution for the labels though. @afercia had already suggested splitting them up so the labels can make sense by themselves. Something like this (rough example):

http://i.imgur.com/VB8JmkB.png

I'll solicit for some more feedback from the a11y peoples too. :)

#7 @afercia
9 years ago

I'm not sure playing with JavaScript can really help here, I'd say it would be a possible enhancement to evaluate after the main issue is solved. IMHO the first thing to do here should be using proper labels that make sense also when read out of context and probably restructure a bit the form. Just use Plain Old Semantic HTML (POSH), nothing fancy, consider HTML give us fieldset and legend elements for free and use them when appropriate.

About the first example:

http://i.imgur.com/0r0lDSn.png

  • the checkbox is meant to enable/disable the setting, it's a general "switch" and this should be clear even when the label is read out of context
  • the other two, are sort of "sub settings", it should be clear they're discarded if the main checkbox is not checked

What I would suggest:

fieldset
legend:                 Comments pagination
checkbox + label:       Break comments into pages
label + input field:    Top level comments per page
label + select:         Comments page to display by default: last page|first page
(added) label + select: Comments to display at the top of each page: older comments|newer comments

Consider this other example:
https://cldup.com/doimEsOQKY.png

Here, the labels are:

"Automatically close comments on articles older than"
"days"

The first one doesn't tell me it's an on/off switch, but says something about "older than". Confusing. The second one says just "days", I don't have a clue what it refers to, when read out of context. This second example is a bit more difficult to handle, still thinking about a proper solution :)

I understand this would require some minor UI changes, so it needs UI feedback and I'm pretty confident we could find a good balance between functionality and visual. Any thougths more than welcome.

This ticket was mentioned in Slack in #accessibility by rianrietveld. View the logs.


8 years ago

#9 @rianrietveld
8 years ago

  • Milestone changed from Awaiting Review to Future Release

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


7 years ago

#11 @afercia
7 years ago

  • Keywords settings-api added

This ticket was mentioned in Slack in #core by afercia. View the logs.


7 years ago

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


3 years ago

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


14 months ago

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


12 months ago

#16 @joedolson
12 months ago

  • Keywords core-fields added
  • Owner set to joedolson
  • Status changed from new to accepted

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


4 months ago

#18 @joedolson
4 months ago

  • Milestone changed from Future Release to 6.5

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


7 weeks ago

#20 @joedolson
7 weeks ago

  • Keywords needs-refresh changes-requested added; has-patch needs-testing removed

#21 @joedolson
7 weeks ago

  • Milestone changed from 6.5 to Future Release
Note: See TracTickets for help on using tickets.