- Issue created by @spryah
- @spryah opened merge request.
- Status changed to Needs review
10 months ago 10:14am 24 August 2023 - Assigned to pdureau
- Issue was unassigned.
- Status changed to Needs work
10 months ago 2:59pm 29 August 2023 - 🇫🇷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 10:01am 4 September 2023 - 🇫🇷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 2:12pm 5 September 2023 - 🇫🇷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 12:11pm 25 September 2023 - 🇫🇷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 7:52am 26 September 2023 - 🇫🇷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 8:37am 2 October 2023 - Status changed to Fixed
9 months ago 12:01pm 3 October 2023 Automatically closed - issue fixed for 2 weeks with no activity.