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

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?

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

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

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

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) %}

Thanks you, merged.

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

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