-
Notifications
You must be signed in to change notification settings - Fork 92
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
Corrige les défaut d'accessibilité de l'étape 1 d'une démarche #10879
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10879 +/- ##
==========================================
+ Coverage 84.69% 84.70% +0.01%
==========================================
Files 1130 1130
Lines 25064 25064
Branches 4701 4701
==========================================
+ Hits 21228 21231 +3
+ Misses 3836 3833 -3 ☔ View full report in Codecov by Sentry. |
@@ -11,6 +11,7 @@ | |||
%fieldset#radio-rich-hint.fr-fieldset | |||
%legend.fr-fieldset__legend--regular.fr-fieldset__legend | |||
= t('views.users.dossiers.identite.legend') | |||
= render EditableChamp::AsteriskMandatoryComponent.new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dans la mesure ou tous les champs de cette page sont obligatoires, est ce qu'on allègerait pas la page en retirant la mention certains champs avec des * sont obligatoires ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ça c'est une question d'UX à voir avec @marleneklok. :)
D'un point de vue perso, je trouverai ça beaucoup plus lisible et moins lourd d'indiquer que tous les champs sont obligatoires.
Par contre, cela impliquerait de supprimer les astérisques derrière chaque champ. Car si l’astérisque est présente, on ne peut pas ne pas expliciter sa signification (pour les personnes présentant des troubles cognitifs, par exemple).
Donc s'il est techniquement possible de conditionner l'affichage de l'astérisque, je pense que ce serait clairement la meilleure option.
Reste à savoir si c'est possible... ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je serais d'avis de conserver le code standard "astérisque" pour matérialiser les champs obligatoires.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comme quoi, ça vaut le coup de demander son avis à une personne dont c'est le métier. ^^
@@ -48,9 +48,15 @@ | |||
.fr-fieldset__element.fr-mb-0 | |||
.fr-fieldset.width-100 | |||
.fr-fieldset__element.fr-fieldset__element--short-text | |||
= render Dsfr::InputComponent.new(form: individual, attribute: :prenom, opts: { autocomplete: (for_tiers? ? false : 'given-name') }) | |||
- if for_tiers? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
je ne comprends pas ce change, ce n'est pas ce autocomplete: (for_tiers? ? false : 'given-name') })
est sensé faire ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
La valeur false
n'existe pas pour l'attribut autocomplete
;)
Et la valeur off
n'est pas forcément respectée par tous les navigateurs (notamment sur les formulaires de connexion).
@@ -6,7 +6,7 @@ | |||
- is_administrateur_context = nav_bar_profile == :administrateur && administrateur_signed_in? | |||
- is_expert_context = nav_bar_profile == :expert && expert_signed_in? | |||
- is_user_context = nav_bar_profile == :user | |||
- is_search_enabled = [params[:controller] == 'recherche', is_instructeur_context, is_expert_context, is_user_context && current_user.dossiers.count].any? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
j'ai l'impression qu'avant on permettait aux usagers d'avoir un moteur de recherche dès qu'il avait un dossier (bien que la syntaxe est ici illisible, je te l'accorde).
tu ne veux plus permettre aux usagers de rechercher ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sauf erreur de ma part, le is_search_enabled
n'est utilisé qu'à la ligne 23 pour conditionner l'affichage du bouton permettant d'afficher la modale.
Sachant que le formulaire de recherche est affiché dans le cœur de page sur la page de listing et que la fonctionnalité n'est pas disponible sur les autres pages pour les usagers, nous sommes arrivés à la conclusion que la présence du bouton constituait une erreur.
Je me pingue @marleneklok , @lisa-durand et @mfo pour confirmer. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Met à jour la navigation principale
aria-controls
inappropriéstitle
Après

Avant

Sépare le formulaire de la navigation, lorsqu'il y a un seul champ
Après

Avant

Évite la vocalisation d'une image de décoration par Voice Over
Après
bug.VO.-.apres.mp4
Avant
bug.VO.-.avant.mp4
Met à jour l'indication des champs obligatoires
Après

Avant

Précise l'étape courante de la démarche dans le titre page
Après

Avant

Masque le bouton de soumission intermédiaire aux technologies d'assistance
Après

Avant

Supprime les attributs
aria-labelledby
inutilesAprès

Avant

Supprime les regroupements inutiles
Après

Avant

Améliore la césure dans le menu d'aide
Après

Avant

Supprime les attributs
autocomplete="false"
erronésAprès

Avant

Supprime le bouton de recherche ouvrant la modale en mobile en vue utilisateur
Après

Avant
