Make WordPress Core

Opened 2 years ago

Closed 22 months ago

Last modified 15 months ago

#55498 closed defect (bug) (fixed)

Permalink Settings Page Accessibility Improve

Reported by: rishishah's profile rishishah Owned by: joedolson's profile joedolson
Milestone: 6.1 Priority: normal
Severity: normal Version:
Component: Permalinks Keywords: has-patch commit add-to-field-guide
Focuses: accessibility, ui-copy Cc:

Description (last modified by joedolson)

Labels and input fields are not all properly associated on permalinks settings screen.

Was:

On the Permalink Settings page, the current accessibility score is 94 for Desktop 92 > for Mobile devices.

Attachments (2)

Screenshot 2022-03-31 at 6.06.01 PM.png (301.6 KB) - added by rishishah 2 years ago.
55498.patch (824 bytes) - added by rishishah 2 years ago.
Here is the patch to fix and improve accessibility score to 100 on desktop and 98 on Mobile

Download all attachments as: .zip

Change History (30)

@rishishah
2 years ago

Here is the patch to fix and improve accessibility score to 100 on desktop and 98 on Mobile

#1 @sabernhardt
2 years ago

  • Component changed from General to Permalinks
  • Focuses ui-copy added
  • Version trunk deleted

Thanks for the report!

Yes, that input needs a label, though it should describe the input better than the base URL does. "Custom Permalink Structure Tags" is one option, and I welcome other ideas.

There are a few other considerations on this screen:

  • The radio buttons could have aria-describedby values connecting each to its example URL.
  • The radio button labels still wrap both the input and text (not separated, using id and for).
  • "Available tags:" could be a legend for a fieldset to group the related buttons.
  • I would like to wrap the custom text input plus its new (probably non-visual) label and base URL within a <span class="code"></span> for better RTL support, which might fit well with #47755 instead.
  • When navigating by keyboard, editing the custom input value on changing the radio buttons can cause problems unintentionally replace a custom set of tags. This JS issue can be on its own ticket; I'm not sure about the others.
Last edited 2 years ago by sabernhardt (previous) (diff)

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


2 years ago

This ticket was mentioned in PR #2495 on WordPress/wordpress-develop by kebbet.


2 years ago
#3

  • Keywords has-patch added

kebbet commented on PR #2495:


2 years ago
#4

Screenshot of current PR
https://i0.wp.com/user-images.githubusercontent.com/11491369/161320800-8957a26c-731e-43f9-a35c-0cb561007171.png

#5 @sabernhardt
2 years ago

I missed that the text field value is the permalink structure setting (and the radio buttons would need to continue editing that value on the change event). So the label should communicate that the structure can be customized, yet it is not always custom.

"Customize permalink structure by selecting available tags" could fit.

This ticket was mentioned in PR #2557 on WordPress/wordpress-develop by sabernhardt.


2 years ago
#6

This pull request builds on PR2495 by @kebbet, which:

  • Groups the radio buttons inside one fieldset and the available tags buttons inside another.
  • Adds for and id attributes for associating each input with its label.
  • Connects the URL examples to each input with aria-describedby.

Further edits:

  • Adds missing label for text input: "Customize permalink structure by selecting available tags" is an option.
  • Moves inputs (and other elements) outside of the labels.
  • Uses existing "Permalink structure" text as a legend for the radio buttons ("Select permalink structure" may be more helpful, though I tend to prefer reusing text strings).
  • Adds presentation role to the table.
  • Assigns the structure-selection class to the fieldset instead of the table row.
  • Wraps the Custom Structure option's URL and input in a <span class="code"></span> tag to keep the field on the right side in RTL languages.

This version retains the list item elements for the "Available tags" buttons. I like that my screen reader (NVDA) says the item count. The layout might be improved by flex and flex-wrap instead of floating, but I do not think that belongs on this ticket.

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

sabernhardt commented on PR #2557:


2 years ago
#7

English - desktop

