WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 3 years ago

Last modified 3 years ago

#10646 closed task (blessed) (fixed)

Admin Menus Shouldn't Jump Around on Pageload

Reported by: filosofo Owned by:
Milestone: 3.2 Priority: normal
Severity: normal Version:
Component: Administration Keywords: 3.2-early has-patch needs-testing
Focuses: Cc:

Description (last modified by filosofo)

Problem: When I open an admin page, I see the menu item I want and go to click it, and just at that moment the admin menu suddenly expands so that I click the wrong item.

Solution: Instead of expanding the menus by retrieving their saved state and adding open classes with JavaScript, we should retrieve the saved state from usermeta (it's already being saved there) and print the menus with appropriate classes, accordingly, so that there is no jump.

Attachments (6)

garyc40-10646.patch (1.7 KB) - added by garyc40 3 years ago.
there's a patch for that
garyc40-10646-rev2.patch (1.5 KB) - added by garyc40 3 years ago.
use get_user_setting()
10646.diff (2.9 KB) - added by nacin 3 years ago.
10646.2.diff (2.9 KB) - added by nacin 3 years ago.
10646-3.patch (3.0 KB) - added by azaozz 3 years ago.
10646-4.patch (3.5 KB) - added by nuxwin 3 years ago.
Some fixes added against the previous patch

Download all attachments as: .zip

Change History (32)

comment:1 follow-up: strider725 years ago

If we make the change you suggest, I believe there will be no way to close a menu if someone accesses Admin without JavaScript.

comment:2 in reply to: ↑ 1 filosofo5 years ago

Replying to strider72:

If we make the change you suggest, I believe there will be no way to close a menu if someone accesses Admin without JavaScript.

Well, currently if you access the admin without JavaScript there's no way to open a menu, except for the current children and ones preset as open, so there doesn't seem to be too much concern about allowing the non-JS folks flexibility with saved open/close states.

However, if that's something people want, we could just make the open/close arrows into links that accomplish state change for non-JS users. That might be a good idea anyways, since the current, non-JS menu has arrows that do nothing.

comment:3 filosofo5 years ago

  • Description modified (diff)

comment:4 nacin4 years ago

Related: #11957, changing how menu states are stored.

comment:5 voyagerfan57614 years ago

  • Cc WordPress@… added

comment:6 nacin4 years ago

  • Keywords early added
  • Milestone changed from Future Release to 3.1

Should be looked at with #11957.

comment:7 t31os_4 years ago

I'm constantly clicking wrong menu items because of this problem, the suggestion is sound for me, i'd much prefer classes to be added to the elements on page load, so we avoid all this jumping around.

comment:8 nacin3 years ago

  • Keywords 3.2-early added; early removed
  • Milestone changed from Awaiting Triage to Future Release

Marking for 3.2-early. I want to tackle this.

garyc403 years ago

there's a patch for that

comment:9 garyc403 years ago

  • Keywords has-patch needs-testing added

comment:10 nacin3 years ago

Thanks for doing this!

You can probably kill wp_get_current_user() and the extra argument to get_user_option(), which defaults to the current user.

Actually, avoid get_user_option() all together, and leverage get_user_setting() and/or get_all_user_settings().

garyc403 years ago

use get_user_setting()

comment:11 garyc403 years ago

There you go :)

comment:12 sbressler3 years ago

  • Cc sbressler@… added

nacin3 years ago

comment:13 nacin3 years ago

10646.diff takes a different approach, to support no-JS situations. Also, the increment occurs in a different spot, as only top-level menus that have a submenu are actually counted. (Otherwise, the comments menu throws off the lower menus.)

nacin3 years ago

comment:14 nacin3 years ago

Second patch changes the logic on wp-admin/menu-header.php:61 to ensure only menus with submenus are checked against the user settings. Otherwise for example the Comments menu will get wp-menu-open-if-js if Pages is open, despite it not doing anything.

comment:15 nacin3 years ago

  • Milestone changed from Future Release to 3.2
  • Type changed from defect (bug) to task (blessed)

azaozz3 years ago

comment:16 azaozz3 years ago

10646-3.patch is almost identical to 10646.2.diff except it uses only CSS to initially open/close the menus so it doesn't require any JS to run on DOM ready.

comment:17 follow-up: nuxwin3 years ago

With the last patch (10646-3.patch) applied and under no-js mode, the Posts menu is always displayed (expanded). For me, it's bad behavior.

Edit

The Problem seem to be not linked to the patch since it's same behavior in current version.

Edit:

Ok; that seem to be another bug, but I don't know if I must create separate ticket for it... In no-js mode, the Posts submenu is always expanded because the menu has always the class open-if-no-js added to it. I doesn't known if it's the expected behavior or not. If yes, why ?

For me, under no-js mode, the open-if-no-js must be only added on the current menu (linked to the current loaded admin page).

To finish, I thinks that under no-js mode, all arrows that allow to expand the submenus in js mode must be hidden.

I'll provide a modified version of the current patch , which will include all these considerations.

Note: Sorry for the empty comments bellow. I've found the edit option too late.

Last edited 3 years ago by nuxwin (previous) (diff)

comment:18 nuxwin3 years ago

Last edited 3 years ago by nuxwin (previous) (diff)

comment:19 nuxwin3 years ago

Last edited 3 years ago by nuxwin (previous) (diff)

comment:20 nuxwin3 years ago

The patch 10646-4.patch is almost identical to the 10646-3.patch patch. It include the following fixes:

  • Under no-js mode, only the menu linked to the current loaded page is expanded ;
  • Under no-js mode, all arrows are hidden.

The patch also includes the fix linked to the #17058 issue.

Version 0, edited 3 years ago by nuxwin (next)

comment:21 nuxwin3 years ago

  • Cc l.declercq@… added

comment:22 in reply to: ↑ 17 ; follow-up: azaozz3 years ago

Replying to nuxwin:

With the last patch (10646-3.patch) applied and under no-js mode, the Posts menu is always displayed (expanded). For me, it's bad behavior.

This is an UX decision. Since the Posts submenu is the most used one, it's always open when no-js.

To finish, I thinks that under no-js mode, all arrows that allow to expand the submenus in js mode must be hidden.

Agreed.

comment:23 in reply to: ↑ 22 nuxwin3 years ago

Replying to azaozz:

Replying to nuxwin:

With the last patch (10646-3.patch) applied and under no-js mode, the Posts menu is always displayed (expanded). For me, it's bad behavior.

This is an UX decision. Since the Posts submenu is the most used one, it's always open when no-js.

To finish, I thinks that under no-js mode, all arrows that allow to expand the submenus in js mode must be hidden.

Agreed.

Ok so, I'll update the patch according this UX decision. For me, it's a bad decision but it's only my personal opinion.

nuxwin3 years ago

Some fixes added against the previous patch

comment:24 nuxwin3 years ago

Patch updated (UX decision - Posts menu must be expanded under no-js mode).

comment:25 azaozz3 years ago

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

(In [17623]) Don't refresh the admin menu after page load, apply the user-state from PHP, fix behaviour after folding the menu, props nacin, props nuxwin, fixes #10646, fixes #17058

comment:26 sbressler3 years ago

Excited to see this applied - this should significantly speed up the seeming load time of all admin pages!

Note: See TracTickets for help on using tickets.