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

bug(chips): matChipRemove should not apply to icons #23463

Closed
jelbourn opened this issue Aug 26, 2021 · 3 comments · Fixed by #23538
Closed

bug(chips): matChipRemove should not apply to icons #23463

jelbourn opened this issue Aug 26, 2021 · 3 comments · Fixed by #23538
Assignees
@crisbeto
Labels
Accessibility This issue is related to accessibility (a11y) area: material/chips docs This issue is related to documentation 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

@jelbourn
Copy link
Member

All of our examples that use matChipRemove apply directly to mat-icon, which is not great for a11y; instead, the directive should go on a button that contains an icon.

Alternatively, we can project <mat-icon matChipRemove> into a button within the component. This fix might be preferable since it will "fix" anyone who has copied this example so far.
@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) docs This issue is related to documentation G This is is related to a Google internal issue area: material/chips labels Aug 26, 2021
@crisbeto
Copy link
Member

I think that we'll have to make some changes before we recommend <button matChipRemove>, because the user agent styles will interfere with the icon.

@jelbourn
Copy link
Member Author

jelbourn commented Sep 3, 2021

@crisbeto yeah, definitely. We could try making button[matChipRemove] a component with the right styles, but I don't have a sense off the top of my head how breaking that would be.

@crisbeto crisbeto self-assigned this Sep 6, 2021
@crisbeto crisbeto added the has pr label Sep 6, 2021
crisbeto added a commit to crisbeto/material2 that referenced this issue Sep 6, 2021
@crisbeto
fix(material/chips): handle matChipRemove set on a button
2595e7d 
Fixes that `matChipRemove` looked off when it's set on a `button` element. Also updates all the examples to use buttons.

Note that we don't need any extra logic, because the `MatChipRemove` directive already sets `type="button"` when it's applied to a `button` element.

Fixes angular#23463.
@crisbeto crisbeto mentioned this issue Sep 6, 2021
Merged
andrewseguin pushed a commit that referenced this issue Sep 10, 2021
@crisbeto
fix(material/chips): handle matChipRemove set on a button (#23538)
bb9c36b 
Fixes that `matChipRemove` looked off when it's set on a `button` element. Also updates all the examples to use buttons.

Note that we don't need any extra logic, because the `MatChipRemove` directive already sets `type="button"` when it's applied to a `button` element.

Fixes #23463.
andrewseguin pushed a commit that referenced this issue Sep 10, 2021
@crisbeto @andrewseguin
fix(material/chips): handle matChipRemove set on a button (#23538)
b12018a 
Fixes that `matChipRemove` looked off when it's set on a `button` element. Also updates all the examples to use buttons.

Note that we don't need any extra logic, because the `MatChipRemove` directive already sets `type="button"` when it's applied to a `button` element.

Fixes #23463.

(cherry picked from commit bb9c36b)
@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 Oct 11, 2021
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) area: material/chips docs This issue is related to documentation 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
Development

Successfully merging a pull request may close this issue.

fix(material/chips): handle matChipRemove set on a button crisbeto/material2
2 participants
@jelbourn @crisbeto