Problem/Motivation
Following issue #3375273.
We're missing the desired markup on level 3 items.
Steps to reproduce
- In the side menu preview, move the
in_active_trailoption 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 - In the preview page, open accordions "Data" (lvl 1) and "Data Sub 2" (lvl 2).
- 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.
Issue fork ui_suite_dsfr-3382940
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
Comment #3
keshavv commentedComment #4
pdureau commentedComment #5
pdureau commentedThanks 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?
Comment #6
pdureau commentedComment #7
pdureau commentedSee how https://www.drupal.org/project/menu_link_attributes & the new setting type are doing that
Comment #8
pdureau commentedHi 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:
#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
Comment #9
pdureau commentedComment #10
mh_nichtsJust 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
Comment #11
mh_nichtsComment #12
spryah commentedI've made a lot of refactoring and I also made it so the state of the collapsible can be configured using the
is_expandedvariable.Please let me know if the new version meets your standards
Comment #13
pdureau commentedThanks. It looks great.
I am just worrying about that:
It is always creating an empty attribute object, no?
Maybe you mean that instead?
Because of what you did for:
Comment #14
spryah commentedComment #15
pdureau commentedThanks you, merged.
Comment #17
spryah commentedHello @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?