Motivation

Creating a site's primary navigation is one of the most time-consuming parts of creating a Drupal site. The primary nav needs to function both in desktop and mobile mode.

Tasks

Bring Olivero's basic menu into starterkit theme.

To decide

Should this be behind a theme setting?
If so, should it be enabled by default?

Issue fork drupal-3301623

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 opened merge request !2586

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Assigned: Unassigned » mherchel

Wide menu is functioning. Still need to work on mobile menu.

Assigning to myself. I plan to work on this through the next week or two.

mherchel’s picture

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

This should be close to ready (lets see if tests pass). Reviews welcome (even if tests do not pass).

mherchel’s picture

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

Tests are failing because of #3304731: Update remaining tests using Classy to use Starterkit

mherchel’s picture

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

Per @lauriii in https://drupal.slack.com/archives/C0D5GJZ8B/p1664283312014609,

I think in the long term we should convert tests away from starterkit_theme to either stark or testing themes but for now until that is done, we probably need to keep updating the tests

corn696’s picture

corn696
Primary language German
Location Flensburg
commented

Hi,

I am trying to integrate the olivero menu into our existing custom theme.
I thought this might be helpful to see what files are needed.

So i tried this out.

php core/scripts/drupal generate-theme --starterkit starterkit_theme my_custom_theme

But it doesn't seem to work. The icons for submenus are there, but there is no hover functionality on desktop and the + icon on mobile has no function either.

Just for interest, was there a discussion about a (contrib) module solution for the olivero menu?
I would guess, it's probably not an uncommon use case that someone wants to use the olivero menu with their existing theme.

mherchel’s picture

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

Make sure the "Expand all menu links" checkbox is selected within the block settings for the main navigation block (and the
"number of levels to display" is set to at least 2).

I'm willing to help out on a contrib solution for an olivero-like menu, but I don't have the personal bandwidth to do it on my own.

corn696’s picture

corn696
Primary language German
Location Flensburg
commented

Thanks for the quick response.

The checkbox is checked and number of levels to display is set to 2.
When I uncheck the box or set the level to 1, the arrow/plus icons disappear.

mherchel’s picture

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

Are there any messages in the browser JS console?

corn696’s picture

corn696
Primary language German
Location Flensburg
commented

No messages.

mherchel’s picture

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

🤔

Does it work when you disable JS (There's special styles for it there)?

Does the markup contain the submenus (inspect via devtools, there should be a nested <ul> element.

Is the JS loaded? Look in the source for main-menu.js

I'm assuming that you applied the diff from above right?

corn696’s picture

corn696
Primary language German
Location Flensburg
commented

Does it work when you disable JS (There's special styles for it there)?

Dropdown on desktop works. But the plus sign on mobile has still no function.

Does the markup contain the submenus (inspect via devtools, there should be a nested <ul> element.

Yes.

Is the JS loaded? Look in the source for main-menu.js

Yes, when i disable "Aggregate JavaScript files".

I'm assuming that you applied the diff from above right?

I cloned the issue fork and the last shown commit is "No more JS build step."

Btw. the dropdown of the olivero theme is working but the mobile hamburger toggle throws an error:

Uncaught TypeError: Cannot read properties of null (reading 'classList')
    at toggleNav (navigation.js?v=10.1.0-dev:31:16)
    at HTMLButtonElement.<anonymous> (navigation.js?v=10.1.0-dev:47:7)

The hamburger toggle is not present in the generated starter theme.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

MR is behind a little bit but appears there are failures from the last commit.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs work » Needs review

Fixed nightwatch tests. Lets see if this comes up green.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative

Seems there some failures in the last commit.

Can re-test after those.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

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

gauravvvv’s picture

gauravvvv commented

Leaving to NW as tests still needs to be added

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

mortona2k opened merge request !11958

mortona2k opened merge request !11959

mortona2k changed the visibility of the branch 3301623-add-olivero-like-primary_10.4.x to hidden.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.