fix(textfield): add separate classes for leading/trailing icons #5367

asyncLiz · 2019-12-19T23:57:35Z

BREAKING_CHANGE: icons must use .mdc-text-field__leading-icon or .mdc-text-field__trailing-icon classes. mdc-text-field-icon-color() mixin has been split into mdc-text-field-leading-icon-color() and mdc-text-field-trailing-icon-color().

mdc-web-bot · 2019-12-20T00:12:19Z

All 728 screenshot tests passed for commit 2167ee0 vs. master! 💯🎉

codecov-io · 2019-12-20T00:17:52Z

Codecov Report

Merging #5367 into master will increase coverage by 1.43%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #5367      +/-   ##
==========================================
+ Coverage   97.22%   98.66%   +1.43%     
==========================================
  Files         164      163       -1     
  Lines        6267     6281      +14     
  Branches      834      850      +16     
==========================================
+ Hits         6093     6197     +104     
+ Misses        174       84      -90

Impacted Files	Coverage Δ
packages/mdc-textfield/constants.ts	`100% <ø> (ø)`	⬆️
packages/mdc-textfield/component.ts	`99% <100%> (+0.99%)`	⬆️
testing/helpers/foundation.ts	`87.23% <0%> (-1.23%)`	⬇️
packages/mdc-menu/foundation.ts	`95.55% <0%> (-1.04%)`	⬇️
packages/mdc-menu-surface/foundation.ts	`99.12% <0%> (-0.88%)`	⬇️
packages/mdc-list/foundation.ts	`98.82% <0%> (-0.78%)`	⬇️
...ages/mdc-textfield/character-counter/foundation.ts	`100% <0%> (ø)`	⬆️
packages/mdc-switch/constants.ts	`100% <0%> (ø)`	⬆️
packages/mdc-textfield/foundation.ts	`100% <0%> (ø)`	⬆️
packages/mdc-switch/foundation.ts	`100% <0%> (ø)`	⬆️
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 784fa79...e761a87. Read the comment docs.

packages/mdc-textfield/component.ts

patrickrodee

Overall comment: let's leave the mdc-text-field__icon element class on the icons but add the mdc-text-field__icon--trailing or mdc-text-field__icon--leading modifier classes. It'll allow us to keep generalized behavior but also apply variant-specific behavior where necessary.

packages/mdc-textfield/icon/_mixins.scss

asyncLiz · 2019-12-20T17:06:24Z

Overall comment: let's leave the mdc-text-field__icon element class on the icons but add the mdc-text-field__icon--trailing or mdc-text-field__icon--leading modifier classes. It'll allow us to keep generalized behavior but also apply variant-specific behavior where necessary.

@patrickrodee does that make sense for the long term though? A lot of the shared styles right now have to deal with absolute positioning, which will go away. In the end the only shared styles will be for the cursor and scaling with density.

It's still a breaking change since we need the leading/trailing modifiers, and it feels like an unnecessary class to me at that point.

patrickrodee · 2019-12-20T17:09:35Z

@asyncLiz Yeah, let's keep the existing mdc-text-field__icon class and add the mdc-text-field__icon--leading/trailing classes. Having an element class on both is good future proofing and better follows BEM.

asyncLiz · 2019-12-20T17:13:28Z

Would we want to do something similar for prefix/suffix?

<span class="mdc-text-field__affix mdc-text-field__affix-prefix">$</span>

patrickrodee · 2019-12-20T17:36:13Z

@asyncLiz Yeah, something similar with prefix/suffix would be a good idea.

mdc-web-bot · 2019-12-23T18:31:56Z

All 728 screenshot tests passed for commit dc81574 vs. master! 💯🎉

mdc-web-bot · 2019-12-23T18:37:14Z

All 728 screenshot tests passed for commit 3bf0ec9 vs. master! 💯🎉

packages/mdc-textfield/icon/_mixins.scss

packages/mdc-textfield/_mixins.scss

packages/mdc-textfield/icon/mdc-text-field-icon.scss

mdc-web-bot · 2020-01-06T15:55:01Z

All 698 screenshot tests passed for commit 41d5d91 vs. master! 💯🎉

packages/mdc-textfield/icon/_mixins.scss

mdc-web-bot · 2020-01-06T20:34:48Z

All 704 screenshot tests passed for commit 5dbfcc3 vs. master! 💯🎉

patrickrodee

🔥

abhiomkar · 2020-01-07T16:38:56Z

@asyncLiz nit: BREAKING_CHANGE => BREAKING CHANGE when your squash & merge in commit description to show up on CHANGELOG.

BREAKING_CHANGE: icons must use `.mdc-text-field__leading-icon` or `.mdc-text-field__trailing-icon` classes. `mdc-text-field-icon-color()` mixin has been split into `mdc-text-field-leading-icon-color()` and `mdc-text-field-trailing-icon-color()`.

mdc-web-bot · 2020-01-08T19:55:19Z

All 602 screenshot tests passed for commit 1988a4a vs. master! 💯🎉

…r leading/trailing icons PR: go/mdc-web-pull/5367 Description: BREAKING_CHANGE: icons must use `.mdc-text-field__icon--leading` or `.mdc-text-field__icon--trailing` classes. `mdc-text-field-icon-color()` mixin has been split into `mdc-text-field-leading-icon-color()` and `mdc-text-field-trailing-icon-color()`. Tested: https://test.corp.google.com/ui#id=OCL:287554248:BASE:288701267:1578500687772:eaf3ab49 COPYBARA_INTEGRATE_REVIEW=material-components/material-components-web#5367 from material-components:fix/textfield/leading-trailing-icon-classes 3bf0ec97cba823052241450b65ae1edb3af18963 PiperOrigin-RevId: 288741151

