Skip to content

UX/UI: Remplacer role=button par des <button>#4912

Merged
hellodeloo merged 1 commit intomasterfrom
deloo/update-role=button-to-button
Nov 7, 2024
Merged

UX/UI: Remplacer role=button par des <button>#4912
hellodeloo merged 1 commit intomasterfrom
deloo/update-role=button-to-button

Conversation

@hellodeloo
Copy link
Copy Markdown
Contributor

@hellodeloo hellodeloo commented Oct 10, 2024

🤔 Pourquoi ?

Amélioration niveau accessibilité et vérification du markup

🍰 Comment ?

Décrivez en quelques mots la solution retenue et mise en oeuvre, les difficultés ou problèmes rencontrés. Attirez l'attention sur les décisions d'architecture ou de conception importantes.

@notion-workspace
Copy link
Copy Markdown

🧹 A11Y : Verifier et améliorer les ou role=”button

@hellodeloo hellodeloo added the modifié Modifié dans le changelog. label Oct 10, 2024
@hellodeloo hellodeloo changed the title WIP UX/UI: Remplacer role=button par des <button> Oct 10, 2024
@hellodeloo hellodeloo self-assigned this Oct 10, 2024
@hellodeloo hellodeloo force-pushed the deloo/update-role=button-to-button branch from b12c5ff to 94acdc5 Compare October 17, 2024 13:30
@gip-inclusion gip-inclusion deleted a comment from francoisfreitag Oct 18, 2024
@hellodeloo hellodeloo force-pushed the deloo/update-role=button-to-button branch from c60a0ec to bf04486 Compare October 18, 2024 14:55
Copy link
Copy Markdown
Contributor

@leo-naeka leo-naeka left a comment

Choose a reason for hiding this comment

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

LGTM.

Est-ce que ça ne vaudrait pas le coup de réfléchir à un linter HTML pour fixer mais surtout maintenir ce genre de choses ? Déjà discuté ?

@francoisfreitag
Copy link
Copy Markdown
Member

Le plan est de sortir un template tag maison pour faire qu’un élément (pour le moment, <input type="checkbox">) soit le control d’un autre collapse. Une <div role="button"> n’est pas une bonne idée. Par exemple, lorsque la div contient un input, il est possible de cliquer dans la div sans changer l’état de l’input associé, ce qui déclenche le collapse et amène à une UI incohérente.

Exemple récent : https://itou-inclusion.slack.com/archives/C0412CTV63D/p1729243852173949

Ça peut être bien pire, je peux trouver d’autres exemples au besoin.

Cette PR est sur ma pile à retravailler, je pensais le faire dans la semaine. Une fois retravaillée, on n’aura plus besoin des tabindex. Et normalement, on pourra copier le markup des autres composants 🙈

@francoisfreitag francoisfreitag force-pushed the deloo/update-role=button-to-button branch from 96f556c to 7c40949 Compare October 31, 2024 15:57
@francoisfreitag
Copy link
Copy Markdown
Member

J’ai poussé 7c40949, je te laisse me dire s’il te convient et mettre à jour les autres champs ?

@hellodeloo hellodeloo force-pushed the deloo/update-role=button-to-button branch from 3a01c57 to a0f396b Compare October 31, 2024 17:55
@hellodeloo
Copy link
Copy Markdown
Contributor Author

J’ai poussé 7c40949, je te laisse me dire s’il te convient et mettre à jour les autres champs ?

Merci c'est good. De mon coté, j'ai pushé la suite. Si c'est ok pour toi, je rebaserais les commits

@francoisfreitag francoisfreitag force-pushed the deloo/update-role=button-to-button branch 2 times, most recently from ea7bacd to 35e7def Compare November 4, 2024 10:07
Comment thread itou/templates/includes/demo_accounts.html Outdated
Copy link
Copy Markdown
Member

@francoisfreitag francoisfreitag left a comment

Choose a reason for hiding this comment

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

Autrement ça m’a l’air bien 👍

@hellodeloo hellodeloo force-pushed the deloo/update-role=button-to-button branch from 35e7def to 0e1375b Compare November 7, 2024 09:19
@hellodeloo hellodeloo added this pull request to the merge queue Nov 7, 2024
Merged via the queue into master with commit 175786a Nov 7, 2024
@hellodeloo hellodeloo deleted the deloo/update-role=button-to-button branch November 7, 2024 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

modifié Modifié dans le changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants