Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: 🐛 DsfrCallout description critere 8.9 RGAA 4.1.2 NC #981

Merged

Conversation

MaitreManuel
Copy link

Bonjour à tous,

Une petite modification du DsfrCallout car on a eu une NC sur le critère 8.9 RGAA et ça nous a posé reflexion.

Dans notre cas, nous utilisons le slot par default pour la description obligatoire et nous n'utilisons pas la props content.
Sauf que dans votre implémentation cette description obligatoire se situe dans la balise <p class="fr-callout__text">.
On se retrouve de notre côté avec une balise vide qui remonte en NC.

Dans l'implémentation que je propose, on teste si content est défini, si oui, le comportement actuel est conservé, sinon fr-callout_text devient une classe du wrapper du slot par défaut.

Et si on a et props.content et $slots.default, alors la balise p sera la détentrice du fr-callout_text pour la description, et le slot par défault un simple contenu additionnel.

Au final, on respecte les spécifications DSFR du composant "Mise en avant" et on ne remonte pas de NC RGAA.

Merci pour votre lecture !

Copy link

netlify bot commented Nov 26, 2024

Deploy Preview for vue-dsfr-demo ready!

Name Link
🔨 Latest commit 0176363
🔍 Latest deploy log https://app.netlify.com/sites/vue-dsfr-demo/deploys/674580dccc81fa0008c6c673
😎 Deploy Preview https://deploy-preview-981--vue-dsfr-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@laruiss laruiss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oui, ce v-if dans le p aurait dû y être dès la première implémentation, en effet !
Merci !

Comment on lines 52 to 57
<div
v-if="$slots.default"
:class="['fr-callout__default', { 'fr-callout__text': !content }]"
>
<slot />
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pourquoi cette classe fr-callout__default ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C'était pour cibler le wrapper du slot default, si on est en contenu additionnel pur

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ça me gêne d’utiliser un nom de classe qui fait penser qu’elle existe dans le CSS du DSFR alors que ce n’est pas le cas.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je comprends j'étais moi-même peu convaincu, c'était pour eviter d'avoir une div sans attribut qui fait tâche ou une condition un peu capillotracté du style :

<div
  v-if="$slots.default && !content"
  class="'fr-callout__text"
>
    <slot />
</div>
<slot v-else />

Quoi que c'est pas si terrible finalement

Copy link
Collaborator

@laruiss laruiss Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oui, je préfère cette solution avec le v-if="$slots.default && !content"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modification faite ✔️

@laruiss laruiss changed the base branch from main to develop December 5, 2024 13:54
@laruiss
Copy link
Collaborator

laruiss commented Dec 5, 2024

Il faut partir de develop et demander la fusion dans develop. La branche est a rebaser sur develop

@MaitreManuel MaitreManuel force-pushed the dsfr-callout-default-slot-vs-text branch from 0176363 to feff7a7 Compare December 5, 2024 13:57
Copy link

netlify bot commented Dec 5, 2024

👷 Deploy request for vue-dsfr pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 2064d39

Copy link

netlify bot commented Dec 5, 2024

Deploy Preview for docs-vue-dsfr ready!

Name Link
🔨 Latest commit 2064d39
🔍 Latest deploy log https://app.netlify.com/sites/docs-vue-dsfr/deploys/6751b1cb74d68a0008c3c969
😎 Deploy Preview https://deploy-preview-981--docs-vue-dsfr.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@MaitreManuel MaitreManuel force-pushed the dsfr-callout-default-slot-vs-text branch from feff7a7 to 2064d39 Compare December 5, 2024 13:59
Copy link

netlify bot commented Dec 5, 2024

Deploy Preview for demo-vue-dsfr ready!

Name Link
🔨 Latest commit 2064d39
🔍 Latest deploy log https://app.netlify.com/sites/demo-vue-dsfr/deploys/6751b1cb9b46d40008325bdd
😎 Deploy Preview https://deploy-preview-981--demo-vue-dsfr.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@MaitreManuel
Copy link
Author

Il faut partir de develop et demander la fusion dans develop. La branche est a rebaser sur develop

C'est tout bon, fait à l'instant

@laruiss laruiss merged commit 4c9762e into dnum-mi:develop Dec 18, 2024
8 of 9 checks passed
Copy link

🎉 This PR is included in version 7.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants