-
Notifications
You must be signed in to change notification settings - Fork 40
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
(PC-33528)[PRO] feat : add new Multiselect component #15628
base: master
Are you sure you want to change the base?
Conversation
e4f34d2
to
a6935c0
Compare
96161cf
to
d83f8ac
Compare
a38d613
to
51e3823
Compare
bf5e2e9
to
4c43813
Compare
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.
Ca rend super bien 🎉
J'ai quelques petits retours d'utilisation et d'a11y, mais l'affichage visuel / fonctionnement souris m'ont l'air nickel :
- Quand on est focus sur le bouton, je m'attends à ce que le panneau s'ouvre automatiquement quand je presse "Enter" ou "Space"
- Il faut que le panneau se ferme quand le focus sort du panneau (sinon il risque de cacher du contenu inutilement)
- Quand on est focus sur l'input de la checkbox c'est la touche "Space" qui doit toggle la sélection et pas la touche "Enter" (la doc du pattern est ici), ça fonctionne bien pour le "Tout sélectionner", mais pas pour les checkbox individuelles
- Il faudrait un attribut
aria-controls
sur le bouton d'ouverture avec pour valeur l'id de la div du panneau (idéalement il faudrait que l'attribut controls apparaisse quand la div du paneau apparait pour ne pas avoir une référence à un id inexistant) - Au moment de faire une recherche dans l'input de recherche, il faudrait un moyen d'annoncer (et d'afficher p-être ?) le nombre de résultats (souvent via un
role="status"
sur un texte genre "n résultats")
Ce serait cool d'avoir l'avis de Julie au global sur le composant (en particulier au lecteur d'écran)!
4c43813
to
e84a161
Compare
Visit the preview URL for this PR (updated for commit e998578): https://pc-pro-testing--pr15628-pc-33528-new-multi-s-wlndeqgo.web.app (expires Thu, 16 Jan 2025 10:35:44 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 032d233ee67e1c50d6af12e29c936c7076770eb1 |
e1b3605
to
c412b65
Compare
c412b65
to
c80fc3d
Compare
c80fc3d
to
e998578
Compare
But de la pull request
Ticket Jira (ou description si BSR) : https://passculture.atlassian.net/browse/PC-33528
Vérifications