feat(text-field): Move color for default text-field to mixins. #1885

williamernest · 2018-01-05T21:54:07Z

references: #1526

Moves colors for the default text-field into mixins. Separated private/public mixins to prevent users from modifying disabled state colors.

lynnmercier · 2018-01-05T22:51:25Z

packages/mdc-textfield/_mixins.scss

+  }
+}
+
+@mixin mdc-text-field-bottom-line-hover-color($color) {


mdc-text-field-hover-bottom-line-color is better I think. Since you hover on the text field, not the bottom line.

lynnmercier · 2018-01-05T22:51:38Z

packages/mdc-textfield/_mixins.scss

+  }
+}
+
+@mixin mdc-text-field-bottom-line-focused-color($color) {


mdc-text-field-focused-bottom-line-color is better I think. Since you focus on the text field, not the bottom line.

lynnmercier · 2018-01-05T22:59:02Z

packages/mdc-textfield/_mixins.scss

+  }
+}
+
+@mixin mdc-text-field-input-ink-color($color) {


just use mdc-text-field-ink-color. <input> is an implementation detail

lynnmercier · 2018-01-05T23:02:13Z

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

@@ -38,14 +38,21 @@
 // postcss-bem-linter: define text-field

 .mdc-text-field {
+  @include mdc-text-field-bottom-line-color($mdc-text-field-underline-on-light-idle);
+  @include mdc-text-field-bottom-line-hover-color($mdc-text-field-underline-on-light);
+  @include mdc-text-field-bottom-line-focused-color(primary);


Love these bottom line mixins

lynnmercier · 2018-01-05T23:02:46Z

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

+  @include mdc-text-field-bottom-line-focused-color(primary);
+  @include mdc-text-field-label-ink-color($mdc-text-field-light-label);
+  @include mdc-text-field-input-ink-color(text-primary-on-light);
+  @include mdc-text-field-input-placeholder-ink-color(text-hint-on-light);


combine the label-color, input-placeholder-ink-color (one is CSS only and one is JS)

lynnmercier · 2018-01-05T23:03:06Z

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

+  @include mdc-text-field-label-ink-color($mdc-text-field-light-label);
+  @include mdc-text-field-input-ink-color(text-primary-on-light);
+  @include mdc-text-field-input-placeholder-ink-color(text-hint-on-light);
+  @include mdc-text-field-input-placeholder-hover-ink-color($mdc-text-field-light-placeholder);


drop this "change label color on hover" feature from the CSS only version of text field

lynnmercier · 2018-01-05T23:03:30Z

packages/mdc-textfield/_mixins.scss

+  }
+}
+
+@mixin mdc-text-field-label-ink-color($color) {


you can just say label-color there is only one way to color the label

.mdc-text-field--textarea .mdc-text-field__label has a background-color attribute.

codecov-io · 2018-01-05T23:12:04Z

Codecov Report

Merging #1885 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1885   +/-   ##
=======================================
  Coverage   99.43%   99.43%           
=======================================
  Files          84       84           
  Lines        3721     3721           
  Branches      484      484           
=======================================
  Hits         3700     3700           
  Misses         21       21

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 10ca23e...8392863. Read the comment docs.

lynnmercier · 2018-01-06T00:39:51Z

packages/mdc-textfield/README.md

@@ -198,6 +198,12 @@ Mixin | Description
 --- | ---
 `mdc-text-field-box-corner-radius($radius)` | Customizes the border radius for a box text field
 `mdc-text-field-textarea-corner-radius($radius)` | Customizes the border radius for a `<textarea>` text field
+`mdc-text-field-bottom-line-color($color)` | Customizes the color of the default bottom line of the text-field.
+`mdc-text-field-hover-bottom-line-color($color)` | Customizes the hover color of the bottom line of the text-field. 
+`mdc-text-field-focused-bottom-line-color($color)` | Customizes the bottom-line ripple color when the text-field is selected. 


when the text field is focused, not selected

lynnmercier · 2018-01-06T00:45:32Z

packages/mdc-textfield/_mixins.scss

+
+@mixin mdc-text-field-bottom-line-color_($color) {
+  .mdc-text-field__input,
+  .mdc-text-field__input::placeholder {


why do you need the ::placeholder? Maybe add a comment explaining this, since it is not obvious

I definitely don't need it there. I need it on the label color mixin for the css only version uses a placeholder instead of a label.

lynnmercier · 2018-01-08T17:32:00Z

packages/mdc-textfield/README.md

@@ -194,10 +194,21 @@ CSS Class | Description

 ### Sass Mixins

+To customize the colors of any part of the text-field, use the following mixins. We recommend you apply 
+these mixins within CSS selectors like .foo-text-field:not(.mdc-text-field--focused) to select your inactive text-fields, 


to select your unfocused text fields

(no dashes in text field, it is two words)

lynnmercier · 2018-01-08T17:33:02Z

packages/mdc-textfield/README.md

+To customize the colors of any part of the text-field, use the following mixins. We recommend you apply 
+these mixins within CSS selectors like .foo-text-field:not(.mdc-text-field--focused) to select your inactive text-fields, 
+and .foo-tab.mdc-text-field--focused to select your active text-fields (Note: the `mdc-text-field-focused-bottom-line-color`
+mixin should be applied from the not focused class `foo-text-field:not(.mdc-tab--focused)`). 


put this note in an '> NOTE: ' comment. You can see examples though-out this README

lynnmercier · 2018-01-08T17:33:51Z

packages/mdc-textfield/_mixins.scss

+  &:not(.mdc-text-field--disabled) {
+    @include mdc-text-field-bottom-line-color_($color);
+  }
+}


move all the bottom-line mixins to bottom-line/_mixins.scss (and then export them through this file)

lynnmercier · 2018-01-08T17:34:06Z

packages/mdc-textfield/_mixins.scss

+  &:not(.mdc-text-field--disabled) {
+    @include mdc-text-field-label-color_($color);
+  }
+}


move all the label mixins to label/_mixins.scss (and then export them through this file)

lynnmercier · 2018-01-08T17:34:21Z

packages/mdc-textfield/_mixins.scss

+
+// Private Color Mixins
+
+@mixin mdc-text-field-bottom-line-color_($color) {


move the private ones too

lynnmercier

LGTM

lynnmercier · 2018-01-08T18:14:09Z

packages/mdc-textfield/README.md

-and .foo-tab.mdc-text-field--focused to select your active text-fields (Note: the `mdc-text-field-focused-bottom-line-color`
-mixin should be applied from the not focused class `foo-text-field:not(.mdc-tab--focused)`). 
+these mixins within CSS selectors like .foo-text-field:not(.mdc-text-field--focused) to select your unfocused text fields, 
+and .foo-tab.mdc-text-field--focused to select your active text-fields.


your focused text fields. Fix this too!!

And add a mention about the invalid state.

…olor, remove hover placeholder color change. Add demos for Alternate Colors and Alternate Error Colors

…l color mixin

…ubcomponent directories.

…state colors

…ult_component

Upcoming work: * implement theme * add component tests fixes #1885 fixes #2109 fixes #2108 fixes #523 fixes #1028 fixes #1671 PiperOrigin-RevId: 367260030

williamernest force-pushed the feat/text-field/sass_color_mixins_for_default_component branch from 45c9ae3 to 07205bd Compare January 5, 2018 22:53

lynnmercier suggested changes Jan 5, 2018

View reviewed changes

williamernest force-pushed the feat/text-field/sass_color_mixins_for_default_component branch 2 times, most recently from 08b875d to 8abecc7 Compare January 6, 2018 00:21

lynnmercier suggested changes Jan 6, 2018

View reviewed changes

acdvorak assigned lynnmercier Jan 6, 2018

lynnmercier suggested changes Jan 8, 2018

View reviewed changes

lynnmercier approved these changes Jan 8, 2018

View reviewed changes

williamernest added 5 commits January 8, 2018 10:21

feat(text-field): Move color for default text-field to mixins.

4d6865e

WIP: Update color mixin names. Make placeholder color same as label c…

3fb168b

…olor, remove hover placeholder color change. Add demos for Alternate Colors and Alternate Error Colors

WIP: Update readme. Move placeholder selector to proper place on labe…

b3e51c0

…l color mixin

fix(text-field): Update Readme. Move subcomponent color mixins into s…

2e5c0a9

…ubcomponent directories.

feat(text-field): Update readme to document how to implement invalid …

c54ebf4

…state colors

williamernest force-pushed the feat/text-field/sass_color_mixins_for_default_component branch from f60c4df to c54ebf4 Compare January 8, 2018 18:21

williamernest added 2 commits January 8, 2018 12:03

Merge branch 'master' into feat/text-field/sass_color_mixins_for_defa…

ed2e65b

…ult_component

Merge branch 'master' into feat/text-field/sass_color_mixins_for_defa…

8392863

…ult_component

williamernest merged commit ca1eadc into master Jan 8, 2018

williamernest deleted the feat/text-field/sass_color_mixins_for_default_component branch January 11, 2018 18:18

bonniezhou mentioned this pull request Jun 15, 2018

Split up the "variants" CSS classes in text field into private mixins #1599

Closed

copybara-service bot mentioned this pull request Jul 23, 2021

feat(slider): BREAKING: Implements m2 slider #7259

Closed

copybara-service bot pushed a commit that referenced this pull request Jul 23, 2021

feat(slider): BREAKING: Implements m2 slider

be37826

Upcoming work: * implement theme * add component tests fixes #1885 fixes #2109 fixes #2108 fixes #523 fixes #1028 fixes #1671 PiperOrigin-RevId: 367260030

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(text-field): Move color for default text-field to mixins. #1885

feat(text-field): Move color for default text-field to mixins. #1885

williamernest commented Jan 5, 2018 •

edited

Loading

lynnmercier Jan 5, 2018

lynnmercier Jan 5, 2018

lynnmercier Jan 5, 2018

lynnmercier Jan 5, 2018

lynnmercier Jan 5, 2018

lynnmercier Jan 5, 2018

lynnmercier Jan 5, 2018

williamernest Jan 5, 2018

codecov-io commented Jan 5, 2018 •

edited

Loading

lynnmercier Jan 6, 2018

lynnmercier Jan 6, 2018

williamernest Jan 6, 2018

lynnmercier Jan 8, 2018

lynnmercier Jan 8, 2018

lynnmercier Jan 8, 2018

lynnmercier Jan 8, 2018

lynnmercier Jan 8, 2018

lynnmercier left a comment

lynnmercier Jan 8, 2018


		// Private Color Mixins

		@mixin mdc-text-field-bottom-line-color_($color) {

feat(text-field): Move color for default text-field to mixins. #1885

feat(text-field): Move color for default text-field to mixins. #1885

Conversation

williamernest commented Jan 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Jan 5, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lynnmercier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

williamernest commented Jan 5, 2018 •

edited

Loading

codecov-io commented Jan 5, 2018 •

edited

Loading