Problem/Motivation

New to 11.3 is a regression where Olivero's dropdown menus don't work if you're logged in, and Big Pipe is enabled (which it is by default).

This is likely a regression caused by changes to Big Pipe with HTMX. However the root cause is Olivero's. /core/themes/olivero/js/second-level-navigation.js does not use Drupal Behaviors 🙈

Steps to reproduce

  1. Install default Drupal
  2. Add a nested menu structure
  3. Be logged in, and try to access a nested menu item. It doesn't work

Proposed resolution

Lets use behaviors in /core/themes/olivero/js/second-level-navigation.js

Release notes snippet

Issue fork drupal-3568566

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mherchel created an issue. See original summary.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

Crediting @mandclu who reported the issue at https://drupal.slack.com/archives/C0D5GJZ8B/p1768834175431069

mherchel opened merge request !14428

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Active » Needs review
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

The MR above moves the event listeners into a new Drupal behavior.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Issue summary: View changes
mandclu’s picture

mandclu commented
Status: Needs review » Reviewed & tested by the community

I tested this on a fresh install of Drupal 11.3. Where previously it would show just the context menu but not the dropdown for nested items in the main navigation on hover, with the patch applied the menu works as expected even when logged in as an admin.

godotislate’s picture

godotislate
he/him
commented

I was curious about why HTMX would make a difference, if the issue was that Olivero did not use behaviors, so I did some investigation with these steps:

  1. Install core with standard
  2. Go to /node/add/article
  3. Full in a title, open the menu settings, check Provide a link, set the Parent link to Home, save
  4. Observe hover behavior of Home link
  5. Observe behavior of Home link after reload

On 11.2.x, the hover behavior of the home link on the first cold page load does not work as expected. It does work subsequently on reloads. This does confirm it's Big Pipe related, but 11.2.x does not use HTMX for Big Pipe.

I also tested against 11.1.x, and the hover behavior of the home link on the first cold page load does work as expected. If anything, either the issue was introduced between 11.1.x and 11.2.x (not going to do a git bisect now) or it was a race condition.

catch’s picture

catch
he/him
Primary language English
commented

Based on #8, pretty sure it was #3437499: Use placeholdering for more blocks - before that issue, the main menu would be rendered with the main content, after that issue, on cache misses, it's rendered via big pipe, on cache hits, as part of the main content again (due to another change that landed in 11.2).

lauriii made their first commit to this issue’s fork.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

Reviewed the patch and made the following improvements:

  1. Renamed init(el) parameter to init(navElement) to avoid variable shadowing with the el used inside the forEach callbacks
  2. Added documentation to the init() function
  3. Wrapped the document-level keyup and touchstart event listeners with once() to prevent duplicate handlers when the behavior is attached to multiple elements
godotislate’s picture

godotislate
he/him
commented

Tested updated MR locally with same steps per #8 and still works as expected.

  • lauriii committed cdd9f9c3 on main 
    fix: #3568566 Olivero's secondary menus inoperable when authenticated...

  • lauriii committed 7cffd025 on 11.x 
    fix: #3568566 Olivero's secondary menus inoperable when authenticated...

  • lauriii committed b638f429 on 11.3.x 
    fix: #3568566 Olivero's secondary menus inoperable when authenticated...

lauriii closed merge request !14428

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

Thank you for the review @godotislate! Committed 🚀

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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

kentr’s picture

kentr commented

I think this caused a regression: #3584314: Mobile menu doesn't close by ESC key with search block + empty main menu