Wrong behavior when the dashboard menu is folded

[nuxwin]

When the dashboard menu is folded, the submenus cannot be displayed on hover event, and in other cases, cannot be hidden. The problems occur in the following cases:

I. An opened submenu cannot be hidden when the menu is folded

How to reproduce the bug:

• 1. In unfolded state, toggle a submenu (open)
• 2. Folding the menu by clicking on separator

Result:

The submenu appears now on the right, and cannot be hidden.

Expected result:

When the menu is folding, all submenu must be defaulted hidden

II. Hover event don't work when the menu is folded

How to reproduce the bug:

• 1. In unfolded state, toggle a submenu (open)
• 2. In unfoled state, toggle the submeu previously open (close)
• 3. Fold the menu by clicking on separator

Result:

Now, the hover event for the menu (parent of the submenu that we have toggled) don't works and so, the submenu is never displayed.

Espected behavior:

On hover event, the submenu must be displayed.

III .Reason of the bug:

The reason of the bugs is the behavior of the the jQuery slideToggle() method used to toggle the submenus.

Note: I've a fix for that if you want.

Note: Sorry for my very poor English, I'm french.

[nuxwin]

patch to fix issue

[nuxwin]

I've added a patch. The inline style added by the slideToggle() method must be removed at end of process to make sure that the hover/out events will works when the menu is folded.

[scribu]

Bug confirmed.

patch.txt is a raw diff. We prefer svn diffs.

See ​http://core.trac.wordpress.org/#HowtoSubmitPatches

[scribu]

Also, please name attachments name.diff or name.patch for coloring to kick in.

[nuxwin]

Updated patch

[nuxwin]

Hello ;

I've updated the patch according your rules. I'm sorry for the previous one. Please confirm that the fix works.

[nuxwin]

Oh, I've not provided patch for the minified version. Don't forget it.

Thanks

[duck_]

Replying to nuxwin:

Oh, I've not provided patch for the minified version. Don't forget it.

That's fine. It's preferred that you only provide the patch for .dev.(js|css). Patching the minified version creates a very large patch which is almost impossible to review and so it will be redone by the committer anyway.

[scribu]

Patch works.

[nacin]

We can chain removeAttr() after toggleClass().

[nuxwin]

Replying to nacin:

We can chain removeAttr() after toggleClass().

Sorry but I thinks that you wrong. We can chain removeAttr() before toggleClass() but not after since we must operate on the submenu and not on the parent.

To resume:

var id = el.parent().toggleClass( 'wp-menu-open' ).attr('id');

becomes:

var id = el.removeAttr('style').parent().toggleClass( 'wp-menu-open' ).attr('id');

For the record, I've not done this in this way for a better understanding.

[nacin]

Sorry, yeah, meant before all of that.

[azaozz]

Partially related #10646. @nuxwin could you see if https://core.trac.wordpress.org/attachment/ticket/10646/10646-3.patch also fixes this issue. It changes the CSS classes for open/closed menus a bit and removes some of the JS.

[nuxwin]

Replying to azaozz:

Partially related #10646. @nuxwin could you see if https://core.trac.wordpress.org/attachment/ticket/10646/10646-3.patch also fixes this issue. It changes the CSS classes for open/closed menus a bit and removes some of the JS.

Sure, I'll try by applying your patch (without my pacth) to see the behavior. however, I'm not sure that only applying your patch without my patch will solve this issue. I'll also try to apply both patch in same time to be sure that both work together.

[nuxwin]

@azaozz

I. Your patch (10646-3.patch) without my patch.

Result: Your patch alone doesn't solve the #17058 issue.

Little question about the changes made by your patch. Now, when an admin page is loaded (at first time without any state data saved), the menu is defaulted folded. it's normal ? I say that because it 's not the case in current version.

Also, you can remove the following part:

	restoreMenuState : function() {
		// (perhaps) needed for back-compat
	},

Since it's not longer needed with your changes.

II. Both patches applyed:

All works fine.

I provides a merge of both patches (see 10646-3_17058_merge.patch).

[nuxwin]

Merge of patches (issue 10646 (3) and issue 17058)

[azaozz]

Replying to nuxwin:

...Now, when an admin page is loaded (at first time without any state data saved), the menu is defaulted folded. it's normal ? I say that because it 's not the case in current version.

The menu that has current submenu gets the classes wp-has-current-submenu wp-menu-open set from PHP. That overrides the saved state.

Also, you can remove the following part... Since it's not longer needed with your changes.

The comment there says it all :) It's unsafe to remove functions from WordPress, even JS functions. What if a plugin calls adminMenu.restoreMenuState() directly? It will throw a fatal error and probably break other JS.

[nuxwin]

Replying to azaozz:

Replying to nuxwin:

Also, you can remove the following part... Since it's not longer needed with your changes.

The comment there says it all :) It's unsafe to remove functions from WordPress, even JS functions. What if a plugin calls adminMenu.restoreMenuState() directly? It will throw a fatal error and probably break other JS.

Hmm, I've not thinking about this possibility. I understand now the purpose. Thanks for you explaination.

[azaozz]

(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