mdc-web-bot · 2020-01-08T20:12:46Z

All 602 screenshot tests passed for commit a3c5534 vs. master! 💯🎉

mdc-web-bot · 2020-01-08T20:15:07Z

All 602 screenshot tests passed for commit e761a87 vs. master! 💯🎉

asyncLiz requested a review from patrickrodee December 19, 2019 23:57

asyncLiz self-assigned this Dec 19, 2019

googlebot added the cla: yes label Dec 19, 2019

patrickrodee reviewed Dec 20, 2019

View reviewed changes

packages/mdc-textfield/component.ts Show resolved Hide resolved

patrickrodee requested changes Dec 20, 2019

View reviewed changes

packages/mdc-textfield/icon/_mixins.scss Outdated Show resolved Hide resolved

asyncLiz requested a review from patrickrodee December 23, 2019 19:52

asyncLiz added the Ready for Google label Dec 30, 2019

patrickrodee requested changes Jan 2, 2020

View reviewed changes

packages/mdc-textfield/icon/_mixins.scss Outdated Show resolved Hide resolved

packages/mdc-textfield/_mixins.scss Outdated Show resolved Hide resolved

packages/mdc-textfield/_mixins.scss Show resolved Hide resolved

packages/mdc-textfield/icon/mdc-text-field-icon.scss Show resolved Hide resolved

asyncLiz requested a review from patrickrodee January 6, 2020 15:31

asyncLiz force-pushed the fix/textfield/leading-trailing-icon-classes branch from 119febc to 41d5d91 Compare January 6, 2020 15:40

patrickrodee reviewed Jan 6, 2020

View reviewed changes

packages/mdc-textfield/icon/_mixins.scss Outdated Show resolved Hide resolved

asyncLiz requested a review from patrickrodee January 6, 2020 20:08

asyncLiz force-pushed the fix/textfield/leading-trailing-icon-classes branch from 357049e to 5dbfcc3 Compare January 6, 2020 20:20

patrickrodee approved these changes Jan 6, 2020

View reviewed changes

asyncLiz added 4 commits January 8, 2020 13:41

chore(textfield): use --leading/trailing modifiers for icons

705dd66

chore(textfield): docs typo

3cf74a5

chore(textfield): revert hover fix

f9f135b

asyncLiz added 3 commits January 8, 2020 13:41

chore(textfield): fix unclosed css block

32b91d7

chore(textfield): separate position mixins for leading/trailing icons

4b5969a

chore(textfield): restore two-icons position mixin

1988a4a

asyncLiz force-pushed the fix/textfield/leading-trailing-icon-classes branch from 5dbfcc3 to 1988a4a Compare January 8, 2020 19:41

asyncLiz added 2 commits January 8, 2020 13:59

chore(textfield): update component test with icon classes

a3c5534

chore(textfield): update foundation test for preventDefault error

e761a87

allan-chen mentioned this pull request Jan 8, 2020

test(textfield): Migrate component unit test for icon to Jasmine #5423

Merged

asyncLiz merged commit 70c708d into master Jan 8, 2020

asyncLiz deleted the fix/textfield/leading-trailing-icon-classes branch January 8, 2020 20:17

abhiomkar mentioned this pull request Jan 9, 2020

feat(text-field): add feature targeting for styles #5378

Merged

This was referenced Feb 13, 2020

chore: Release #5609

Merged

chore: Release #5610

Closed

snyk-bot mentioned this pull request Mar 7, 2020

[Snyk] Upgrade material-components-web from 5.0.0 to 5.1.0 philwareham/textpattern-material-components-web#179

Open

adrianschmidt mentioned this pull request Dec 10, 2020

Update to the latest version of MDC Lundalogik/lime-elements#1070

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(textfield): add separate classes for leading/trailing icons #5367

fix(textfield): add separate classes for leading/trailing icons #5367

asyncLiz commented Dec 19, 2019

mdc-web-bot commented Dec 20, 2019

codecov-io commented Dec 20, 2019 •

edited

Loading

patrickrodee left a comment

asyncLiz commented Dec 20, 2019

patrickrodee commented Dec 20, 2019

asyncLiz commented Dec 20, 2019

patrickrodee commented Dec 20, 2019

mdc-web-bot commented Dec 23, 2019

mdc-web-bot commented Dec 23, 2019

mdc-web-bot commented Jan 6, 2020

mdc-web-bot commented Jan 6, 2020

patrickrodee left a comment

abhiomkar commented Jan 7, 2020

mdc-web-bot commented Jan 8, 2020

mdc-web-bot commented Jan 8, 2020

mdc-web-bot commented Jan 8, 2020

fix(textfield): add separate classes for leading/trailing icons #5367

fix(textfield): add separate classes for leading/trailing icons #5367

Conversation

asyncLiz commented Dec 19, 2019

mdc-web-bot commented Dec 20, 2019

codecov-io commented Dec 20, 2019 • edited Loading

Codecov Report

patrickrodee left a comment

Choose a reason for hiding this comment

asyncLiz commented Dec 20, 2019

patrickrodee commented Dec 20, 2019

asyncLiz commented Dec 20, 2019

patrickrodee commented Dec 20, 2019

mdc-web-bot commented Dec 23, 2019

mdc-web-bot commented Dec 23, 2019

mdc-web-bot commented Jan 6, 2020

mdc-web-bot commented Jan 6, 2020

patrickrodee left a comment

Choose a reason for hiding this comment

abhiomkar commented Jan 7, 2020

mdc-web-bot commented Jan 8, 2020

mdc-web-bot commented Jan 8, 2020

mdc-web-bot commented Jan 8, 2020

codecov-io commented Dec 20, 2019 •

edited

Loading