https://i0.wp.com/user-images.githubusercontent.com/17100257/162689425-cfc9bf48-2023-43ae-955b-a81200fbca7f.png

Arabic (right-to-left) - desktop

https://i0.wp.com/user-images.githubusercontent.com/17100257/162689422-4be85ed1-551c-478e-9bc5-5116d940c815.png

English - narrow mobile

https://i0.wp.com/user-images.githubusercontent.com/17100257/162689417-b9de0458-530b-422f-8539-e04d3c2583c3.png

Arabic (right-to-left) - narrow mobile

https://i0.wp.com/user-images.githubusercontent.com/17100257/162689414-39df4e3a-5ebf-4f73-8eef-15e52432ea6e.png

kebbet commented on PR #2557:


2 years ago
#8

Nice! Well done!

#9 @kebbet
2 years ago

I was sketching to add a paragraph between the heading Common Settings and the selection table. Something like this.
Select the permalink structure for your website. A structure that includes the post name makes sharing links to others more understandable.

Any thoughts?

kebbet commented on PR #2557:


2 years ago
#10

Does this makes any sence? Better alignments of radio buttons.
https://github.com/WordPress/wordpress-develop/pull/2563

This ticket was mentioned in PR #2563 on WordPress/wordpress-develop by kebbet.


2 years ago
#11

This pull request builds on PR2557 by @sabernhardt.

Following changes are made in relationship to 2557.

  • Build an array to permalink structure options
  • Loop the array to make sure markup is consistent
  • Adds div-tags for better alignments of labels and descriptions to input-rows.

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

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


2 years ago

#13 @ryokuhi
2 years ago

  • Milestone changed from Awaiting Review to 6.1

This ticket was reviewed today during the Accessibility Team's weekly bug-scrub.
We're milestoning it for 6.1, so that there's already a timeframe for it.

#14 @audrasjb
2 years ago

The PR looks pretty good to me, thanks!
I added a small comment in the last PR, but it looks pretty solid.

#15 @audrasjb
2 years ago

Thanks for the changes @kebbet!
PR2563 looks great to me

kebbet commented on PR #2563:


23 months ago
#16

@sabernhardt Did the two commits cover your feedback? Thanks so much for your time!

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


22 months ago

#18 @sabernhardt
22 months ago

  • Keywords commit added
  • Owner set to joedolson
  • Status changed from new to reviewing

#19 @joedolson
22 months ago

  • Description modified (diff)

#20 @joedolson
22 months ago

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

In 53706:

Permalinks: Label and describe permalink settings fields.

Restructure the permalink settings fields so URL formats are communicated to screen readers through aria-describedby relationships, avoid the usage of implicitly labeled input fields, labeling the custom permalink format correctly, and clarifying values. Make settings table element presentational.

Props rishishah, sabernhardt, kebbet.
Fixes #55498.

#21 @joedolson
22 months ago

While testing this for commit, noticed a pre-existing bug with the aria-live messages. Opened a follow-up ticket at #56230.

kebbet commented on PR #2557:


22 months ago
#23

Could be closed since https://github.com/WordPress/wordpress-develop/pull/2563 now is merged. @sabernhardt

#24 @SergeyBiryukov
22 months ago

In 53713:

Coding Standards: Use a single input array on Permalink Settings screen.

This reduces the amount of code to edit in case of any changes to the default permalink structures.

Follow-up to [53706], [53710].

See #55647, #55498.

#25 @milana_cap
19 months ago

  • Keywords add-to-field-guide added

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


19 months ago

#27 @sabernhardt
19 months ago

#56757 was marked as a duplicate.

#28 @audrasjb
15 months ago

In 55103:

Permalinks: Remove floating on Permalinks settings screen.

This changeset replaces floating elements with flex CSS positioning, while the markup stays the same to keep the elements' semantic value.

Follow-up to [53706].

Props sabernhardt, kebbet, audrasjb.
Fixes #56673.
See #55498.

Note: See TracTickets for help on using tickets.