[beta5] RE: Missing active trail inside side menu pattern (lvl.3)

Created on 23 August 2023, 10 months ago
Updated 3 October 2023, 9 months ago

Problem/Motivation

Following issue #3375273 🐛 [beta4] Missing active trail inside side menu pattern Fixed .
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.

🐛 Bug report
Status

Fixed

Version

1.0

Component

Code

Created by

🇫🇷France spryah

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @spryah
  • @spryah opened merge request.
  • Status changed to Needs review 10 months ago
  • Assigned to pdureau
  • Issue was unassigned.
  • Status changed to Needs work 10 months ago
  • 🇫🇷France pdureau Paris

    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?

  • Assigned to pdureau
  • Status changed to Needs review 10 months ago
  • 🇫🇷France pdureau Paris

    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

  • Issue was unassigned.
  • Status changed to Needs work 10 months ago
  • 🇫🇷France pdureau Paris

    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.

    #334507: CSV export: Option to show header row is not merged yet, so we are open to suggestion if you prefer to manage those attributes differently.

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

  • 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 Add links setting type Needs review

  • Status changed to Needs review 9 months ago
  • 🇫🇷France spryah

    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

  • Status changed to Needs work 9 months ago
  • 🇫🇷France pdureau Paris

    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) %}
  • Status changed to Needs review 9 months ago
  • Status changed to Fixed 9 months ago
  • 🇫🇷France pdureau Paris

    Thanks you, merged.

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

Production build 0.69.0 2024