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

mat-dialog-close wrongly adds an aria-label when the button text content is bound rather than static #15542

Closed
stevenyxu opened this issue Mar 19, 2019 · 2 comments · Fixed by #15654
Closed

mat-dialog-close wrongly adds an aria-label when the button text content is bound rather than static #15542

stevenyxu opened this issue Mar 19, 2019 · 2 comments · Fixed by #15654
Assignees
@crisbeto
Labels
Accessibility This issue is related to accessibility (a11y) G This is is related to a Google internal issue P2 The issue is important to a large percentage of users, with a workaround

Comments

@stevenyxu
Copy link
Contributor

What is the expected behavior?

When I have <button mat-dialog-close>Submit</button> in a template, I expect for the resulting HTML to be basically the same, and mat-dialog-close should not add an aria label to the button.

What is the current behavior?

<button mat-dialog-close aria-label="Close dialog">Submit</button>

What are the steps to reproduce?

https://stackblitz.com/edit/angular-j9paiz?file=app/dialog-overview-example-dialog.html

Which versions of Angular, Material, OS, TypeScript, browsers are affected?

Head.

Is there anything else we should know?

This area was partly addressed in #11093 but has this remaining bug. A workaround is available (just set aria-label explicitly in addition to the inner content), but this is not ideal.

stevenyxu@6645309 demonstrates the bug in the form of a failing unit test. I tried to do a fix but couldn't quickly come up with a performant solution without more work.
@jelbourn jelbourn added P2 The issue is important to a large percentage of users, with a workaround Accessibility This issue is related to accessibility (a11y) G This is is related to a Google internal issue labels Mar 23, 2019
@jelbourn
Copy link
Member

@crisbeto we should probably just use a mutation observer here

crisbeto added a commit to crisbeto/material2 that referenced this issue Mar 23, 2019
@crisbeto
fix(dialog): aria-label not being removed if text content changes aft…
6337dd4 
…er init

Currently we set an `aria-label` on the `mat-dialog-close` button if it's an icon button or it doesn't have text, however it wasn't accounting for the case where the text comes in at a later point.

Fixes angular#15542.
crisbeto added a commit to crisbeto/material2 that referenced this issue Mar 23, 2019
@crisbeto
fix(dialog): aria-label not being removed if text content changes aft…
90df4c6 
…er init

Currently we set an `aria-label` on the `mat-dialog-close` button if it's an icon button or it doesn't have text, however it wasn't accounting for the case where the text comes in at a later point.

Fixes angular#15542.
@crisbeto crisbeto mentioned this issue Mar 23, 2019
Closed
crisbeto added a commit to crisbeto/material2 that referenced this issue Mar 23, 2019
@crisbeto
fix(dialog): aria-label not being removed if text content changes aft…
de963dd 
…er init

Currently we set an `aria-label` on the `mat-dialog-close` button if it's an icon button or it doesn't have text, however it wasn't accounting for the case where the text comes in at a later point.

Fixes angular#15542.
@stevenyxu stevenyxu mentioned this issue Mar 27, 2019
Closed
crisbeto added a commit to crisbeto/material2 that referenced this issue Mar 29, 2019
@crisbeto
fix(dialog): remove default aria-label from mat-dialog-close
4e84016 
Currently we add an `aria-label` by default to close buttons that don't have text. This is unreliable, because text can come in at any time and it takes a lot of code to keep track of it. Furthermore, the default text is in English which means that sites in different languages will have to override it anyway. These changes remove the default `aria-label` and add some guidance to the docs on how to deal with labelling the close button.

Fixes angular#15542.
@crisbeto crisbeto mentioned this issue Mar 29, 2019
Merged
mmalerba pushed a commit that referenced this issue Apr 3, 2019
@crisbeto @mmalerba
fix(dialog): remove default aria-label from mat-dialog-close (#15654)
c3ead65 
Currently we add an `aria-label` by default to close buttons that don't have text. This is unreliable, because text can come in at any time and it takes a lot of code to keep track of it. Furthermore, the default text is in English which means that sites in different languages will have to override it anyway. These changes remove the default `aria-label` and add some guidance to the docs on how to deal with labelling the close button.

Fixes #15542.
jelbourn pushed a commit that referenced this issue Apr 4, 2019
@crisbeto @jelbourn
fix(dialog): remove default aria-label from mat-dialog-close (#15654)
307889e 
Currently we add an `aria-label` by default to close buttons that don't have text. This is unreliable, because text can come in at any time and it takes a lot of code to keep track of it. Furthermore, the default text is in English which means that sites in different languages will have to override it anyway. These changes remove the default `aria-label` and add some guidance to the docs on how to deal with labelling the close button.

Fixes #15542.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@crisbeto crisbeto

Labels
Accessibility This issue is related to accessibility (a11y) G This is is related to a Google internal issue P2 The issue is important to a large percentage of users, with a workaround
Projects
None yet
Milestone
No milestone
3 participants
@stevenyxu @jelbourn @crisbeto