Make WordPress Core

Opened 3 months ago

Last modified 1 second ago

#60685 assigned defect (bug)

Keyboard focus order mismatch in adminbar in front-end

Reported by: joedolson's profile joedolson Owned by: joedolson's profile joedolson
Milestone: 6.6 Priority: normal
Severity: normal Version: 3.3
Component: Toolbar Keywords: has-patch needs-dev-note 2nd-opinion
Focuses: accessibility Cc:

Description

When viewing the Admin bar on the front-end, the search tool is added visually in the far right corner, after the profile menu. In the DOM, however, the search bar is located before the profile menu.

These should be reversed in the DOM so that keyboard order matches visual order.

See r19518.

Attachments (2)

60685.diff (1.4 KB) - added by sabernhardt 8 weeks ago.
removes float: right and increases priority number for the Search node
60685.9999.diff (1.4 KB) - added by sabernhardt 5 days ago.
sets Search menu priority at 9999

Download all attachments as: .zip

Change History (12)

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


3 months ago

#2 @joedolson
3 months ago

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

@sabernhardt
8 weeks ago

removes float: right and increases priority number for the Search node

#3 @sabernhardt
8 weeks ago

  • Keywords has-patch added; needs-patch removed

To keep the Search form to the right of other links without the float, the priority would need to change. I chose 20: more than recovery mode's 8, with more than one number in between for anyone who wants to have a link between the "Howdy" account links and the Search icon.

At least two of the plugins that add links to the secondary group have a priority of 999 (see directory search), so it may be appropriate to increase the priority more.

#4 @joedolson
2 weeks ago

I think the question here is what the intent is for plugins with a higher priority. Previously, with the float in place, it was impossible to add items to the right of the search without also customizing CSS. Now, everything will be ordered according to priority.

While some plugins may need to make changes, I'm not sure that's a significant concern. Nothing will actually break here as a result, a link may just appear in a different place than previously expected.

We could certainly increase the value to provide more space; though it's not strictly necessary - you can have multiple items in a single priority, after all.

Can you specify any of the plugins that have a priority of 999? I browsed those results a bit, but I didn't see them.

I think this is basically ready to commit; the only question is what the ultimate priority level should be. I'm inclined to think that 20 offers enough space, and that moving later doesn't really offer any significant advantages.

#5 @sabernhardt
2 weeks ago

Yeah, it is not so easy to search for 'parent' => 'top-secondary' and then to find where that specific node is added with the admin_bar_menu action.

For some additional priorities:

#6 @joedolson
2 weeks ago

Based on those values, with the patch as is, it would result in a change in the adminbar for all of those plugins.

I don't think there's anything we can do about the Polylang switch; we should just add that in a dev note and possible contact them directly with a heads up. It's a trivial change for them to make if needed.

But we can make this work for all of the other plugins, just by deciding that our aim is to have the search item as the last element of the adminbar, and giving it a priority of 9999, which would keep it at the end of the adminbar for the vast majority of cases.

#7 @joedolson
10 days ago

@sabernhardt I think it would be good to get this in sooner rather than later; what do you think about the proposal to just give it a very high priority?

#8 @sabernhardt
10 days ago

  • Keywords needs-dev-note added

I'm not sure about 9999, but that is probably a better choice than 20 (or a similarly low number). Maybe it's worth mentioning in the dev chat (agenda) first.

I tried a version-based priority to confirm that plugin authors could change it if they do not like where their links appear:

$special_priority = ( version_compare( $GLOBALS['wp_version'], '6.6-alpha', '>=' ) ) ? 2 : 100;

add_action( 'admin_bar_menu', 'wpadmin_toolbar_test_link', $special_priority );

function wpadmin_toolbar_test_link( $wp_admin_bar ) {
	$wp_admin_bar->add_node(
		array(
			'parent' => 'top-secondary',
			'id'     => 'mylink',
			'href'   => '#',
			'title'  => __( 'My Link' ),
		)
	);
}

@sabernhardt
5 days ago

sets Search menu priority at 9999

#9 @sabernhardt
5 days ago

I uploaded a patch to consider the 9999 priority.

#10 @joedolson
1 second ago

  • Keywords 2nd-opinion added

The value of 9999 feels quite arbitrary; but I think that any value here is ultimately pretty arbitrary. It's easy for extenders to get around, and in most cases will have the desired result.

We can run it by a dev chat, but it seems like a somewhat trivial issue to raise in a meeting. If we can just get a second opinion from another committer, I think that would be sufficient.

Pinging @audrasjb as the 6.6 tech lead.

Note: See TracTickets for help on using tickets.