Admin Menu: Fix multiple active menu highlights on mobile. - #11902
Admin Menu: Fix multiple active menu highlights on mobile.#11902itzmekhokan wants to merge 6 commits into
Conversation
Use :focus-visible for admin menu focus indicators so touch taps do not leave a stale active appearance on previously selected items. Clear the responsive selected state when another top-level menu is opened or when navigating via a submenu link. Limit admin menu view transitions to the current menu item to avoid overlapping active styles during navigation. Fixes #65280.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
|
@itzmekhokan thank you very much for the PR! I wasnt able to replicate the issue on desktop but on mobile (iOS, edge browser) with wordpress playground though. when i tested it the dashboard menu item showed up marked active. can you recheck? |
When the responsive menu is opened, focus was moved to the first menu item (Dashboard). On browsers that match :focus-visible for scripted focus (e.g. Safari/WebKit on iOS), that first item receives the active highlight, so two menu items appear active at once. Move focus to the current top-level item when one exists, falling back to the first menu link, so the focused item is the one that is meant to be highlighted. Follow-up to the :focus-visible changes for #65280.
The admin color schemes (compiled from colors/_admin.scss) styled the top-level menu highlight on plain :focus, overriding the :focus-visible rules in admin-menu.css since color scheme CSS loads later. This brought back the active appearance for pointer/scripted focus on browsers that match :focus-visible inconsistently. Switch the color scheme highlight to :focus-visible to match admin-menu.css, and drop the now-redundant :not(:focus-visible) overrides that were compensating for the mismatch. Follow-up to the :focus-visible changes for #65280.
Thanks @masteradhoc for the careful retest, and @neo2k23 for cross-checking on iPad/iPhone — the conflicting results (reproducible on iOS Edge, not on your installs) were the key clue. Pushed a fix in 792cbd4 and c62309b. |
There was a problem hiding this comment.
Pull request overview
This PR updates the WordPress admin menu’s interaction and styling to avoid multiple “active” highlights in responsive/mobile contexts, aligning focus indication with :focus-visible and tightening which menu item participates in view transitions.
Changes:
- Limit admin-menu view transitions to the current top-level item (
.wp-has-current-submenu/.current) to prevent overlapping active styles during navigation. - Switch various admin-menu focus highlight rules from
:focusto:focus-visibleso touch interactions don’t leave stale focus highlights. - Update responsive admin-menu JS behavior to focus the current menu item on open and to clear the responsive
selectedstate when opening other top-level menus or navigating via submenu links.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/phpunit/tests/view-transitions/wpEnqueueViewTransitionsAdminCss.php | Extends assertions to cover the new admin-menu selector present in the generated view-transitions CSS. |
| src/wp-admin/css/view-transitions.css | Restricts view-transition-name assignment to only the current admin menu item(s). |
| src/wp-admin/css/colors/_admin.scss | Updates admin color-scheme focus highlight selectors to use :focus-visible. |
| src/wp-admin/css/admin-menu.css | Updates admin-menu focus highlight selectors to use :focus-visible. |
| src/js/_enqueues/admin/common.js | Adjusts responsive menu focus/selection behavior to avoid stale active appearance on mobile. |
Comments suppressed due to low confidence (2)
src/wp-admin/css/admin-menu.css:170
- This focus styling is now :focus-visible-only. For browsers without :focus-visible support, focused menu items won’t get this background/color (and common.css suppresses default focus styles for #adminmenu links), which can make keyboard focus hard to see. Recommend adding an
@supports-notfallback to apply the same styling on :focus when :focus-visible isn’t available.
#adminmenu li.menu-top:hover,
#adminmenu li.opensub > a.menu-top,
#adminmenu li > a.menu-top:focus-visible {
src/wp-admin/css/admin-menu.css:650
- The auto-fold z-index rule is now :focus-visible-only. In browsers without :focus-visible support, focused items won’t get the intended stacking behavior. Consider the same
@supports-notfallback to :focus here for compatibility.
.auto-fold #adminmenu li.menu-top:hover,
.auto-fold #adminmenu li.opensub > a.menu-top,
.auto-fold #adminmenu li > a.menu-top:focus-visible {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
joedolson
left a comment
joedolson
left a comment
There was a problem hiding this comment.
While this does resolve the problem, it also adds a significant behavior change in the menu, jumping users down to the currently active page instead of simply entering the menu. While I can see that possibly being a desirable change, I think that would merit further discussion; it's while beyond the scope of the associated ticket.
Remove the change of focus target on the initial menu open, but I encourage opening that as a possible new enhancement.
| // Move focus to the current menu item when one exists, so browsers that | ||
| // match :focus-visible for scripted focus do not show a stale active | ||
| // highlight on the first item. Fall back to the first menu link. | ||
| var $focusTarget = $adminmenu.find( 'li.wp-has-current-submenu > a.menu-top, li.current > a.menu-top' ).first(); |
There was a problem hiding this comment.
Changing the focus move from the top of the menu to the current page is a major behavior change, and I think should be discussed as a separate possible enhancement.
There was a problem hiding this comment.
Thanks for the review! I've removed the focus target change on the initial menu open in 4c09b2e
… menu link. Moving focus to the current menu item when the responsive menu is opened is a behavior change beyond the scope of this ticket, and is better considered as a separate enhancement. See #65280.
Use :focus-visible for admin menu focus indicators so touch taps do not leave a stale active appearance on previously selected items. Clear the responsive selected state when another top-level menu is opened or when navigating via a submenu link. Limit admin menu view transitions to the current menu item to avoid overlapping active styles during navigation.
Fixes #65280.
Trac ticket: https://core.trac.wordpress.org/ticket/65280
Use of AI Tools
N/A
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.