When a menu is rendered in the ul tag we can easily find the nav and menu class what is practical to use to target our styles; however if we need to target specific styles to specific menus we don't have the posibility to target that concrete menu.

In the template "templates/menu/menu.html.twig" the menu_name value is available but not included in the macro.

This is a patch that includes the menu machine name as attribute and will help with this need.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

casivaagustin created an issue. See original summary.

casivaagustin’s picture

bandanasharma’s picture

Status: Active » Needs review
bandanasharma’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
30.45 KB
25.36 KB

#2 patch is apply successfully. I have attached the after and before attachment.

markhalliwell’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: -CSS, -markup
+++ b/templates/menu/menu.html.twig
@@ -21,15 +21,15 @@
+      <ul{{ attributes.addClass('menu', 'nav', menu_name) }}>
...
+      <ul{{ attributes.addClass('dropdown-menu', menu_name) }}>

Since this is a variable (machine name), then it should use the |clean_class filter as well when adding it.

markhalliwell’s picture

@bandanasharma, taking screenshots of code changes isn't necessary and honestly, a little confusing; that is what the patch is for.

Screenshots are typically only useful for changes that affect the UI/site itself.

casivaagustin’s picture

Here goes a patch for the comment #6.

casivaagustin’s picture

Status: Needs work » Needs review
casivaagustin’s picture

Bump: Ready for Review

gerzenstl’s picture

The last patch looks good.

sylus’s picture

I might have missed something but I see that the following code was directly removed? Was that by design?

 <a href="{{ item.url }}" class="dropdown-toggle" data-target="#" data-toggle="dropdown">{{ item.title }} <span class="caret"></span></a>
markhalliwell’s picture

Status: Needs review » Needs work

There was a lot of markup/line changes between #2 and #7. I just asked that the |clean_class filter was applied. I don't think the level of complexity in #7 is needed.

casivaagustin’s picture

Here goes a new patch, like #2 but adding the clean_class

Thanks for the feedback

casivaagustin’s picture

Status: Needs work » Needs review
markhalliwell’s picture

Status: Needs review » Needs work
+++ b/templates/menu/menu.html.twig
@@ -21,15 +21,16 @@
+{% set menu_name = menu_name | clean_class  %}

Never override existing variables in templates. This can cause unintended and unforeseen issues when attempting to extend or override in sub-themes.

The filter should be used like the following (without spaces around the pipe):
<ul{{ attributes.addClass('dropdown-menu', menu_name|clean_class) }}>

casivaagustin’s picture

D'oh!, Here it goes without override the variable.

Thanks for the feedback

casivaagustin’s picture

Status: Needs work » Needs review
casivaagustin’s picture

One more time, I miss out something from the comment.

markhalliwell’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

markhalliwell’s picture

Just an FYI: #2936370: Consolidate menu macro in main template

This adds a menu-- prefix to the menu name.