Closed (fixed)
Project:
UI Suite DSFR
Version:
1.0.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
23 Aug 2023 at 16:29 UTC
Updated:
23 Aug 2024 at 07:30 UTC
Jump to comment: Most recent
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
spryahI'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
spryahComment #15
pdureau commentedThanks you, merged.
Comment #17
spryahHello @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?