Make WordPress Core

Opened 8 months ago

Closed 4 months ago

Last modified 4 months ago

#60673 closed enhancement (fixed)

Indicate submenu item level in admin nav menu editor and customizer nav menu editor

Reported by: joedolson's profile joedolson Owned by: rcreators's profile rcreators
Milestone: 6.6 Priority: normal
Severity: normal Version:
Component: Menus Keywords: has-patch commit
Focuses: accessibility, javascript Cc:

Description

Follow up to #32728

Following the closure of #32728, both navigation menu interfaces indicate whether an item is in a submenu. It would improve the experience for screen reader users to indicate what level an item is at.

Change History (21)

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


7 months ago

#2 @joedolson
7 months ago

  • Owner set to rcreators
  • Status changed from new to assigned

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


7 months ago
#3

  • Keywords has-patch added; needs-patch removed

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


7 months ago

#5 @oglekler
5 months ago

  • Keywords needs-testing needs-testing-info added

@rcreators can you please check that patch don't need to refresh, provide screenshot and testing instructions? 🙏 This way it will be easier to understand where to look and what to expect. Thank you!

#6 @joedolson
5 months ago

@oglekler There are no visual changes in this patch; it's only changing the text passed to screen reader users.

The change, currently:

For top level menu items:
Was: [item name]. Menu item [x] of [y]
New: Edit menu item: [item name] ([type])

For sub menu items:

Was: [item name]. Sub item number [x] under [parent item].
New: Edit menu item: [name] ([type]), sub-item (level [depth])

A couple things I'd like to see change here:

1) The old sentences ended with a period; that should still be true.
2) Per the conversations on #60672, I think keeping the item position and parent name in the menu would be better.

Perhaps the new text should be more like

New: Edit [item name] ([type] menu item, [x] of [y])

and

New: Edit [item name] ([type] menu item, sub-item [x] of [y] at level [depth]

This provides a user with the full scope of context about where this item is located and what it is. It is verbose, but because of the ordering, a user could move on if they know there's only one item of the appropriate name.

I've shortened the edit prefix as well to account for @afercia's feedback on #60672. For voice recognition, I honestly think that the name only has a very low probability of triggering these buttons in any case; but I'm wondering if we changed the icon to the edit pencil & simplified like this whether it could be more intuitive for voice command usage. The pencil is more likely to be interpreted as 'edit', so a user might try speaking 'Edit [item name], and that would seem like a good possible compromise.

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


5 months ago

#8 @joedolson
5 months ago

In today's accessibility bug scrub, we talked about refining these messages further. We're going to move to three different levels of message depending on depth, and do what we can to simplify the messages:

First level:

Edit [item name] ([type], [x] of [y]).

Second level:

Edit [item name] ([type], sub-item [x] of [y] of [parent]).

Third level:

Edit [item name] ([type], sub-item [x] of [y] of [parent], level [depth]).

@rcreators will work on updating the patch and merging in the changes recommended by @afercia in #60672.

The text 'menu item' is being removed to simplify the message, and because that context should be sufficiently clear when you're in the menu editing context.

#9 @joedolson
5 months ago

#60672 was marked as a duplicate.

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


5 months ago

#11 @rcreators
5 months ago

I have implemented above messages for admin section and this is working fine. Now I have issue with doing it with customizer area.

In customizer, the item control php file get data from item control. Also few data coming from js file render function control. I tried to add or fetch data but it's quite complicated to fetch or add data for item position, total position and parent name.

If someone have idea to do that for customizer, please do let me know.

#12 @rcreators
5 months ago

  • Keywords needs-testing-info removed

@joedolson I was able to complete this with some custom functions and took the same logic as per the admin area. I created accessibility refresh functions and updated screen reader text for customizer menu items on focus or click. Please check code and test it.

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


5 months ago

#14 @joedolson
4 months ago

Requested some changes on the PR. I think this is looking pretty good, and the resolution of the previous changes requested seems sound. Would appreciate a 2nd opinion from @afercia.

I still stand in favor of having the text start with 'Edit', because there is no visible text label, so I'm not convinced that the label in name argument is very sound.

#15 @rcreators
4 months ago

@joedolson The Requested changes have been amended.

The text starts with Edit in the changes as decided and commented above https://core.trac.wordpress.org/ticket/60673#comment:8

The translator points from the #60672 ticket was already amended. So not many changes are required apart from some textual changes.

@rcreators commented on PR #6318:


4 months ago
#16

@afercia All changes have been amended.

@rcreators commented on PR #6318:


4 months ago
#17

Please see inline comments

All the requested changes are pushed.

@afercia commented on PR #6318:


4 months ago
#18

Thanks for addressing the coding standards points I raised @Rcreators
I understand part of this PR is code copied from already existing code there's some more occurrences of missing spaces between parehthesis and single quote (' or ') that would be nice to fix if possible. Thanks!
Other than that, I would leave functional and final review to @joedolson

#19 @joedolson
4 months ago

  • Keywords commit added; needs-testing removed

Testing:

Admin menus:
Root level menu item: Edit [name] ([type], 1 of 2)
1st level submenu item: Edit [name] ([type], sub-item 1 of 2 under [parent])
2nd level submenu item: Edit [name] ([type], sub-item 1 of 2 under [parent], level 2)

JS disabled: No visible changes seen.

Customizer:
1st level menu item: Edit [name] ([type], 1 of 2)
2nd level menu item: Edit [name] ([type], sub-item 1 of 2 under [parent])
3rd level menu item: Edit [name] ([type], sub-item 1 of 2 under [parent], level 2)

Both:

  • Changes in position were consistently made when an item was changed, either to a different level, parent, or position.
  • Two different menu editors have parity in button description content.

#20 @joedolson
4 months ago

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

In 58306:

Menus: Accessibility: Improve screen reader text for edit button.

Change the edit menu item toggle to communicate more context about the item to be edited. Make edit text consistent between Customizer menu editor and admin menu editor.

The menu position is conveyed only visually, using indentation, because there are no organizational semantics in either editor. This change helps provide screen reader users with consistent contextual information about the order, position, and parent of the current item.

Props joedolson, rcreators, afercia, mohonchandra.
Fixes #60673, See #60672.

Note: See TracTickets for help on using tickets.