Opened 10 years ago
Closed 2 months ago
#31354 closed defect (bug) (fixed)
Compound settings confusing with screen readers (checkboxes)
Reported by: | Cheffheid | Owned by: | joedolson |
---|---|---|---|
Milestone: | 6.7 | Priority: | normal |
Severity: | normal | Version: | 4.1 |
Component: | Administration | Keywords: | settings-api core-fields has-patch commit |
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:
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 (11)
Change History (71)
#1
@
10 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.
10 years ago
#3
@
10 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:
↓ 5
@
10 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.
#5
in reply to:
↑ 4
@
10 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
@
10 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):
I'll solicit for some more feedback from the a11y peoples too. :)
#7
@
10 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:
- 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
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.
9 years ago
This ticket was mentioned in Slack in #accessibility by afercia. View the logs.
8 years ago
This ticket was mentioned in Slack in #core by afercia. View the logs.
8 years ago
This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.
4 years ago
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
2 years ago
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
21 months ago
#16
@
21 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.
13 months ago
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
10 months ago
@
7 months ago
Image of decoupled discussion options fields, need feedback (indents? hidden/disabled state from parent?)
#22
@
7 months ago
Working on this locally. I decoupled the options so there would be explicit check boxes followed by additional related inputs.
I am still uncertain the best way to convey the relation of the 'parent' check box element to the affected sub-selections and whether they should be visible but disabled or hidden.
example:'Automatically close comments for old posts' affects whether 'Number of days to keep old comments' is used, but doesn't necessarily have to affect if it is set.
May pursue the way this is handled on 'Reading Settings' with static page selection using a UL for options and disabling selectors.
This ticket was mentioned in Slack in #accessibility by jwgoedert. View the logs.
7 months ago
This ticket was mentioned in PR #6595 on WordPress/wordpress-develop by @jwgoedert.
7 months ago
#24
- Keywords has-patch added; needs-refresh removed
This patch addresses the difficulty with using input fields inline with text for screen readers.
For example: 'Breaking comments into pages' with inputs on a single line-
Becomes: 'Breaking comments into pages' with a parent checkbox that controls styles(visibility in this case-with a disabled version to come) and displays the options one after the other for better screen reader and tab accessibility.
An 'aria-live' div has been added to announce the state of the checkbox and display or removal of dependent elements.
Trac ticket:
https://core.trac.wordpress.org/ticket/31354
This ticket was mentioned in Slack in #accessibility by jwgoedert. View the logs.
7 months ago
This ticket was mentioned in PR #6659 on WordPress/wordpress-develop by @jwgoedert.
6 months ago
#26
Trac ticket:
This ticket was mentioned in Slack in #accessibility by jwgoedert. View the logs.
6 months ago
@
6 months ago
Disable dependent input fields on parent toggle patch-need feedback on accessibility and structure
#28
@
6 months ago
- Milestone changed from Future Release to 6.6
Adding to 6.6 milestone; this is looking promising. In accessibility bug scrub, we discussed this and prefer the disabled
model of the hidden
model, as it then uses the same pattern as on the Reading settings.
#29
@
6 months ago
- Keywords needs-testing added; changes-requested removed
- Milestone changed from 6.6 to 6.7
We have 2 days before Beta 1 and, objectively, no time to test, so I am moving it into the next milestone.
#30
@
6 months ago
- Milestone changed from 6.7 to 6.6
Moving back to 6.6. This is on my schedule for today and tomorrow.
#31
@
6 months ago
This updated patch removes the ARIA live announcement and the aria-expanded
attributes. The aria-expanded
attribute is not required since the checkboxes do not open and close anything, and their current state is conveyed using the natural semantics of the checkbox input.
The aria-live
announcement isn't needed, in my opinion; it would just make things more verbose. If the change was made to something that preceded the current position in the DOM, it might be necessary, but since the change is to items that follow in the DOM, I think it's fine as is.
Edit: and I also added the disabled
attribute in PHP, so it would apply without JS.
Also changed the CSS & HTML so to reuse the styles from the Reading options screen, updated the function comments to better describe what the added JS now does, and removed the styles that impacted the label design. It's generally considered acceptable for a disabled input to be below color contrast guidelines, but the label still needs to be visible.
#32
@
6 months ago
- Milestone changed from 6.6 to 6.7
While I think this is a highly worthwhile change, I also think it has not had sufficient feedback to go into core this late as an enhancement.
It may be reasonable to re-characterize this as a bug - the existing labeling is a poor experience, but is borderline as a bug. WCAG 3.3.2 requires that fields have labels or instructions that describe the input; and whether that's true or not on these is open to discussion.
I'll raise this issue in a bug scrub to determine whether it's viable to change to a bug for 6.6, but in the meanwhile I'm moving to 6.7.
#33
@
6 months ago
I would have to agree from a screenreader perspective that this falls more under bug territory as the content and quantities to not line up in a discernible way.
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
6 months ago
@
6 months ago
Adjusted previous patch to remove js and php toggle disable setting to focus on structural changes only(bug related)
This ticket was mentioned in PR #6730 on WordPress/wordpress-develop by @jwgoedert.
6 months ago
#37
Nested child elements in list items to create visual hierarchy, moved input fields to end of labels for screen reader usability
Trac ticket:
https://core.trac.wordpress.org/attachment/ticket/31354/
This ticket was mentioned in Slack in #accessibility by rcreators. View the logs.
6 months ago
This ticket was mentioned in Slack in #accessibility by sabernhardt. View the logs.
6 months ago
#40
@
6 months ago
- Milestone changed from 6.6 to 6.7
The new patch does not have testing yet, and it contains new text strings. I'll move the ticket to 6.7 (but it still could move back in the next week if a committer is confident).
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
4 months ago
This ticket was mentioned in Slack in #core-test by ankit-k-gupta. View the logs.
4 months ago
#43
@
4 months ago
Test Report
Patch tested: https://core.trac.wordpress.org/attachment/ticket/31354/31354.6.2.patch
Environment
- OS: macOS Sonoma 14.5
- WordPress: 6.7-alpha-58576-src
- Browser: Google Chrome Version 126.0.6478.114
Actual Results
- After applying the patch I see that certain discussion settings are displayed one after the other for better screen reader and tab accessibility (see attachment).
Additional Notes
- I have only tested Discussion Settings.
- I noticed that when I change the 14 for Number of days to keep old comments to 13 the screen reader says selection replaced 3, somehow I was expecting it to say 13?
Supplemental Artifacts
This ticket was mentioned in Slack in #core by stoyangeorgiev. View the logs.
3 months ago
This ticket was mentioned in Slack in #accessibility by rcreators. View the logs.
3 months ago
#47
@
2 months ago
Test Report
Tested Patch: https://core.trac.wordpress.org/attachment/ticket/31354/31354.6.2.patch
Environment
- WordPress: 6.7-alpha-59086
- PHP: 8.2.23
- Server: Apache/2.4.62 (Debian)
- Database: mysqli (Server: 11.5.2-MariaDB-ubu2404 / Client: mysqlnd 8.2.23)
- Browser: Chrome 129.0.0.0 (macOS)
- Theme: Twenty Twenty-Four 1.2
- MU-Plugins: None activated
- Plugins:
- WordPress Beta Tester 3.5.6
Results
I noticed that certain discussion settings are now displayed sequentially to enhance compatibility with screen readers after the patch was applied.
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
2 months ago
#50
@
2 months ago
Updated patch makes a couple of text changes; one because the text used for the age of post to automatically close comments was labeled "Number of days to keep old comments", which is not accurate.
Other changes are minor wording clarifications.
#51
@
2 months ago
- Keywords commit added
@sannevndrmeulen The screen reader says "selection replaced" or "selection removed" because the value of the value is selected when you focus into the input field. When you type or change the value using arrow keys, the selection highlight is removed, and that's announced to the user. It's as expected!
#52
@
2 months ago
Test Report
Patch tested: https://core.trac.wordpress.org/attachment/ticket/31354/31354.7.patch
Environment
- OS: Ubuntu 22.04.4 LTS
- WordPress: 6.7-alpha-58576-src
- Browser: Chrome (127.0.6533.119)
- Theme: Twenty Twenty-One
- Active Plugins:
- None
Actual Results
- ✅ Issue resolved with patch.
Additional Notes
- If I don't select the check box, Shouldn't it supposed go to the next option? Right now it is going inside of that option, if it's not selected
Supplemental Artifacts
#53
@
2 months ago
@tirth03 If I understand your question correctly, you're expecting that if one of the parent options, e.g. "Break comments into pages" is not selected, then the next tab
action should move past the related options.
We did consider making those options dynamically enabled, but decided that it was not necessary to change when the settings were available. It is entirely feasible that somebody might want to change those settings without enabling the parent option.
Thanks for testing!
This ticket was mentioned in PR #7494 on WordPress/wordpress-develop by @joedolson.
2 months ago
#54
Trac ticket: https://core.trac.wordpress.org/ticket/31354
@joedolson commented on PR #7494:
2 months ago
#56
in r59160
@joedolson commented on PR #6595:
2 months ago
#57
Closed in favor of later PR
@joedolson commented on PR #6659:
2 months ago
#58
Closed in favor of later PR
@joedolson commented on PR #6730:
2 months ago
#59
in r59160
Patch for the issue