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

[BUGFIX] Réordonnancement des fieldset legend de Modulix (PIX-12382) #8835

Merged
merged 1 commit into from
May 22, 2024

Conversation

yannbertrand
Copy link
Member

@yannbertrand yannbertrand commented May 3, 2024

🦄 Problème

On a une erreur d’accessibilité (non détectée par Wave).

La balise legend doit être descendant direct du fieldset et ne pas avoir de wrapper. Nous avons des wrappers element-qrocm__header, element-qcu__header et element-qcm__header qu’il faut donc supprimer.

🤖 Proposition

  • Ne pas englober les legend dans une div,
  • Ajouter un aria-describedby

Source : cet échange.

🌈 Remarques

Tout ça est testable avec le validateur du W3C, voir : https://stackoverflow.com/questions/18065565/is-it-valid-html-to-add-a-link-to-a-fieldset-legend.

💯 Pour tester

Vérifier visuellement que rien n'a changé.

Vérifier au lecteur d'écran que le legend est correctement lu à la tabulation. Je n'ai pas réussi à le confirmer personnellement.

@pix-bot-github
Copy link

Une fois les applications déployées, elles seront accessibles via les liens suivants :

Les variables d'environnement seront accessibles via les liens suivants :

@yannbertrand yannbertrand force-pushed the PIX-12382-fix-legend-fieldset branch 2 times, most recently from 00e0c97 to b2d5efb Compare May 3, 2024 14:19
@dlahaye
Copy link
Contributor

dlahaye commented May 6, 2024

Est-ce que le test via https://validator.w3.org/ est valide ? étant donné que tous les grains ne sont pas affichés d'un coup au lancement de la page

@yannbertrand
Copy link
Member Author

Est-ce que le test via https://validator.w3.org/ est valide ? étant donné que tous les grains ne sont pas affichés d'un coup au lancement de la page

Je l'ai pas retesté avec 100% du contenu de Modulix effectivement. Dans le 3ème onglet de w3c validator, tu peux copier/coller des bouts de code, c'est comme ça qu'on a conclu à notre erreur https://validator.w3.org/#validate_by_input.

@dlahaye
Copy link
Contributor

dlahaye commented May 6, 2024

Dans le 3ème onglet de w3c validator, tu peux copier/coller des bouts de code, c'est comme ça qu'on a conclu à notre erreur https://validator.w3.org/#validate_by_input.

Ah bah en plus j'apprends des trucs ^^

@yannbertrand yannbertrand force-pushed the PIX-12382-fix-legend-fieldset branch from b2d5efb to 19f6e8d Compare May 6, 2024 08:32
@yannbertrand
Copy link
Member Author

yannbertrand commented May 6, 2024

Dans le 3ème onglet de w3c validator, tu peux copier/coller des bouts de code, c'est comme ça qu'on a conclu à notre erreur https://validator.w3.org/#validate_by_input.

Ah bah en plus j'apprends des trucs ^^

Du coup j'ai retesté, c'est aussi interdit d'avoir une balise p dans une legend 😢

<fieldset>
  <legend><p>coucou</p></legend>
  <p>truc</p>
</fieldset>

Les balises qu'on a le droit d'utiliser sont du "phrasing content" et référencées ici : https://html.spec.whatwg.org/multipage/dom.html#phrasing-content-2 :(

@yannbertrand yannbertrand marked this pull request as draft May 6, 2024 11:32
@yannbertrand yannbertrand force-pushed the PIX-12382-fix-legend-fieldset branch from 19f6e8d to da9ca66 Compare May 7, 2024 12:06
Comment on lines +2 to +3
// eslint-disable-next-line no-restricted-imports
import { click, find } from '@ember/test-helpers';
Copy link
Member Author

Choose a reason for hiding this comment

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

Je ne sais pas comment récupérer le formulaire pour vérifier la valeur de son aria-describedby avec testing library... Preneur de mieux !

Copy link
Contributor

Choose a reason for hiding this comment

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

Une PR est en cours sur le repo de testing-library sur le sujet.
A11yance/aria-query#547
C'est lié à ce bug-ci
testing-library/dom-testing-library#1293
D'ici là, la doc ne semble pas proposer d'alternatives qui puissent convenir.

