Problem/Motivation

Following issue #3375273.
We're missing the desired markup on level 3 items.

Steps to reproduce

  1. In the side menu preview, move the in_active_trail option to a menu with 3 levels:
            - title: Home
              url: "/"
            - title: Library
              url: "#"
              below:
                - title: Library Sub 1
                  url: "#"
                - title: Library Sub 2
                  url: "#"
            - title: Data
              url: "#"
              in_active_trail: true
              below:
                - title: Data Sub 1
                  url: "#"
                  below:
                    - title: Data Sub 1.1
                      url: "#"
                    - title: Data Sub 1.2
                      url: "#"
                - title: Data Sub 2
                  url: "#"
                  in_active_trail: true
                  below:
                    - title: Data Sub 2.1
                      url: "#"
                    - title: Data Sub 2.2
                      url: "#"
                      in_active_trail: true
    
  2. In the preview page, open accordions "Data" (lvl 1) and "Data Sub 2" (lvl 2).
  3. Observe the missing styles and markup on the last item, "Data Sub 2.2".

Proposed resolution

Fix side menu pattern Twig file, add the active trail mode to a level 3 item.

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:

Comments

spryah created an issue. See original summary.

keshavv’s picture

Status: Active » Needs review
pdureau’s picture

Assigned: Unassigned » pdureau
pdureau’s picture

Assigned: pdureau » Unassigned
Status: Needs review » Needs work

Thanks a lot for your proposal.

1. responsive_collapse_title string prop ("textfield setting)" is not defined in the YML definition file:

{% set responsive_collapse_title = responsive_collapse_title|default('In this section'|t) %}

2. Please don't use |raw() filter: it is unsafe and unnecessary because aria_current & aria_current_page are defined in the Twig template as string.

3. Not because of your MR but we need to fix that anyway because deeply related: attributes and item.attributes objects are not used in menu_links() macro.
You may need to leverage at least one of them, for example (humble & not tested proposal):

<li{{ item.attributes.addClass(['fr-sidemenu__item ', class_active_trail) }}>

4. aria_current & aria_current_page are defined outside item.below condition but used only if the condition is let or not. Can you move them in the condition?

pdureau’s picture

Title: RE: Missing active trail inside side menu pattern (lvl.3) » [beta4] RE: Missing active trail inside side menu pattern (lvl.3)
pdureau’s picture

Assigned: Unassigned » pdureau
Status: Needs work » Needs review

Not because of your MR but we need to fix that anyway because deeply related: attributes and item.attributes objects are not used in menu_links() macro.
You may need to leverage at least one of them, for example (humble & not tested proposal):

See how https://www.drupal.org/project/menu_link_attributes & the new setting type are doing that

pdureau’s picture

Assigned: pdureau » Unassigned
Status: Needs review » Needs work

Hi Sorya,

In the soon-to-be-released "links" (previously "menu" but shared with breadcrumbs and pagers) setting type we have introduced an new property for item, link_attributes:

attributes: HTML attributes for the menu item.
below: The menu item child items.
title: The menu link title.
link_attributes: HTML attributes for the link
url: The menu link URL as string
localized_options: Menu link localized options.
is_expanded: TRUE if the link has visible children within the current menu tree.
is_collapsed: TRUE if the link has children within the current menu tree that are not currently visible.
in_active_trail: TRUE if the link is in the active trail.

#3345071: Add links setting type is not merged yet, so we are open to suggestion if you prefer to manage those attributes differently.

FYI, related issue: #3381502: [beta5] Handle target/title in footer menu links

pdureau’s picture

Title: [beta4] RE: Missing active trail inside side menu pattern (lvl.3) » [beta5] RE: Missing active trail inside side menu pattern (lvl.3)
mh_nichts’s picture

Just to fix the link : the related issue about link_attributes (new links setting type) is https://www.drupal.org/project/ui_patterns_settings/issues/3345071

mh_nichts’s picture

Status: Needs work » Needs review
spryah’s picture

I've made a lot of refactoring and I also made it so the state of the collapsible can be configured using the is_expanded variable.
Please let me know if the new version meets your standards

pdureau’s picture

Status: Needs review » Needs work

Thanks. It looks great.

I am just worrying about that:

          {% set link_attributes = item.link_attributes|default({}) %}
          {% set link_attributes = create_attribute() %}

It is always creating an empty attribute object, no?

Maybe you mean that instead?

          {% set link_attributes = item.link_attributes|default({}) %}
          {% set link_attributes = create_attribute(link_attributes) %}

Because of what you did for:

        {% set item_attributes = item.attributes|default({}) %}
        {% set item_attributes = create_attribute(item_attributes) %}
spryah’s picture

Status: Needs work » Needs review
pdureau’s picture

Status: Needs review » Fixed

Thanks you, merged.

Status: Fixed » Closed (fixed)

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

spryah’s picture

Hello @pdureau,
I never had the chance to use this pattern and now that i'm looking at the differences in the merge request, it seems to me that the commits were somehow lost in a parallel universe?
Should I try to rebase or create another branch?