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

Améliorations d'accessibilité sur le formulaire d'inscription #769

Closed
wants to merge 1 commit into from
Closed

Améliorations d'accessibilité sur le formulaire d'inscription #769

wants to merge 1 commit into from

Conversation

arnaudriegert
Copy link
Contributor

Résout une partie de #755

Résumé

  • Ajout de labels aux champs du formulaire
  • Le texte sur la politique de données est intégré dans la balise form

Détails

Labels ajoutés et placeholders modifiés. Cela rend le formulaire légèrement plus long verticalement mais plus accessible.

L'intégration du texte en bas du formulaire dans la balise form est sans impact visuel.

Avant

image

Après

image

Question

On peut éventuellement raccourcir un peu en enlevant la phrase "Votre date de naissance sera vérifiée lors du rendez-vous." et modifier le label pour mettre "Date de naissance (sera vérifiée lors du rendez-vous)"

@arnaudriegert
Copy link
Contributor Author

Hello @mathieuripert @ssaunier @carsso @rchampourlier je voudrais essayer de contribuer un peu occasionnellement (aucune expérience en Rails mais c'est l'occasion d'apprendre).

Je crois qu'il faut que l'un de vous me whiteliste pour faire tourner les workflows et que je puisse ajouter des reviewers :)

Pour info je suis un collègue de @mininao 🙂

Copy link
Collaborator

@EmCousin EmCousin left a comment

Choose a reason for hiding this comment

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

Top, juste une petite suggestion en termes d'accessibilité 👍

Comment on lines +6 to +8
<% @user.errors.full_messages.each do |msg| %>
<%= msg %><br/>
<% end %>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pour améliorer l'accessibilité on va également éviter les <br /> 🙂

Suggested change
<% @user.errors.full_messages.each do |msg| %>
<%= msg %><br/>
<% end %>
<ul>
<% @user.errors.full_messages.each do |msg| %>
<li><%= msg %></li>
<% end %>
</ul>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Merci @EmCousin ! C'est mieux effectivement, je te propose de garder ça pour une PR séparée :

  • pour le faire sur toutes les pages qui contiennent des alertes de ce type
  • et éventuellement affiner un peu pour éviter de faire une ul s'il n'y a qu'un seul message à afficher

En l'état je n'ai fait que reprendre l'existant (ça n'apparait dans le diff qu'à cause des indentations qui ont changé)

@arnaudriegert
Copy link
Contributor Author

Je ferme cette PR pour la recréer depuis le dépôt central (je vais fermer mon fork)

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

Successfully merging this pull request may close these issues.

2 participants