chore(notched-outline): separate outline from text-field #2326

moog16 · 2018-02-28T22:31:23Z

fixes #2128
related #1982

BREAKING CHANGE: removed mdc-text-field__outline element for mdc-notched-outline.
Renamed mdc-text-field-outlined-corner-radius to mdc-text-field-outline-corner-radius.

…ate with mdc-text-field

WIP- JS tests

…abel

…t-field

…-components/material-components-web into chore/text-field/decouple-label

…ext-field mixins

…-components/material-components-web into chore/text-field/decouple-label

…l. updated tests

…ext-field accordingly

…d in respective places

…-components/material-components-web into chore/text-field/decouple-label

moog16 · 2018-03-06T04:41:26Z

packages/mdc-textfield/README.md

+Mixin | Description
+--- | ---
+`mdc-text-field-outline-color($color)` | Customizes the border color of the outline text field.
+`mdc-text-field-outlined-corner-radius($radius)` | Sets the border radius of of the text field outline variant.


@bonniezhou I think this should be changed to mdc-text-field-outline-corner-radius what do you think?

Actually I'm going to change mdc-text-field-outline-color --> mdc-text-field-outlined-color so it isn't a breaking change.

How is that not a breaking change? AFAICT mdc-text-field-outline-color is also already a thing that exists.

Plus, it seems like the one "outlined" name is the outlier, and "outline" sounds more natural to me in mixin names, so IMO I'd go with your first suggestion.

True, but that mdc-text-field-outline-color is in the mdc-text-field__outline package, which is moved to the mdc-notched-outline. I this would cover it:

BREAKING CHANGE: removed mdc-text-field__outline element for mdc-notched-outline

But yes you're right, mdc-text-field-outline-color and mdc-text-field-outline-corner-radius are more natural.

Yeah I think initially "Outlined" should refer to the variant of Text Field and "Outline" refers to the actual subcomponent. But changing them both to "Outline" sounds like a good idea, and would cause less confusion.

… consistency

BREAKING CHANGE: renamed mdc-text-field-outlined-corner-radius to mdc-text-field-outline-corner-radius

bonniezhou

LGTM!

acdvorak · 2018-03-09T21:05:29Z

@lynnjepsen will take the review from here 😄

lynnmercier · 2018-03-14T20:47:57Z

package-lock.json

