Improve menu toggle accessible name in admin menus

[joedolson]

Follow up from #32728

The Customizer accessible name format is "Edit menu item: {item title} ({item type})".
The admin accessible name format is "{item title} Menu item n of n".

It doesn't include the purpose of the control or the type, so the Customizer is providing better information; the admin menu needs to be updated.

[rcreators]

Updated File:

[rcreators]

Updated File:

src/js/_enqueues/lib/nav-menu.js
src/wp-admin/includes/class-walker-nav-menu-edit.php
src/wp-admin/nav-menus.php

Admin menu accessible name updated same as customizer menu.

[rcreators]

After Patch:

[joedolson]

Fix translator comments & PHPCS issues

[joedolson]

I think this is an improvement. What this does is makes the Customizer & the admin menu editor match their text for editing a menu item, giving the name of the item and its object type in the edit button/link.

It loses the information about the parent item in the admin menu, but I think that's still an improvement, overall. When browsing, the parent information will be discoverable by context, because menu items with the text 'sub item' that appear after an item without that are children of that parent. If navigating by tab, then the link will be found first, but the parent context is available inside the editing panel, as well.

This makes the edit button slightly simpler, and more consistent with other information. I don't think the position information is useful, since it's only meaningful in relation to the other items in the submenu, and those are only discoverable by exploration anyway.

I'd appreciate a second opinion from @alexstine or @afercia, however.

The alternate option would be to add the parent item to the edit link in both the customizer and this menu.

[joedolson]

Adding the sub-menu level, as well, in #60673, seems like having the parent becomes really verbose: Title / Type / Level / Parent is a lot of context to convey in one accessible name, when much of that data is available from surrounding context and the button action does not actually commit a change itself.

The sub-menu level is less easily discoverable, because you have to trace multiple steps in parentage, however, so I think that's more valuable.

[afercia]

Thanks for the ping @joedolson. A few observations after testing 60672.2.diff​:

JS off

When JavaScript is disabled, the items are supposed to show a different UI, with a 'Move up', 'Move down' and an 'Edit' links. The current patch alters the 'Edit' following the new patter. I'm not sure that would be ideal in this no-js scenario. See attached screenshot.

The position indication

The menu items are placed within an unordered list. However, the list is 'flat' and doesn't convey semantically the visual nesting. I think this is the original reason why the accessibility text that communicates the position was added. Users can change the nesting and order of the items. Updating on the fly the markup structure of the unordered list to semantically reflect the new nesting and order would be very complicated. The accessibility text is a remediation and I'd agree it's a little verbose but I'm not sure removing it entirely would be ideal. Is there any specific feedback from users that removing it is preferable?

The new name with 'Edit menu item'

I'd agree that prepending 'Edit menu item' to the accessible name would make the name consistent with the one used in the Customizer. It would also add more context for screen reader users. However, I'm not sure that would be great for other users, for example speech recognition software users. I'm concerned about the ​Label in Name principle though. Technically, the name contains the text that is presented visually. Practically, that would not work. For exammple:

Visible text: About The Tests Page
With the old aria-label="About The Tests. Menu item 2 of 4." the visible text is at the start of the name. There is a chance a voice command issued via speech recognition software may work.
With the new aria-label="Edit menu item: About The Tests (Page)" the visible text is in the middle of the name. As far as I can tell, this won't work with speech recognition software.
On the other hand, it is true that:

• The pattern that starts with 'Edit ...' and mismatches the visible text is used in other parts of the admin so this is a broader issue.
• Speech recognition software users do have other means to activate those links (e.g. 'Show numbers' or 'Show grid') although these alternative methods are less efficient and tedious.

Concatenation and escaping

Example from the patch:
esc_attr__( 'Edit menu item: ' . esc_html( $title ) . ' (' . esc_html( $menu_item->type_label ) . ')' ),

• We should avoid concatenation within translatable strings. sprintf() and placeholders should be used to provide a fully translatable string.
• In the example above, the entire string is already escaped with esc_attr__(). I'm not sure the variables within the string should be escaped with esc_html(). Of course by using sprintf() the escaping mechanism would be different.

[afercia]

JS off: hot is is supposed to be, and how it is after the patch

[joedolson]

Regarding "label in name" - the control itself has no visible text, so it's a stretch to consider the neighboring text a plausible label for the icon. However, the needs for voice command are definitely relevant. I made a proposal in #60673 that might help with this.

I do think that we should have some actual testing with the voice command issues; my gut feeling is that it's unlikely to work very well with either text.

Also kind of feeling like these two tickets should be collapsed into one; they're very similar, and whatever we decide on one is likely to necessarily resolve the other, too.

[rcreators]

@joedolson I also think these 2 tickets (60672 & 60673) should be merged as we cannot commit one ticket without discussing another one. So its better to have all discussions on one ticket.

[joedolson]

Because there is a significant amount of duplication between this ticket and #60672, I'm going to close this one as a duplicate of #60673. They're not exactly identical, but they are essentially addressing the same issue and cannot be handled independently.

We're going to be super proactive about getting these resolved in the current cycle, however, so that we don't lose track of the two related tickets.

[joedolson]

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.