So first off: love the module.

However, I have a small itch to scratch: if 'adjust top margin' is enabled, the background of the body element is shortly (~1 second) visible, before the admin menu is loaded. This usually means that it flickers from white to black.

Attached is a patch that introduces an empty shell before the actual admin menu is loaded.

Before: offset -> admin_menu loads and fills the offset
After: offset -> admin_menu empty shell that fills the offset -> admin_menu replaces empty shell

Comments

kiphaas7’s picture

StatusFileSize
new6.69 KB

Small illustration to further clarify the problem and what the patch does.

sun’s picture

I think we actually should rather output the div#admin-menu wrapper unconditionally in page_top, and only "fill it in" later on.

Meaning, we'd remove the div#admin-menu wrapper from the cached output.

If possible, we shouldn't break the admin_menu output on 403 and 404 pages by doing so.

kiphaas7’s picture

Probably, but that would still require a class switch on the wrapper (empty wrapper does not have height).

I'll try to look into it, but I never delved deep in the inner workings of admin_menu before...

kiphaas7’s picture

StatusFileSize
new3.43 KB

Don't know if this is entirely correct: please check if this is what you meant...

Visually, for my setup, it works identical to my first patch.

However, with this patch at least some minor problems/question marks exist:

  1. $page['page_top']['admin_menu']content does not run through drupal_render (or drupal_alter).
  2. Not sure if the following change is fully correct:
    -    if (!$adminMenu.length && settings.admin_menu.hash) {
    +    if (!$('#admin-menu-wrapper', context).length && settings.admin_menu.hash) {
    
  3. Don't know if this breaks on 403 or 404 pages. In fact, I don't know what the implications would be on those pages with this patch.

Status: Needs review » Needs work

The last submitted patch, placeholder_2.patch, failed testing.

truls1502’s picture

Issue summary: View changes
Status: Needs work » Postponed (maintainer needs more info)
Issue tags: +postponed2w

I am sorry for no reply until now.

There are many issues regarding this module admin_menu which is a bit difficult for us to follow up since some of the issues might be already outdated, or is already fixed by the module or any other modules or itself core which means that the problem might no longer need to be fixed.

We can see that the issue has been created for a few years ago, I hope it is okay for you that I am postponing the issue, and give you around two weeks. If you still face the problem, could you tell us the step by step when until you get the error message or what is frustrated you, and a list of modules you are using related to admin_menu and a screenshot that might help us? So it makes us easier to reproduce your issue.

However, after two weeks with no feedback - we will close this issue. So in case, you noticed it after the issue is closed, do not hesitate to reopen it like and fill information which is mentioned above.

So before giving us a feedback, do you mind to test it again with our latest 7.x-3.x-dev?

Thank you for understanding! :)

truls1502’s picture

Status: Postponed (maintainer needs more info) » Closed (cannot reproduce)
Issue tags: -postponed2w

This issue has been automatically marked as closed because it has not had recent activity after the last post.

However, if you or someone is still facing the same issue as described to the issue, could you please to re-open the issue by changing the status of the issue, and add an explanation with more details which can help us to reproduce your situation.

Again, thank you for your contributions! :)