feat(text-field): Move final JS colors to mixins. Update demos #2006

williamernest · 2018-01-19T22:29:05Z

BREAKING CHANGE: Moves the text-field colors to mixins. Removes css only. Refer to the docs/demo for usage.

resolves #1526
fixes #1925

codecov-io · 2018-01-19T23:31:23Z

Codecov Report

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

@@           Coverage Diff           @@
##           master    #2006   +/-   ##
=======================================
  Coverage   99.43%   99.43%           
=======================================
  Files          84       84           
  Lines        3717     3717           
  Branches      485      485           
=======================================
  Hits         3696     3696           
  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 bf653a7...421ca50. Read the comment docs.

lynnmercier · 2018-01-20T00:31:31Z

Talked offline, we're going to split this into two PRs: one to remove the CSS only version of text field, and one to finish the Sass color mixin story

lynnmercier · 2018-01-20T00:54:24Z

demos/text-field.scss


  &.mdc-text-field--focused {
    @include mdc-text-field-label-color(rgba(blue, .87));
+    @include mdc-text-field-focused-textarea-stroke-color($focused-border);


Does this belong under the .mdc-text-field--focused selector?

No, this should just be renamed. It's our "double border" for the text area. There's a div and a textarea together with the outer div being non-focused and both of them together being the focused state.

lynnmercier · 2018-01-20T00:56:19Z

packages/mdc-textfield/README.md

 `mdc-text-field-outline-color($color)` | Customizes the color of the border of the outlined text-field. 
 `mdc-text-field-hover-outline-color($color)` | Customizes the hover color of the border of the outlined text-field.
 `mdc-text-field-focused-outline-color($color)` | Customizes the outlined border color when the text-field is focused.
+`mdc-text-field-box-background-color($color)` | Customizes the background color of the text-field box. 


box-fill-color, not background

lynnmercier · 2018-01-20T00:56:29Z

packages/mdc-textfield/README.md

 `mdc-text-field-outline-color($color)` | Customizes the color of the border of the outlined text-field. 
 `mdc-text-field-hover-outline-color($color)` | Customizes the hover color of the border of the outlined text-field.
 `mdc-text-field-focused-outline-color($color)` | Customizes the outlined border color when the text-field is focused.
+`mdc-text-field-box-background-color($color)` | Customizes the background color of the text-field box. 
+`mdc-text-field-textarea-stroke-color($color)` | Customizes the color of the border of the textarea. 
+`mdc-text-field-textarea-background-color($color)` | Customizes the color of the background of the textarea. 


textarea-fill-color, not background

lynnmercier · 2018-01-20T00:57:41Z

packages/mdc-textfield/README.md

 `mdc-text-field-helper-text-color($color)` | Customizes the color of the helper text following a text-field.
 `mdc-text-field-helper-text-validation-color($color)` | Customizes the color of the helper text when it's used as a validation message.
+`mdc-text-field-label-background-color` | Customizes the background color of the label. Used only by the textarea. 


hmmm, I think we should only allow clients to customize the fill color of the entire textarea, not specific to the label. I think we should delete this mixin (At least publicly)

lynnmercier · 2018-01-20T00:58:52Z

packages/mdc-textfield/_mixins.scss

+}
+
+@mixin mdc-text-field-textarea-background-color($color) {
+  &.mdc-text-field--textarea:not(.mdc-text-field--disabled) {


You shouldn't have mdc-text-field--textarea within this mixin. We should assume the developer calling this mixin has already specified a CSS selector which only selects mdc-text-field--textarea elements. So, mdc-text-field--textarea is unnecessarily specific.

lynnmercier · 2018-01-20T01:02:26Z

packages/mdc-textfield/_mixins.scss

@@ -126,6 +164,10 @@
  @include mdc-text-field-hover-outline-color($mdc-text-field-error-on-light);
  @include mdc-text-field-focused-outline-color($mdc-text-field-error-on-light);

+  // Textarea specific mixins


move this to mdc-text-field-textarea-invalid_ mixin. Its at HEAD of master now

Should we move all of the variants to their own invalid mixins? textarea is the only one that has an -invalid mixin.

I opted to condense everything here, but if we break out textarea, shouldn't we also break out outline ?
textfield mixin in master

material-components-web/packages/mdc-textfield/_mixins.scss

Line 341 in fc3a9d5

@mixin mdc-text-field-textarea-invalid_ {

invalid mixin in master

material-components-web/packages/mdc-textfield/_mixins.scss

Line 125 in fc3a9d5

@include mdc-text-field-outline-color($mdc-text-field-error-on-light);

lynnmercier · 2018-01-20T01:06:36Z

packages/mdc-textfield/_mixins.scss

+}
+
+@mixin mdc-text-field-fullwidth-bottom-line-color_($color) {
+  &.mdc-text-field--fullwidth:not(.mdc-text-field--textarea) {


.mdc-text-field--fullwidth is unnecessarily specific here. Assume the developer is already using a CSS selector that only selects mdc-text-field--fullwidth elements

lynnmercier · 2018-01-20T01:09:22Z

packages/mdc-textfield/bottom-line/_mixins.scss

@@ -17,19 +17,19 @@
 //Public

 @mixin mdc-text-field-bottom-line-color($color) {
-  &:not(.mdc-text-field--disabled) {
+  &:not(.mdc-text-field--disabled):not(.mdc-text-field--outlined):not(.mdc-text-field--textarea) {


Don't we say in our README to never use a mdc-text-field__bottom-line element within a mdc-text-field--outlined element? or mdc-text-field--textarea?

Yes. They share the mdc-text-field__input, the bottom line color is automatically set, resulting in some weird behavior on outline and textfield if we don't stop it here. This mixin applies border-bottom-color, and outline applies border-color, resulting in a weird line

Without :not(.mdc-text-field--outlined):not(.mdc-text-field--textarea)

With

The mdc-text-field-border-bottom-line-color mixin is applied to the border-bottom-color of the input field for idle and hover. The actual bottom line element is a separate element that only gets activated on focus.

lynnmercier · 2018-01-20T01:20:09Z

Oh and...this PR is not a breaking change anymore, now that you moved the CSS only stuff to the other PR

lynnmercier

LGTM

lynnmercier · 2018-01-22T19:23:43Z

packages/mdc-textfield/_mixins.scss

-// Private mixins
-
-@mixin mdc-text-field-invalid-label-shake-keyframes_($modifier, $positionY, $scale: .75) {
+@mixin mdc-text-field-invalid-label-shake-keyframes_($modifier, $positionY, $positionX: 0%, $scale: .75) {


is this change something that should be in this PR? Or unrelated to this PR?

Unrelated. This was my merge with master.

lynnmercier · 2018-01-22T19:33:11Z

packages/mdc-textfield/_mixins.scss

@@ -324,13 +312,18 @@
  .mdc-text-field__label--float-above ~ .mdc-text-field__outline {
    opacity: 1;
  }
+
+  &.mdc-text-field--invalid {
+    @include mdc-text-field-outlined-invalid_;


Move this call to mdc-text-field.scss

.mdc-text-field--focused.mdc-text-field--invalid { @include mdc-text-field-outlined-invalue_; }

lynnmercier · 2018-01-22T19:34:06Z

packages/mdc-textfield/_mixins.scss

-    padding: 8px 16px;
+    margin-top: 2px;
+    margin-left: 8px;
+    padding: 8px;


Is this a new change? Or did you move existing logic into here?

Unrelated. This was my merge with master.

lynnmercier · 2018-01-22T19:35:12Z

packages/mdc-textfield/_mixins.scss

  }

+  &.mdc-text-field--invalid {
+    @include mdc-text-field-textarea-invalid_;


Move this call to mdc-text-field.scss

lynnmercier · 2018-01-22T19:35:16Z

packages/mdc-textfield/_mixins.scss

-  &.mdc-text-field--focused:not(.mdc-text-field--invalid) {
-    @include mdc-text-field-focused-textarea-stroke-color(primary);
+  &.mdc-text-field--disabled {
+    @include mdc-text-field-textarea-disabled_;


Move this call to mdc-text-field.scss

lynnmercier · 2018-01-22T19:36:15Z

packages/mdc-textfield/_mixins.scss

-  &.mdc-text-field--disabled {
-    @include mdc-text-field-textarea-disabled_;
+  &.mdc-text-field--focused {
+    @include mdc-text-field-textarea-stroke-color(primary);


Move this call to mdc-text-field.scss

.mdc-text-field--textarea.mdc-text-field--focused { @include mdc-text-field-textarea-stroke-color(primary); }

williamernest added 4 commits January 19, 2018 14:26

feat(text-field): Move final JS colors to mixins. Update demos

381aa4d

feat(text-field): Update docs with new mixins

9eccd2f

feat(text-field): Merge with master

c4b85e8

feat(text-field): Update readme to remove css only examples

a05d24b

williamernest added 2 commits January 19, 2018 16:50

feat(text-field): Merge master

91136bb

feat(text-field): Make linter happy

ceb9304

lynnmercier suggested changes Jan 20, 2018

View reviewed changes

lynnmercier self-assigned this Jan 20, 2018

williamernest added 6 commits January 19, 2018 21:11

feat(text-field): Rename mixins. Reduce specificity. Update Demos.

2a8f39d

feat(text-field): Update demo. Add invalid mixins back.

f6f210d

feat(text-field): Remove focused-textarea color mixin

ee58eeb

feat(text-field): Add removed border from textarea disabled

97439f0

feat(text-field): Merge master

cdbeaf4

feat(text-field): Remove extra line

678629e

lynnmercier approved these changes Jan 22, 2018

View reviewed changes

williamernest and others added 2 commits January 22, 2018 12:25

feat(text-field): Move variant--state mixins to mdc-text-field.scss

6556e31

Merge branch 'master' into feat/text-field/color-mixins-final

421ca50

williamernest merged commit 989c516 into master Jan 22, 2018

williamernest deleted the feat/text-field/color-mixins-final branch January 22, 2018 21:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(text-field): Move final JS colors to mixins. Update demos #2006

feat(text-field): Move final JS colors to mixins. Update demos #2006

williamernest commented Jan 19, 2018 •

edited

Loading

codecov-io commented Jan 19, 2018 •

edited

Loading

lynnmercier commented Jan 20, 2018

lynnmercier Jan 20, 2018

williamernest Jan 20, 2018

lynnmercier Jan 20, 2018

lynnmercier Jan 20, 2018

lynnmercier Jan 20, 2018

lynnmercier Jan 20, 2018

lynnmercier Jan 20, 2018

williamernest Jan 20, 2018 •

edited

Loading

lynnmercier Jan 20, 2018

lynnmercier Jan 20, 2018

williamernest Jan 20, 2018 •

edited

Loading

lynnmercier commented Jan 20, 2018

lynnmercier left a comment

lynnmercier Jan 22, 2018

williamernest Jan 22, 2018

lynnmercier Jan 22, 2018

lynnmercier Jan 22, 2018

williamernest Jan 22, 2018

lynnmercier Jan 22, 2018

lynnmercier Jan 22, 2018

lynnmercier Jan 22, 2018

feat(text-field): Move final JS colors to mixins. Update demos #2006

feat(text-field): Move final JS colors to mixins. Update demos #2006

Conversation

williamernest commented Jan 19, 2018 • edited Loading

codecov-io commented Jan 19, 2018 • edited Loading

Codecov Report

lynnmercier commented Jan 20, 2018

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

williamernest Jan 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

williamernest Jan 20, 2018 • edited Loading

Choose a reason for hiding this comment

lynnmercier commented Jan 20, 2018

lynnmercier left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

williamernest commented Jan 19, 2018 •

edited

Loading

codecov-io commented Jan 19, 2018 •

edited

Loading

williamernest Jan 20, 2018 •

edited

Loading

williamernest Jan 20, 2018 •

edited

Loading