Faute de mieux, ce que tu proposes me convient. Peut-être mettre un commentaire au-dessus de la ligne 31 avec le lien vers la PR de testing-library ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Pour info j'avais eu un process de pensée un peu différent. Sur MDN form n'a le rôle form que si il est décrit par un nom accessible, j'imagine que la complexité d'implémentation côté testing-library vient de là.

Copy link
Contributor

Choose a reason for hiding this comment

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

@yannbertrand ils entendent quoi par un nom accessible ?

Copy link
Member Author

Choose a reason for hiding this comment

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

J'ai pas lu mais la définition est linkée dans MDN, ça renvoie là : https://www.w3.org/TR/accname-1.1/#dfn-accessible-name

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Encore plus clair après notre discussion (après avoir lu mdn).

Par défaut la balise form n'a pas le rôle form. Le debugger d'accessibilité Firefox ne le constate pas, mais c'est bien visible sur Chrome qui annonce un role "Section".

On peut la faire devenir un form soit :

  • avec un role="form". VoiceOver ne dira rien de plus.
  • avec un title="xxx", VoiceOver annonce "xxx, group".
  • avec un aria-label="xxx", VoiceOver annonce "xxx, group".
  • avec un aria-labelledby="xxx", VoiceOver annonce "xxx, group".

Ces 4 méthodes permettent d'utiliser le getByRole('form') de testing-library.

@yannbertrand yannbertrand marked this pull request as ready for review May 7, 2024 14:10
@yannbertrand yannbertrand force-pushed the PIX-12382-fix-legend-fieldset branch from da9ca66 to 20356b8 Compare May 13, 2024 08:53
@yannbertrand yannbertrand force-pushed the PIX-12382-fix-legend-fieldset branch from 20356b8 to ecb612d Compare May 13, 2024 12:25
@mirawlin
Copy link
Contributor

J'ai tester avec VoiceOver pour voir le comportement de legend
resultat:
Le legend est lu d'abord eg. Sélectionnez une seule réponse. group
Apres le direction est lu: Sélectionnez une seule réponse.
et enfin l'instruction est lu: Pix evalue 16 competences ...

@mirawlin mirawlin force-pushed the PIX-12382-fix-legend-fieldset branch from ecb612d to 31e380c Compare May 14, 2024 14:59
@yannbertrand
Copy link
Member Author

J'ai tester avec VoiceOver pour voir le comportement de legend resultat: Le legend est lu d'abord eg. Sélectionnez une seule réponse. group Apres le direction est lu: Sélectionnez une seule réponse. et enfin l'instruction est lu: Pix evalue 16 competences ...

Il y a doublon sur "Sélectionner une seule réponse" alors ? Le second est sensé ne pas être lu avec aria-hidden="true"

@mirawlin
Copy link
Contributor

J'ai tester avec VoiceOver pour voir le comportement de legend resultat: Le legend est lu d'abord eg. Sélectionnez une seule réponse. group Apres le direction est lu: Sélectionnez une seule réponse. et enfin l'instruction est lu: Pix evalue 16 competences ...

Il y a doublon sur "Sélectionner une seule réponse" alors ? Le second est sensé ne pas être lu avec aria-hidden="true"

ahh oui - j'ai pas vu le aria-hidden="true"
oui donc il y a un doublon qui est pas sensé etre la

@mirawlin
Copy link
Contributor

mirawlin commented May 14, 2024

J'ai tester avec VoiceOver pour voir le comportement de legend resultat: Le legend est lu d'abord eg. Sélectionnez une seule réponse. group Apres le direction est lu: Sélectionnez une seule réponse. et enfin l'instruction est lu: Pix evalue 16 competences ...

Il y a doublon sur "Sélectionner une seule réponse" alors ? Le second est sensé ne pas être lu avec aria-hidden="true"

Ah tiens - j'ai trouvé ca sur le premier resultat de google quand j'ai fait une recherche sur ce probleme

Il semblerait que : If you use aria-describedby in the same element as aria-hidden="true" the screen reader may read it

edit: not related to the problem

@yannbertrand
Copy link
Member Author

