This is probably a fringe issue few people would encounter, but I thought I'd mention it:

We have a site that has no main menu on most pages, including the home page. However, there are two landing pages that each include a different Ultimenu block.

We've found that the "hamburger" button is appearing at the bottom of every page, whether an Ultimenu block appears on the page or not. We can certainly style it to hide it, or implement HOOK_page_bottom, but I'm wondering if the _ultimenu_is_applicable function should do a more thorough check, and actually look for an active Ultimenu block on the page. I can probably submit a patch for that if you think it's appropriate; if not, I'll tackle it in our custom theme.

Comments

wrd-oaitsd created an issue. See original summary.

gausarts’s picture

Agreed, bad given that test.

Good use case, thanks.
Never crossed my mind.

Patches are welcome to make ultimenu smarter :)

wrd-oaitsd’s picture

I'll give it a shot, thanks!

wrd-oaitsd’s picture

Ha, well, this is rather more complicated than I expected. I think it would be relatively easy to check the current path against any visibility rules in blocks found within the theme regions, but that doesn't cover the possibility that an Ultimenu block has been placed in Layout Builder, or worse, if an Ultimenu block has been placed within a layout for another entity which then itself was placed as a block in Layout Builder...etc.

The only place I can think of where all the necessary information could be found is within HOOK_preprocess_page, but we probably don't want to preprocess every page for this, do we? I wonder if any solution to figuring out whether an Ultimenu block is going to be rendered would be more expensive than rendering unnecessary markup.

gausarts’s picture

Makes sense :)

Not sure about the complication (CSS fixed positioning, etc.), but perhaps having a dedicated block for hamburger is worth exploring?

This way no more worries about the OP issues.
The only reason the way hamburger is placed into page_bottom is fixed position issues. But seeing Jumper module can cope with a little JS at its latest release, we can apply it to Ultimenu just as well.

One thing that prevented me from doing what Jumper did was FOUC.

Unlike Jumper block which was hidden inititially, hamburger may always be visible if so configured. By default, it is hidden like Jumper, though.

But perhaps no big deal or FOUC since hamburger is not affecting layout. It is just a button. It can appear another second.

And IIRC, most js-dependent megamenu uses the same trick that Jumper did -- that is, move/ append the hamburger from any static/ designated block into before closing body when js kicks in.

If that risks issues with existing installs, we should move it into a new branch, unless we can do a switch with old approach.

Overall, less works, more flexibility, but a detail doc is required since hamburger is no longer auto-populated.

Thoughts?

wrd-oaitsd’s picture

Nice idea -- puts control of whether or not the block appears on a page into the site owner's hands. I like the idea of a switch, kind of like the Views "exposed filter in block" option; people are already used to that kind of behavior, and by default there's no change to current behavior.

gausarts’s picture

Ok. We can also simplify the works a bit.

Since hamburger is coupled with main menu at its current limited not-so-flexible design, we can even embed it as a kind of #suffix to main menu. Then we can move it before closing body.

This way, your issues are solved. Old behaviors are retained. No breaking changes, hopefully.

Perhaps three tasks:

  • Remove hamburger from page_bottom,
  • Move hamburger's markup as main menu #suffix, or alike. Not sure where it is located now :) Likely at the current pre_render, or preprocess at worst.
  • Add a small js to move it to closing body to retain fixed position correctly. To make it work for both jQuery and vanilla js, we can make it at as a shared method to call for at ultimenu.base.js, or alike IIRC :)

We should also attach js and css libraries into main menu. But not urgent for now. We can always refine when things are in place correctly.

wrd-oaitsd’s picture

StatusFileSize
new10.9 KB

Here's a first attempt. It works, as far as I can tell, though I'm not certain I did it the way you wanted.

gausarts’s picture

Yes, thank you!

Perhaps just two minor issues:

  • ES6 const needs changing to old var.
  • Null check is needed since Drupal.attachBehaviors is likely called many times, and might cause dups due to multiple appendChild being repeatedly called. Perhaps your lines should be wrapped with if (document.querySelector('body > #ultimenu-main') === null)

The rest is better than what I initially thought.

Thanks again for your works.
I'll commit it anytime this week :)

wrd-oaitsd’s picture

StatusFileSize
new11.03 KB

Here's an updated patch!

  • gausarts committed 575421d on 8.x-2.x authored by wrd-oaitsd
    Issue #3261137 by wrd-oaitsd: Should _ultimenu_is_applicable check for a...
gausarts’s picture

Status: Active » Fixed

Committed with minor changes.

Thank you for contribution.

wrd-oaitsd’s picture

Terrific -- thank you!

Status: Fixed » Closed (fixed)

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