Separate toolbar and dropdown menu markup [#1564934]

Problem

#742184: Shortcut.module integration introduced a widget/add-on to the toolbar that isn't a dropdown menu and thus needs to revert a lot of CSS styles to work correctly.

Goal

Pave the way towards non-menu widgets.

Proposed solution

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

Notes

Related: http://drupal.org/project/admin_deck

Comment	File	Size	Author
#12	active link not updating.png	23.08 KB	JSCSJSCS
#11	admin_menu.shortcut-css.11.patch	778 bytes	sun
#11
#7	fresh cache clear.png	13.34 KB	JSCSJSCS
#7	after two or three shortcut clicks.png	12.13 KB	JSCSJSCS
#3	initial test.png	3.35 KB	JSCSJSCS
#3	other menu.png	11 KB	JSCSJSCS
#3	reduced browser.png	13.11 KB	JSCSJSCS
#2	admin_menu.dropdown.2.patch	16.97 KB	sun
#2
#1	admin_menu.dropdown.1.patch	17.54 KB	sun
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

sun

German

Karlsruhe

CreditAttribution: sun commented 6 May 2012 at 23:47

Status:	Active	» Needs review
Issue tags:		+API change

File	Size
admin_menu.dropdown.1.patch	17.54 KB

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...

File	Size
admin_menu.dropdown.2.patch	16.97 KB

Comment #3

JSCSJSCS CreditAttribution: JSCSJSCS commented 7 May 2012 at 02:50

File	Size
reduced browser.png	13.11 KB
other menu.png	11 KB
initial test.png	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:

Comment #4

sun

German

Karlsruhe

CreditAttribution: sun commented 8 May 2012 at 17:55

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.

Comment #5

JSCSJSCS CreditAttribution: JSCSJSCS commented 8 May 2012 at 18:06

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.

Comment #6

sun

German

Karlsruhe

CreditAttribution: sun commented 8 May 2012 at 18:18

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.

Comment #7

JSCSJSCS CreditAttribution: JSCSJSCS commented 9 May 2012 at 14:28

File	Size
after two or three shortcut clicks.png	12.13 KB
fresh cache clear.png	13.34 KB

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.

Comment #8

JSCSJSCS CreditAttribution: JSCSJSCS commented 9 May 2012 at 14:39

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.

Comment #9

JSCSJSCS CreditAttribution: JSCSJSCS commented 9 May 2012 at 15:00

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.

Comment #10

sun

German

Karlsruhe

CreditAttribution: sun commented 9 May 2012 at 16:38

#9 is already known, filed under #1567622: Additional attached JS/CSS is missing in cached output

Comment #11

sun

German

Karlsruhe

CreditAttribution: sun commented 11 May 2012 at 15:43

File	Size
admin_menu.shortcut-css.11.patch	778 bytes

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

Committed attached patch.

Comment #12

JSCSJSCS CreditAttribution: JSCSJSCS commented 11 May 2012 at 16:03

File	Size
active link not updating.png	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