@@ -2,6 +2,16 @@
  "requires": true,
  "lockfileVersion": 1,
  "dependencies": {
+    "JSONStream": {


I don't think package-lock.json should change in this PR. We're only adding new modules.

lynnmercier · 2018-03-14T20:59:34Z

packages/mdc-textfield/_mixins.scss

  @include mdc-text-field-outline-color($mdc-text-field-error);
  @include mdc-text-field-hover-outline-color($mdc-text-field-error);
  @include mdc-text-field-focused-outline-color($mdc-text-field-error);
 }

 @mixin mdc-text-field-outlined-focused_ {
-  .mdc-text-field__outline-path {
+  .mdc-notched-outline__path {
    stroke-width: 2px;


I think we should make a mixin to support customizing the width of the notched outline. And then call that mixin here.

lynnmercier · 2018-03-14T21:01:22Z

packages/mdc-textfield/_mixins.scss

-    border: 1px solid;
+  .mdc-notched-outline,
+  .mdc-notched-outline__idle {
+    @include mdc-text-field-outline-corner-radius($mdc-text-field-border-radius);


calling a text-field mixin within the scope of .mdc-notched-outline seems backwards. Shouldn't we call a notched outline mixin here?

good catch...that should be a notched-outline mixin

lynnmercier · 2018-03-14T21:03:33Z

packages/mdc-textfield/_mixins.scss

+
+@mixin mdc-text-field-outline-color_($color) {
+  @include mdc-notched-outline-idle-color($color);
+  @include mdc-notched-outline-color($color);


Add a comment that outlined version of text field wants the "idle" and "notched" version to have the same color. This covers two cases: 1) the "idle" state when a text field renders with no value 2) the "notched" state when a text field renders with an initial value

lynnmercier · 2018-03-14T21:21:53Z

packages/mdc-textfield/_mixins.scss

+    .mdc-text-field__icon:hover ~ {
+      @include mdc-notched-outline-idle-color($color);
+
+      .mdc-notched-outline {


Put a bunch of comments here around the ~ and sibling selector, and why thats hella complicated because mdc-notched-outline__path is a sub-element but mdc-notched-outline__idle is a sibling element.

And then file a GitHub issue to investigate making mdc-notched-outline__idle a sub-element of mdc-notched-outline.

lynnmercier · 2018-03-14T21:24:34Z

packages/mdc-textfield/adapter.js

@@ -43,7 +43,7 @@ let NativeInputType;
 *   helperText: (!MDCTextFieldHelperTextFoundation|undefined),
 *   icon: (!MDCTextFieldIconFoundation|undefined),
 *   label: (!MDCFloatingLabelFoundation|undefined),
- *   outline: (!MDCTextFieldOutlineFoundation|undefined)
+ *   outline: (!MDCNotchedOutlineFoundation|undefined)


You dont need lineRipple, label, or outline anymore. MDCTextFieldFoundation only needs helperText and icon

…eadme

…al-components/material-components-web into chore/text-field/decouple-outline

…xins

…al-components/material-components-web into chore/text-field/decouple-outline

lynnmercier · 2018-03-15T00:49:12Z

packages/mdc-notched-outline/_mixins.scss

+}
+
+@mixin mdc-notched-outline-corner-radius($radius) {
+  &,


Add some comments here about why you have to do this

I think you are looking at an older commit. This is now

@mixin mdc-notched-outline-corner-radius($radius) { border-radius: $radius; } @mixin mdc-notched-outline-idle-corner-radius($radius) { .mdc-notched-outline__idle { border-radius: $radius; } }

lynnmercier · 2018-03-15T18:07:26Z

package-lock.json

@@ -2,16 +2,6 @@
  "requires": true,
  "lockfileVersion": 1,
  "dependencies": {
-    "JSONStream": {


This changed again...remove this from PR

Ah nevermind. Ignore me.

lynnmercier · 2018-03-15T23:23:47Z

packages/mdc-notched-outline/README.md

+`mdc-notched-outline-stroke-width($width)` | Changes notched outline width to a specified pixel value.
+`mdc-notched-outline-corner-radius($radius)` | Sets the corner radius of the notched outline element to the given number.
+`mdc-notched-outline-idle-corner-radius($radius)` | Sets the corner radius of the notched outline element in idle state.
+> NOTE:


Change this whole NOTE to

Because notched-outline has sibling elements, you need to call the "idle" Sass mixins with parent selectors.
Consider the following example HTML:

<div class="foo-line-ripple-parent"> <div class="mdc-notched-outline foo-line-ripple"> <svg> <path class="mdc-notched-outline__path"/> </svg> </div> <div class="mdc-notched-outline__idle"></div> </div>

In order to customize any "non-idle" part of notched-outline, use the .foo-line-ripple CSS selector:

.foo-line-ripple { @include mdc-notched-outline-color(fooColor); }

But in order to customize any "idle" part of the notched-outline, you must use the .foo-line-ripple-parent CSS selector:

.foo-line-ripple-parent { @include mdc-notched-outline-idle-color(fooColor); }

Matt Goo added 30 commits February 6, 2018 13:55

chore(floating-label): moved floating-label to new package and integr…

a9d6c3c

…ate with mdc-text-field

chore(floating-label): changed all css classes to floating-label

9b750cd

WIP- JS tests

Merge branch 'master' into chore/text-field/decouple-label

4b01134

fix(floating-label): fixed all tests

5a4500f

chore(floating-label): created test directory

324a638

fix(floating-label): removed label foundation from text-field

51f16f6

Merge branch 'master' into chore/text-field/decouple-label

af02b19

fix(floating-label): tests

6f9abdf

Merge branch 'master' into chore/text-field/decouple-label

028c00d

fix(floating-label): updated readmes to reflect changes to floating-l…

024ff39

…abel

fix(floating-label): closure annotation

2f72c61

Merge branch 'master' into chore/text-field/decouple-label

c2f6b95

chore(floating-label): moved text-field-placeholder css rule into tex…

f452125

…t-field

Merge branch 'chore/text-field/decouple-label' of github.com:material…

f00ef39

…-components/material-components-web into chore/text-field/decouple-label

chore(text-field): moved invalid label keyframe animation shakes to t…

4a9846c

…ext-field mixins

fix(text-field): removed export of floating label

1f2cf62

Merge branch 'master' into chore/text-field/decouple-label

ab8b035

fix(text-field): closure annotations

040f739

Merge branch 'chore/text-field/decouple-label' of github.com:material…

d3871a3

…-components/material-components-web into chore/text-field/decouple-label

Merge branch 'master' into chore/text-field/decouple-label

252ffc4

fix(floating-label): sass compliation failure

43bd881

fix(text-field): updated foundation methods to guard against !hasLabe…

9b627e2

…l. updated tests

Merge branch 'master' into chore/text-field/decouple-label

808b6ff

Merge branch 'master' into chore/text-field/decouple-label

bed6857

fix(text-field): changed floating-label ink color mixin and updated t…

2c1c55e

…ext-field accordingly

fix(floating-label): updated API for shake and float in component, an…

d7e1776

…d in respective places

Merge branch 'master' into chore/text-field/decouple-label

8a03462

fix(floating-label): reorder scss import statements alphabetical

5973760

Merge branch 'chore/text-field/decouple-label' of github.com:material…

8385248

…-components/material-components-web into chore/text-field/decouple-label

Merge branch 'master' into chore/text-field/decouple-label

182b4b1

moog16 commented Mar 6, 2018

View reviewed changes

Matty Goo added 4 commits March 6, 2018 09:32

Merge branch 'master' into chore/text-field/decouple-outline

4d4c4fe

fix(notched-outline): removed unused scss rules and renamed mixin for…

b98ae09

… consistency

fix(text-field): changed mixin name

b7b9ce7

BREAKING CHANGE: renamed mdc-text-field-outlined-corner-radius to mdc-text-field-outline-corner-radius

fix(text-field): moved textfield outline mixins to appropriate section

fc35362

bonniezhou approved these changes Mar 6, 2018

View reviewed changes

Matt Goo added 2 commits March 6, 2018 19:43

fix(package): remove duplicate package.json entries

2a30f0d

Merge branch 'master' into chore/text-field/decouple-outline

9b539e0

acdvorak assigned lynnmercier and unassigned acdvorak Mar 9, 2018

Merge branch 'master' into chore/text-field/decouple-outline

adf5817

lynnmercier suggested changes Mar 14, 2018

View reviewed changes

moog16 mentioned this pull request Mar 14, 2018

.mdc-notched-outline__idle should be a sub-element of .mdc-notched-outline #2404

Open

Matt Goo added 8 commits March 14, 2018 15:22

fix(notched-outline): added 2 mixins to notched-outline and updated r…

3f3ca89

…eadme

Merge branch 'master' into chore/text-field/decouple-outline

f351e17

test(notched-outline): added notchedOutline to deps to pass test

526a69f

Merge branch 'chore/text-field/decouple-outline' of github.com:materi…

67bb0a5

…al-components/material-components-web into chore/text-field/decouple-outline

fix(notched-outline): updated sass mixins to using 2 broder-radius mi…

e9141ea

…xins

Merge branch 'master' into chore/text-field/decouple-outline

0a6294c

fix(text-field): lint fix

9378b7c

Merge branch 'chore/text-field/decouple-outline' of github.com:materi…

8157968

…al-components/material-components-web into chore/text-field/decouple-outline

lynnmercier suggested changes Mar 15, 2018

View reviewed changes

Matt Goo added 2 commits March 15, 2018 16:27

Merge branch 'master' into chore/text-field/decouple-outline

142a142

fix(notched-outline): readme update explaining hwo to use mixins

0ec3a8f

lynnmercier approved these changes Mar 15, 2018

View reviewed changes

moog16 merged commit e215ca8 into master Mar 16, 2018

This was referenced Mar 19, 2018

Update material-components-web requirement to ^0.33.0 gasolin/gasolin.github.io#7

Closed

build: bump material-components-web from 0.32.0 to 0.33.0 fintechstudios/angularjs-mdc#74

Closed

moog16 deleted the chore/text-field/decouple-outline branch May 29, 2018 20:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(notched-outline): separate outline from text-field #2326

chore(notched-outline): separate outline from text-field #2326

moog16 commented Feb 28, 2018 •

edited

Loading

moog16 Mar 6, 2018

moog16 Mar 6, 2018

kfranqueiro Mar 6, 2018

moog16 Mar 6, 2018 •

edited

Loading

bonniezhou Mar 6, 2018

bonniezhou left a comment

acdvorak commented Mar 9, 2018

lynnmercier Mar 14, 2018

lynnmercier Mar 14, 2018

lynnmercier Mar 14, 2018

moog16 Mar 14, 2018

lynnmercier Mar 14, 2018

lynnmercier Mar 14, 2018

lynnmercier Mar 14, 2018

lynnmercier Mar 15, 2018

moog16 Mar 15, 2018

lynnmercier Mar 15, 2018

lynnmercier Mar 15, 2018

lynnmercier Mar 15, 2018

chore(notched-outline): separate outline from text-field #2326

chore(notched-outline): separate outline from text-field #2326

Conversation

moog16 commented Feb 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moog16 Mar 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bonniezhou left a comment

Choose a reason for hiding this comment

acdvorak commented Mar 9, 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

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

moog16 commented Feb 28, 2018 •

edited

Loading

moog16 Mar 6, 2018 •

edited

Loading