• Pave the way towards non-menu widgets.

Proposed solution

  1. Separate the markup and styling for the dropdown menu from the general toolbar container.


Members fund testing for the Drupal project. Drupal Association Learn more


sun’s picture

Status: Active » Needs review
Issue tags: +API change
17.54 KB
PASSED: [[SimpleTest]]: [MySQL] 228 pass(es). View

That was almost easier than I thought.

Needs lots of manual testing. Make sure to change the module's configuration settings; i.e.,

- enable/disable Admin Menu Toolbar module
- enable/disable Shortcut module
- different browsers
- etc...

sun’s picture

16.97 KB
PASSED: [[SimpleTest]]: [MySQL] 228 pass(es). View

Minor clean-ups.

JSCSJSCS’s picture

13.11 KB
11 KB
3.35 KB

mysteriously "git apply -v admin_menu.dropdown.2.patch" did not apply the patch. Google search suggested "patch -p1 < admin_menu.dropdown.2.patch" for patches that "mysteriously" do not work with git apply.

$ patch -p1 < admin_menu.dropdown.2.patch
patching file `admin_menu.css'
patching file `admin_menu.module'
patching file `admin_menu_toolbar/admin_menu_toolbar.css'
patching file `admin_menu_toolbar/admin_menu_toolbar.js'
patching file `admin_menu_toolbar/admin_menu_toolbar.module'

I tested using Chrome 18.0.1025.168 m

When selecting the Home button, the Edit link moves to the left side and is a bit off. Any other admin menu link has the edit link back to the right side.
But reduced browser window size is looking much improved:

sun’s picture

Thanks for testing!

I believe that the first screenshot (as well as misplaced Edit shortcuts link) is caused by Shortcut module's shortcut.css not being loaded when the menu is delivered from cache. That's a separate issue, which requires us to change the server-side cache once more to not cache an already rendered string, but a render array instead. Let's tackle that in #1567622: Additional attached JS/CSS is missing in cached output

Aside from that, I'm relatively happy with this patch.

The only thing I noticed was that due to the separate .dropdown ULs, the delayed collapsing causes dropdown menus to overlap in the special situation of the icon menu and the admin menu. That is, because the JS that performs this seems to treat each UL.dropdown as a separate menu instance, so it doesn't collapse the icon menu when moving the mouse over another menu. However, I'd consider that minor, and we can fix that in a follow-up as well.

JSCSJSCS’s picture

Perhaps related to the separate dropdown UL, and after playing with this a few days, i found that there was sufficient delay in rendering the admin menu and then the shortcut menu that I often had already clicked a link on the next page, only to find I missed it as the shortcut menu was being rendered and had to click again. It was so distracting that after using shortcut menu to click a link, I would immediately turn it back off.

sun’s picture

Status: Needs review » Fixed

I didn't fully understand what you mean. Can you create a separate follow-up issue for that, please? I'd definitely like to look into performance issues, but let's tackle the topic separately from this basic clean-up.

Thanks for reporting, reviewing, and testing! Committed to 3.x branches.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

JSCSJSCS’s picture

What I was saying is that after clicking on a link, it takes a while (sometimes a second or longer) for the admin menu to reappear. In that time, I may have tried to click on something, but the admin menu just pushed the screen down the 20px or so and I miss the link target.

So testing some more on the patch and I did find that after adding a few more shortcut links, on clicking them a few times, maybe n the third link click, the menu changes. It gets less tall and other changes.

JSCSJSCS’s picture

So this menu change I mentioned above is not a random event. It occurs when the theme changes.

In the first image, I was on a home page with Omega theme. Then I clicked on an administration link that uses the Seven theme. I can click on any admin link and there is no change.

But if I click a link that goes back to a regular page (omega theme), that is when the menu gets messed up as in picture two.

I turned off any Omega debugging tools too. That did not change anything. Hope this helps.

JSCSJSCS’s picture

When I clear admin menu cache and am on my omega theme home page, this css is present for the edit shortcut link from shortcut.css:

div#toolbar a#edit-shortcuts {
float: right;
padding: 5px 10px 5px 5px;
line-height: 24px;
color: #FEFEFE;

But if I reload the page (omega theme, by clicking home button) the menu gets messed up again and the above css is nowhere to be found.

If I click on an admin link (seven theme) in the shortcut menu, then the shortcut.css is loaded and the menu looks good again. Clicking a shortcut link that goes back to the regular omega theme link makes the shortcut.css go away again. No idea why.

sun’s picture

sun’s picture

That said, we can apply a stop-gap fix until aforementioned issue is resolved.

Committed attached patch.

JSCSJSCS’s picture

23.08 KB

Thanks @sun. This patch fixed that issue with the menu getting messed up when switching themes, or simply clearing cache while in the omega theme.

But there is still a little issue that I may not have noticed before: When selecting a link that goes from omega them to seven, the active link is not updated. In this image, I switched to the seven theme by clicking "Administer Blocks", but the active (highlighted) link is still showing the previous omega theme link "Fix Dealer Locations" as the active link.

activve links not follwing

sun’s picture

@JSCSJSCS: Can you create a separate bug report for #12, please?

That bug isn't caused by this change.

JSCSJSCS’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

ShaneOnABike’s picture

It's weird cause I got the latest code and it still is happening for me?

ShaneOnABike’s picture

Status: Closed (fixed) » Needs work
sun’s picture

Status: Needs work » Closed (fixed)

@ShaneOnABike: This issue was closed almost a year ago. Please file a new issue, if you encounter any problems in the latest release.