yannbertrand commented May 14, 2024

J'ai tester avec VoiceOver pour voir le comportement de legend resultat: Le legend est lu d'abord eg. Sélectionnez une seule réponse. group Apres le direction est lu: Sélectionnez une seule réponse. et enfin l'instruction est lu: Pix evalue 16 competences ...

Il y a doublon sur "Sélectionner une seule réponse" alors ? Le second est sensé ne pas être lu avec aria-hidden="true"

Ah tiens - j'ai trouvé ca sur le premier resultat de google quand j'ai fait une recherche sur ce probleme

Il semblerait que : If you use aria-describedby in the same element as aria-hidden="true" the screen reader may read it

aria-describedby est sur le form (et référence autre chose qui n'a rien à voir) et aria-hidden="true" est dans une autre balise, je suis pas sûr de comprendre 🤔

Sur Wave j'ai l'impression qu'on ne détecte pas :
image

@mirawlin
Copy link
Contributor

J'ai tester avec VoiceOver pour voir le comportement de legend resultat: Le legend est lu d'abord eg. Sélectionnez une seule réponse. group Apres le direction est lu: Sélectionnez une seule réponse. et enfin l'instruction est lu: Pix evalue 16 competences ...

Il y a doublon sur "Sélectionner une seule réponse" alors ? Le second est sensé ne pas être lu avec aria-hidden="true"

Ah tiens - j'ai trouvé ca sur le premier resultat de google quand j'ai fait une recherche sur ce probleme
Il semblerait que : If you use aria-describedby in the same element as aria-hidden="true" the screen reader may read it

aria-describedby est sur le form (et référence autre chose qui n'a rien à voir) et aria-hidden="true" est dans une autre balise, je suis pas sûr de comprendre 🤔

Sur Wave j'ai l'impression qu'on ne détecte pas : image

Oui - tu as raison - il n y a rien a voir (on peut ignorer le comment :P)

J'ai retester ce matin et j'ai le meme resultat avec Wave qui dit que c'est bien aria-hidden=true mais c'est toujours lu 2 fois. Et avec firefox c'est pareil

@yannbertrand
Copy link
Member Author

J'ai tester avec VoiceOver pour voir le comportement de legend resultat: Le legend est lu d'abord eg. Sélectionnez une seule réponse. group Apres le direction est lu: Sélectionnez une seule réponse. et enfin l'instruction est lu: Pix evalue 16 competences ...

Il y a doublon sur "Sélectionner une seule réponse" alors ? Le second est sensé ne pas être lu avec aria-hidden="true"

Ah tiens - j'ai trouvé ca sur le premier resultat de google quand j'ai fait une recherche sur ce probleme
Il semblerait que : If you use aria-describedby in the same element as aria-hidden="true" the screen reader may read it

aria-describedby est sur le form (et référence autre chose qui n'a rien à voir) et aria-hidden="true" est dans une autre balise, je suis pas sûr de comprendre 🤔
Sur Wave j'ai l'impression qu'on ne détecte pas : image

Oui - tu as raison - il n y a rien a voir (on peut ignorer le comment :P)

J'ai retester ce matin et j'ai le meme resultat avec Wave qui dit que c'est bien aria-hidden=true mais c'est toujours lu 2 fois. Et avec firefox c'est pareil

Revue ensemble, ce n'est pas la div aria-hidden="true" qui re déclenche la lecture de "Sélectionner une seule réponse".

En fait le fieldset est un groupe libellé par cette legend donc c'est aussi normal que "Sélectionner une seule réponse" soit lu 2 fois (donc une fois où c'est précisé "group").

Par contre on est pas certain de pourquoi aria-describedby sur le form n'est pas lu avec VoiceOver...

@yannbertrand yannbertrand force-pushed the PIX-12382-fix-legend-fieldset branch from 31e380c to f6ee965 Compare May 22, 2024 13:04
@pix-service-auto-merge pix-service-auto-merge merged commit 8e30f20 into dev May 22, 2024
6 of 8 checks passed
@pix-service-auto-merge pix-service-auto-merge deleted the PIX-12382-fix-legend-fieldset branch May 22, 2024 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants