[1.0.2] Notice pattern evolution

Hi Yassine,

That's a good idea, but:

• let's avoid a constant value in an HTML ID attribute: {% set block_id = 'fr-notice--block-id' %}
• let's avoid using random() function outside a default() filter: {% set block_id = 'fr-notice--' ~ random() %}
• the name of the new prop must not be isBlock but something like permanent_state, because of the expected feature

So, a proposal:

    permanent_state:
      type: boolean
      label: Permanent state?
      description:  If true, the notice will remember its state in between page reloads. Expect a non random block_id value.
      preview: false

The wording proposal may change.

With:

{% set block_id = block_id|default('fr-notice--' ~ random()) %}
{% set attributes =attributes.setAttribute("id", block_id)  %}
{% set attributes = permanent_state ? attributes.setAtttibute("data-permanent-state", "true") : attributes %}
<div{{ attributes.addClass('fr-notice').addClass('fr-notice--' ~ variant)}}>

So:

• Can you rebuild your script around this proposal?
• Do you think upstream DSFR will accept such a proposal?
• I will need to ask other people to validate such a proposal because we are at the limit between application state and design system.

I have adapted the JavaScript script and added a new CSS file 'notice.css'.
I initialized .fr-notice[data-permanent-state="true"] { display: none; }, to prevent the notice from flickering every time the page changes.
I added the JavaScript and CSS files to the .libraries.yml file.

Hello,
I have a comment about the Javascript code added.
why a vanilla JS code instead of a proper drupal JS behavior?
The JS file is added to the global theme library, therefore in my understanding it should follow best practices → .
what do you think?
i can help in writing the code if needed.
thank you

Let's not enforce JS Behaviour here.

There are interesting talks in the Drupal community about using WebComponent (with the is attributes instead of the "custom element" syntax) to leverage WebComponents as a standard JS behaviours container where the lifecycle is managed by the browser.

Do you want to try?

Behaviours

The script to embed: https://git.drupalcode.org/project/ui_suite_dsfr/-/blob/aeb85b6a07c39f1e...

Example of behaviour:

Drupal.behaviors.myBehavior = {
    once('myCustomBehavior', 'input.myCustomBehavior', context).forEach(
      function (element) {
           // Your code is here
      }
    );
  }
};

Asset library management

In the MR, it is declared at the theme level: https://git.drupalcode.org/project/ui_suite_dsfr/-/blob/aeb85b6a07c39f1e...
With files in /css and /js folders

It would be better to add them directly in the component, so they are loaded only when the component is used:

• Move notice.css and notice.js to the component folder
• Move the library to https://git.drupalcode.org/project/ui_suite_dsfr/-/blob/aeb85b6a07c39f1e...

Hello Yassine,

Looking at https://git.drupalcode.org/project/ui_suite_dsfr/-/merge_requests/70

• the source branch was not been rebased
• there is a merge conflict
• you didn't use the Drupal behabiour notation for your javascript
• File mode changed from 100644 to 100755 on 124 unrelated files.

Let's work on the new branch: https://git.drupalcode.org/project/ui_suite_dsfr/-/merge_requests/111

There is also a feature update to do, because the component has evolved since first implementation:

Add missing variants

• title stays a slot, because backward compatibility. You can cerate a 1.1.x issue if you want to convert to prop later
• Add new string prop: description (a prop because of the P and SPAN element)
• Add an icon prop with a enum string ("select" in UIP1) (It will be replaced by an icon prop type in 1.1.x branch). What is happening when we also pick a variant with icon?

For fr-notice__link, 2 possibilities:

• • Add new string prop: button_label
  • Add new url prop: button_url
• Add a link slot with |add_class() filter

What we discussed today in the weekly meeting :

- for the moment, the notice title stays a slot, but i can be converted to prop when. switching to 1.1.x branch
- description, link text, link url are introduced as props
- link title attribute may be a prop with a good default value in twig when injected empty
- icon will be a prop with the list of classed declared in the yml, as done in button component.
- the js code is a behavior with a behavior unique name like "ui_suite_dsfr_notice" for Drupal.behaviors.ui_suite_dsfr_notice

hello, according to our discussions with @yassrzg , i have modified the code.
please review :)

Eslint error: https://git.drupalcode.org/project/ui_suite_dsfr/-/jobs/3218389

----

{% set base_url = base_url %}
{% set is_external = base_url not in link_url %}
...
 {% if link_url and link_text %}<a href="{{ link_url }}" class="fr-notice__link" title="{{ link_description|default(link_text) }}" {% if is_external %} target="_blank" rel="noopener external" {% else %} target="_self" rel="noreferrer"{% endif %} >{{ link_text }}</a>{% endif %}

This is a mess, there are 3 props, 5 variables, and too much logic for a simple link. And {% set base_url = base_url %} is useless.

Why not a simple slot expecting a link element renderable:

{{ link.addClass("fr-notice__link") }}

----

Instead of hardcoding Button component markup:

<button class="fr-btn fr-btn--close" title="{{ close_title }}">{{ close_title }}</button>

We can embed the component directly:

{{ pattern("button", {
  label:  close_title,
 title: close_title
}) }} 

But we need to add the --close modifier to button component. Not as a variant because it can be combined with existing variants. So, as a boolean prop.

I will propose something

I have pushed a commit with my proposal

yes you are right Pierre, we need to modify the code to target 1.11 only