🇫🇷France @mh_nichts

This issue affects also the preprocess naming : it works only with underscores (not possible to use hyphens).

I guess it's more a documentation issue then : it shouldn't be allowed to use hyphens in a pattern id ?

Thanks for the feedback.
Nevertheless, I think the fix is worth being handled for v1 : to be able to use hyphens in patterns id, and preview/variant templates would be correctly used.

g4mbini → credited mh_nichts → .

mh_nichts → created an issue.

I think maybe there is an exception to add when handling content which is a kind of special object ?

I posted a "reduced test case" but yes, the children of '#theme': layout__mytheme are only the regions names, and they aren't special keywords.

mh_nichts → created an issue.

@lkmorlan Thank you ! It should be OK now, I could apply the patch from the MR locally.

Hello,

Thanks for filing this issue.

However, I suggest solving it in another way, as using a span to wrap a div and a whole form is invalid according to W3C HTML standards.

My suggestion would be :

• in JS : remove '-content' from the selector, so it targets the initial wrapper div which effectively contains the form
• in PHP : keep span where it is, but add tabindex="-1" so it can effectively get the focus (currently it can't, whether wrapping the form or not)

I will try to commit this changes next week.

Hello,

I've seen the same error, but caused by the button pattern preprocess (visible on pages : /patterns or /patterns/button ou /patterns/button_group) :

TypeError: strtolower(): Argument #1 ($string) must be of type string, Drupal\Core\Render\Markup given in strtolower() (line 440 of themes/contrib/ui_suite_dsfr/ui_suite_dsfr.theme).

ui_suite_dsfr_preprocess_pattern_button(Array, 'pattern_button', Array)
...

It seems the same kind of condition causes the same error.

The strange thing is :

• if we comment those lines in ui_suite_dsfr but implement the same preprocess in our child theme, there is no error, everything works as expected
• I see the link pattern preprocess has the same kind of condition but it doesn't trigger any error...

I confirm that this preprocess is not obsolete, as it handles the "external" setting automatically based on the URL, so the best would be to find the cause of the error and fix it, if possible, instead of removing.

mh_nichts → created an issue.

mh_nichts → created an issue.

Hello,

Thanks to both of you for the quick actions !

However I'm afraid there is a typo in the fix, on line 217 : the condition should be on $variables['add_error'] instead of !$variables['add_error'] (= the error class should be added only if "add_error" is true).

mh_nichts → created an issue.

mh_nichts → created an issue.

mh_nichts → created an issue.

I've fixed the attributes, thanks.
For the title, it must always be present (it's used to label the modal) and it has a default('Transcription') so I don't think we need a condition ?

To avoid conflicts, this might be better done after https://www.drupal.org/project/ui_suite_dsfr/issues/3340340 ✨ [beta5] Update patterns & styles according to 1.8, 1.9 & 1.10 changelog Needs review (big content-media changes).

mh_nichts → created an issue.

I've created tickets for the other changes :

• https://www.drupal.org/project/ui_suite_dsfr/issues/3390150 🐛 Replace fielset__content with fieldset__element system (1.8 changelog) Active (1.8)
• https://www.drupal.org/project/ui_suite_dsfr/issues/3390160 ✨ Handle status/error messages in form wuth messages-group (1.8 changelog) Active (1.8)
• https://www.drupal.org/project/ui_suite_dsfr/issues/3390145 🐛 Update default title levels (1.8 changelog) Active (1.8)
• https://www.drupal.org/project/ui_suite_dsfr/issues/3390166 🐛 Side-menu : replace aria-label by aria-labelledby (1.8 changelog) Active (1.8)
• https://www.drupal.org/project/ui_suite_dsfr/issues/3390173 ✨ Upload : new modifiers (1.8 changelog) Active (1.8)
• https://www.drupal.org/project/ui_suite_dsfr/issues/3390174 ✨ [1.1.0] Password input (1.8 changelog) Active (1.8)
• https://www.drupal.org/project/ui_suite_dsfr/issues/3390179 ✨ New icons (1.9 & 1.10 changelogs) Active (1.9 & 1.10)
• https://www.drupal.org/project/ui_suite_dsfr/issues/3390189 ✨ Replace download-block/download-link with card--download/link--donwload/tile--download (1.10 changelog) Active (1.10)
• https://www.drupal.org/project/ui_suite_dsfr/issues/3390190 ✨ Tile adjustments (1.10 changelog) Active (1.10)

mh_nichts → created an issue.

mh_nichts → created an issue.

mh_nichts → created an issue.

mh_nichts → created an issue.

mh_nichts → created an issue.

mh_nichts → created an issue.

mh_nichts → created an issue.

mh_nichts → created an issue.

mh_nichts → created an issue.

I added the new "transcription" pattern, to use it in content-media.
I also made small fixes for accessibility in content-media (as mentioned in DSFR documentation) :

• use figure also for image (previously only for video)
• role group on figure only if there is an associated caption or transcription
• add aria-label on figure for caption

I will see for creating separate issues for the other changes.
Maybe we will have to change the title of this issue ?

I'm willing to work on the content-media pattern (with transcription), which is the main change in v1.8.

I think it will be easier to create separate issues for the other changes (one issue per minor version at least), because there are too many !

Changes for 1.8.0 : https://github.com/GouvernementFR/dsfr/releases/tag/v1.8.0
Changes for 1.8.1 : https://github.com/GouvernementFR/dsfr/releases/tag/v1.8.1 (nothing for us, it seems)
Changes for 1.8.2 : https://github.com/GouvernementFR/dsfr/releases/tag/v1.8.2
Changes for 1.8.4 : https://github.com/GouvernementFR/dsfr/releases/tag/v1.8.4
Changes for 1.8.5 : https://github.com/GouvernementFR/dsfr/releases/tag/v1.8.5 (nothing for us, it seems)

Changes for 1.9.0 : https://github.com/GouvernementFR/dsfr/releases/tag/v1.9.0
Changes for 1.9.1 : https://github.com/GouvernementFR/dsfr/releases/tag/v1.9.1
Changes for 1.9.2 : https://github.com/GouvernementFR/dsfr/releases/tag/v1.9.2 (nothing for us, it seems)
Changes for 1.9.3 : https://github.com/GouvernementFR/dsfr/releases/tag/v1.9.3

Changes for 1.10.0 : https://github.com/GouvernementFR/dsfr/releases/tag/v1.10.0
Changes for 1.10.1 : https://github.com/GouvernementFR/dsfr/releases/tag/v1.10.1

I've handled link_attributes in the same way as the soon-to-come links setting, but the code will have to be deleted once it's available, so I let a "@TODO" for that in a comment.

The code is now simplified, and handles only the target in preprocess : title is handled in Twig.
It works with the preview links as well as a "real" Drupal menu.

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

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 handled all the points, including conditions on operator_logo & service_title, in order to match the various link positions according to DSFR documentation.

As discussed together, we put a div to be sure of the appearance, and leave to the developer/webmaster the responsibility to use p tags for accessibility.

Sorry about all the separate pushes, but MR should be OK now :)

Issue créée côté DSFR pour aria-haspopup : https://github.com/GouvernementFR/dsfr/issues/744

mh_nichts → created an issue.

Thanks for your feedback.
I understand the roadmap to avoid preprocesses, but I'm wondering how we could do in this kind of cases (same as in link/button patterns), as we need to "check if URL is external" and this is currently done with Drupal functions. Except for this part, I thing the rest (change title according to target) could be handled easily in the Twig file.

Thanks for your comment.
You're right that the root cause is the missing child element : any element (like div) would do to match the expected appearance. However, it should be a p tag for accessibility reasons : this is a text and should semantically tagged as such, in order to be correctly handled by assistive technologies (if it's only a div, it could be completely bypassed in some situations, and the info would be missed).
That's also why there should be a p tag according to the DSFR documentation : https://www.systeme-de-design.gouv.fr/elements-d-interface/composants/pi...

