Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not preprocess navigation_menu. #405

Open
Joaoeduardoramos opened this issue Jul 3, 2024 · 2 comments
Open

Do not preprocess navigation_menu. #405

Joaoeduardoramos opened this issue Jul 3, 2024 · 2 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@Joaoeduardoramos
Copy link
Contributor

With the introduction of the core 'navigation' module from 10.3 onwards, the 'navigation_menu' theme is introduced. Like the toolbar, it is a menu more focused on website administration.

See https://www.drupal.org/node/3443691 .

@drishu drishu added enhancement New feature or request question Further information is requested labels Aug 19, 2024
@drishu
Copy link
Contributor

drishu commented Aug 19, 2024

Can you explain more on how the processing affects the menu, what the exact issue is that the pr fixes?

@Joaoeduardoramos
Copy link
Contributor Author

Can you explain more on how the processing affects the menu, what the exact issue is that the pr fixes?

Hi @drishu ,

the preprocess create an array structure that is not compatible with the navigation module menu items.
Producing the following warning (at least on my side):

"Undefined array key "url" in Drupal\oe_bootstrap_theme\MenuPreprocess->menuLink() (line 165 of themes/contrib/oe_bootstrap_theme/src/MenuPreprocess.php)."

Also, the module provides its own template for concerning menu: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/navigation/templates/navigation-menu.html.twig

Finally, I believe it's the same rationale applies to the removal of the 'menu__toolbar' hook. It’s a menu primarily intended for website administration, and I would say it should not be confused with the default theme if they do not match.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants