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.

Sign up for GitHub

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

Jump to bottom

add aria accessibility #56

Open
wants to merge 1 commit into
base: feature/button-summary
Choose a base branch
from
Open

add aria accessibility #56

wants to merge 1 commit into from

Conversation

amine-annanouch-ext
Copy link

No description provided.

@amine-annanouch-ext amine-annanouch-ext requested a review from a team as a code owner March 22, 2022 19:32
@amine-annanouch-ext amine-annanouch-ext force-pushed the feature/button-summary-accessibility branch 3 times, most recently from c124928 to da3483f Compare March 22, 2022 20:34
gregpetit
gregpetit suggested changes Mar 23, 2022
View reviewed changes
Copy link
Contributor

@gregpetit gregpetit left a comment

Choose a reason for hiding this comment

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

Il ya des problèmes d'espacements sous le libellé et sous le texte d'assistance.

Le libellé devrait avoir un margin-bottom de 0.5rem, mais il semble que ce margin-bottom soit appliqué deux fois. Une fois à la balise , et une fois sur la balise

. Le margin-bottom de 0.5rem devrait seulement être appliqué à une seule de ces balises.
Capture d’écran, le 2022-03-23 à 09 23 40

Le texte d'assistance à présentement un margin-top de 0.5rem, alors que cela devrait plus tôt être un margin-bottom de 0.5rem.
Capture d’écran, le 2022-03-23 à 09 53 10

@amine-annanouch-ext amine-annanouch-ext force-pushed the feature/button-summary-accessibility branch from da3483f to 4b40186 Compare March 23, 2022 15:05
OlivierAlbertini
OlivierAlbertini reviewed Mar 23, 2022
View reviewed changes
Copy link
Member

@OlivierAlbertini OlivierAlbertini left a comment

Choose a reason for hiding this comment

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

serait il possible d'ajouter des tests pour s'assurer du comportement des aria ?
Exemple de tests : https://github.com/VilledeMontreal/angular-ui/blob/master/projects/angular-ui/src/lib/button/button.component.spec.ts
ou https://github.com/VilledeMontreal/angular-ui/blob/master/projects/angular-ui/src/lib/breadcrumb/breadcrumb.component.spec.ts

Merci

OlivierAlbertini
OlivierAlbertini reviewed Mar 23, 2022
View reviewed changes
Copy link
Member

@OlivierAlbertini OlivierAlbertini left a comment

Choose a reason for hiding this comment

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

est-ce qu'il y a un ticket relié à ces changements ?

@OlivierAlbertini OlivierAlbertini added the enhancement New feature or request label Mar 24, 2022
@OlivierAlbertini
Copy link
Member

S'il n'y a plus d'activité, je vais fermer cette PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OlivierAlbertini OlivierAlbertini OlivierAlbertini left review comments

@gregpetit gregpetit gregpetit requested changes

@uelanom uelanom uelanom approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@amine-annanouch-ext @OlivierAlbertini @uelanom @gregpetit