Make WordPress Core

Opened 11 months ago

Closed 8 months ago

Last modified 8 months ago

#58703 closed enhancement (fixed)

wp-list-table: <label> is preceding <input> in the checkbox column

Reported by: dimitrism's profile dimitris.m Owned by: joedolson's profile joedolson
Milestone: 6.4 Priority: normal
Severity: minor Version:
Component: Administration Keywords: has-patch commit
Focuses: accessibility Cc:

Description

Our automated UI tests caught a difference in WordPress 6.3-beta2 where the <label> element comes before the <input> one for the checkbox that's selecting all items in the admin tables (wp-list-table)

From https://www.w3.org/TR/WCAG20-TECHS/H44.html:

For all input elements of type checkbox or radio in the Web page::
Check that there is a label element that identifies the purpose of the control after the input element

Due to the above, I thought that this is worth raising and you can evaluate whether it's worth addressing or not.

Attachments (3)

Screenshot from 2023-07-03 11-40-56.png (99.0 KB) - added by dimitris.m 11 months ago.
wp-list-table: label element appearing before input
58703.diff (12.7 KB) - added by joedolson 10 months ago.
Switch label/input order
58703.2.diff (12.7 KB) - added by joedolson 9 months ago.
Refreshed patch

Download all attachments as: .zip

Change History (29)

@dimitris.m
11 months ago

wp-list-table: label element appearing before input

#1 @dimitris.m
11 months ago

Actually the same applies in all check-boxes, not just the top-ones but I can't edit the ticket now :)

I also want to add that this is potentially an accessibility defect introduced with 6.3

Last edited 11 months ago by dimitris.m (previous) (diff)

#2 @sabernhardt
11 months ago

  • Focuses accessibility added
  • Summary changed from wp-list-table: <label> is preceding <input> for the checkbox selecting all items to wp-list-table: <label> is preceding <input> in the checkbox column

Thanks for the report!

Most of the list tables had the label first in previous WordPress versions. However, #21516 switched the order for class-wp-ms-themes-list-table.php and update-core.php, and it followed the same pattern for the new labels in class-wp-privacy-requests-table.php.

#3 @joedolson
10 months ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 6.3
  • Owner set to joedolson
  • Severity changed from normal to minor
  • Status changed from new to accepted

It's not an accessibility defect, per se. The reason for placing a label after a checkbox is for visual clarity: it's the most predictable and usual location for a label to be associated with a checkbox. However, since these labels are not visible, this concern doesn't apply.

See: https://www.w3.org/TR/WCAG20-TECHS/G162.html#:~:text=Labels%20for%20radio%20buttons%20and,fields%20sometimes%20vary%20in%20length.

Techniques documents are non-conforming, so not following those instructions to the letter does not constitute a WCAG violation.

That said, since the label is not visible, the placement is arbitrary - so there's no reason we couldn't move the label after the checkbox. It would have no visual consequences for design, and could resolve a small set of automated testing false positives.

I do think there's a value in avoiding producing false positives, since those just add to the labor burden of managing accessibility, even if they have no real world consequences for the user.

@joedolson
10 months ago

Switch label/input order

#4 @joedolson
10 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

Changing the order of the label/input means that we can no longer use the + selector in CSS. Makes me wish for a preceding element selector. The patch is functional, but it opens up a wider gateway to potential compatibility issues with plugins, since it's now selecting descendents of .check-column. Could have also done this by adding two new classes, but I prefer this for simplicity.

However, I'm hesitant to make the switch this close to release candidate, since it does introduce new potential for compatibility issues and only fixes a potential false positive in testing. This may be better considered as a change to be made in 6.4, so that issues can be resolved early.

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


10 months ago

#6 @audrasjb
10 months ago

  • Milestone changed from 6.3 to 6.4
  • Version trunk deleted

As per today's bug scrub:
Since the issue wasn't introduced in 6.3 cycle, and as WP 6.3 release candidate 1 is planned tomorrow, let's move this ticket to the next milestone.

#7 @oglekler
9 months ago

  • Keywords needs-refresh added; needs-testing removed

@joedolson can we use check-column class? Or if this can have side effects, the new class can be added to this column additionally to handle input style inside. I was unable to apply the patch, possibly it needs to be updated or there is something with that patch is from SVN, and I am using git.

$ git apply --directory src 58703.diff didn't work for me.

#8 @joedolson
9 months ago

  • Keywords commit added; needs-refresh removed

I think we should use the .check-column class; while it could have some side effects, one of the possible side effects is expanding this functionality to plugins that have labels attached to their check column inputs. Plugins without labels shouldn't be impacted. This should probably get committed promptly so that any possible side effects are found early.

I was able to apply the patch, so not sure what's up there. Refreshing it, because there were a few minor line offsets.

@joedolson
9 months ago

Refreshed patch

#10 @sabernhardt
9 months ago

  • Keywords needs-testing added; commit removed
  • Type changed from defect (bug) to enhancement

Give it a little time for testing first; I am not comfortable with any side effects from this change. When plugin authors test the admin with a tool that marks these checkboxes as invalid code, they could ignore the error/warning as it's unrelated to their plugin. The markup order has been the same on most pages for years, and this is the first report.

#11 @joedolson
9 months ago

@sabernhardt What testing are you specifically looking for? Getting testing from plugin authors frequently only happens once code is committed, so that can be very difficult to get any other way.

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


9 months ago

#13 @sabernhardt
9 months ago

For a list of some plugins to test, I found quite a few that extend the WP_List_Table class.

#14 @marybaum
9 months ago

  • Keywords commit added; needs-testing removed

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


9 months ago

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


9 months ago

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


8 months ago

#18 @sabernhardt
8 months ago

  • Keywords changes-requested added; commit removed

#19 @marybaum
8 months ago

  • Keywords needs-patch added; has-patch removed

Here are the two changes @sabernhardt would like to see:

  1. Add box-sizing to the label so any padding from plugin styles would not make width or height more than 100%.
  2. Move the checkbox before the label within the Themes section of update-core.php.

This reflects activity on the PR.

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


8 months ago

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


8 months ago

@joedolson commented on PR #5044:


8 months ago
#22

There are a lot of conflicts, so for simplicity I'm opening a new PR.

This ticket was mentioned in PR #5276 on WordPress/wordpress-develop by @joedolson.


8 months ago
#23

  • Keywords has-patch added; needs-patch removed

Adds additional handling in update-core.php for themes, adds border-box, updates sprintf placeholders to match new content order where applicable.

Trac ticket: https://core.trac.wordpress.org/ticket/58703

#24 @joedolson
8 months ago

  • Keywords commit added; changes-requested removed

Requested changes made; @sabernhardt confirmed the changes met his expectations based on viewing the new PR. (via DM.)

#25 @joedolson
8 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 56665:

Administration: Switch order of label/checkbox in WP_List_Table.

Move the label after the checkbox in WP_List_Table instances. Resolve a false positive that will be presented by automated accessibility testing tools. Follow up to [55954].

Props dimitrism, joedolson, sabernhardt, oglekler, marybaum, tobiasbg.
Fixes #58703.

Note: See TracTickets for help on using tickets.