I agree that the best would be to handle the child tag in the Twig file, but I thought you wanted to let the freedom to site builders/developers to put any HTML text they wanted (maybe several p tags), that's why I handled it only in the preview.
Your suggestion would solve this point, but as it's only a div element, as you said, most site builders/developers will still forget to put a p tag, so it will still be an accessibility fail...

I'm wondering, as this license text is mandatory (according to the DSFR documentation), maybe the best way to follow all the constraints is to include it completely in the Twig file (tag & translatable text) ? So we would be sure that it's not forgotten, it's the correct text, and the correct tag ? (but it wouldn't be customizable anymore)

I've made a proposal.
For the aria-haspopup attribute, I just changed it to "dialog" : I didn't delete it, and I didn't add one on the search "open" button, to stay closest to the DSFR documentation (https://www.systeme-de-design.gouv.fr/elements-d-interface/composants/en...). Nevertheless we will open an issue on the DSFR side to fix their code.

mh_nichts → created an issue.

mh_nichts → created an issue.

I've made a fix proposal, but I'm wondering :
- In the header pattern, the logo_title setting might be deleted (it's currently used for a condition on the title attribute on the p tag, I kept it but to handle on the a tag)
- In the DSFR header documentation (https://www.systeme-de-design.gouv.fr/elements-d-interface/composants/en...), there is no link on the logo, except when there is no "service title" : this condition is not handled in the pattern currently
- In the footer pattern, there is a condition to handle the link on the operator logo, but according to the DSFR footer documentation (https://www.systeme-de-design.gouv.fr/elements-d-interface/composants/pi...), the title attribute should then contain the "operator logo alt" (for accessibility reasons again : the title attribute should contain the visible text) : this isn't handled in the pattern currently, I think it would require an additional pattern field

Should I handle these changes on the same MR, or in a separate issue ?

mh_nichts → created an issue.

I've made a proposal (Twig changes & new preprocess, to test if the URL is external). The tricky part was handling both cases : a "real" Drupal menu link with attributes, and the "pattern preview" where the URL is a simple string.
I welcome any suggestion to make it better :)

mh_nichts → created an issue.