fix(md-chips): change md-icon class to inherit size #10491
Conversation
googlebot
added
the
cla: yes
batsauto
changed the title
batsauto
changed the title
|
Looks good, but can you please update the commit message & PR description to follow our guidelines? https://github.com/angular/material/blob/master/.github/CONTRIBUTING.md#submitting-pull-requests Thanks! |
batsauto
changed the title
batsauto
changed the title
batsauto
changed the title
This will allow the chip remove icon to inherit the proper size from the css class
change your code in src/components/chips/chips.scss from
md-icon {
height: $chip-delete-icon-size;
width: $chip-delete-icon-size;
position: absolute;
top: 50%;
left: 50%;
transform: translate3d(-50%, -50%, 0);
}
to this
md-icon {
height: $chip-delete-icon-size;
width: $chip-delete-icon-size;
min-height: $chip-delete-icon-size;
min-width: $chip-delete-icon-size;
position: absolute;
top: 50%;
left: 50%;
transform: translate3d(-50%, -50%, 0);
}
fixes issue #9619
|
I updated the commit message and PR description |
|
@batsauto Based on what I'm seeing in your changes, this should not be classified as a breaking commit because the user shouldn't need to change anything. Also, your description in the PR still says that you need to change Am I not understanding something? Thanks! |
|
I accidently placed the commit message info into the PR description. It has been fixed. Thanks for your understanding. |
|
LGTM 👍 |
Splaktar
added
type: bug
P1: urgent
|
This would normally have to go into a minor release and not a patch release since there are CSS changes. But I need to review how we should handle this for the case of patch release regressions. By the time a minor release comes out, we'll hopefully be aligning with the Material Design spec as called out in #9883. |
|
This breaking CSS change was part of the 1.1.0 minor release as the CSS is changed from what was in 1.0.9. As this was a minor release, breaking CSS changes like this were acceptable. Additionally, we'll need to wait for another minor release (1.2.0) before this can be fixed since it is a breaking visual change via CSS. |
|
Feedback for presubmit failures: This PR "has mostly good looking screenshot diffs, except one app where the chip remove button isn't vertically centered." It's about 4px lower than it should be. |
Splaktar
added
in progress
Splaktar
added
pr: lgtm
fix(chips): md-chips remove icon not inheriting class size
This fix will allow the chip remove icon to inherit the proper size from the css class.
This addresses issue #9619