refactor(notched-outline): remove text-field notched outline styles and coupling #2401

moog16 · 2018-03-14T21:26:13Z

fixes #2128
related #1982

…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

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

…d-outline/update-api

codecov-io · 2018-03-16T05:34:40Z

Codecov Report

Merging #2401 into master will increase coverage by 0.02%.
The diff coverage is 83.78%.

@@            Coverage Diff             @@
##           master    #2401      +/-   ##
==========================================
+ Coverage   98.88%   98.91%   +0.02%     
==========================================
  Files         104      104              
  Lines        4210     4234      +24     
  Branches      533      534       +1     
==========================================
+ Hits         4163     4188      +25     
+ Misses         47       46       -1

Impacted Files	Coverage Δ
packages/mdc-notched-outline/constants.js	`100% <100%> (ø)`	⬆️
packages/mdc-notched-outline/foundation.js	`100% <100%> (+12.5%)`	⬆️
packages/mdc-textfield/foundation.js	`98.89% <100%> (+0.03%)`	⬆️
packages/mdc-notched-outline/index.js	`77.77% <33.33%> (-0.8%)`	⬇️
packages/mdc-textfield/index.js	`95.51% <50%> (-0.6%)`	⬇️

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 a8b3193...c015302. Read the comment docs.

williamernest

A few minor nits, but LGTM

williamernest · 2018-03-19T15:41:36Z

packages/mdc-notched-outline/README.md

 `setOutlinePathAttr(value: string) => void` | Sets the "d" attribute of the outline element's SVG path.
 `getIdleOutlineStyleValue(propertyName: string) => string` | Returns the idle outline element's computed style value of a given css `propertyName`.

 ### `MDCNotchedOutlineFoundation`

 Method Signature | Description
 --- | ---
+`notch(activateNotch: boolean)` | Adds remove the outline notched selector if activateNotch is true. Updates the component to activate/deactivate notched outline.


I think this description needs to be reworded. Maybe just the second sentence is needed

williamernest · 2018-03-20T00:20:54Z

packages/mdc-notched-outline/foundation.js

@@ -50,13 +57,31 @@ class MDCNotchedOutlineFoundation extends MDCFoundation {
    super(Object.assign(MDCNotchedOutlineFoundation.defaultAdapter, adapter));
  }

+  /**
+   * Adds the outline notched selector and updates the notch width
+   * if activateNotch is true, otherwise it will remove notched


"remove the notched selector"

williamernest · 2018-03-20T00:22:56Z

packages/mdc-textfield/README.md

@@ -274,6 +274,6 @@ Method Signature | Description
 `activateFocus() => void` | Activates the focus state of the Text Field. Normally called in response to the input focus event.
 `deactivateFocus() => void` | Deactivates the focus state of the Text Field. Normally called in response to the input blur event.
 `setHelperTextContent(content: string) => void` | Sets the content of the helper text


Trailing period.

nice find <needle_in_haystack_emoji>

…rial-components/material-components-web into refactor/notched-outline/update-api

lynnmercier · 2018-03-21T16:31:52Z

packages/mdc-notched-outline/README.md

@@ -82,6 +83,7 @@ Mixin | Description

 Method Signature | Description
 --- | ---
+`notch(activateNotch: boolean)` | Updates outline to activate/deactivate notch in outline path.
 `updateSvgPath(notchWidth: number, isRtl: boolean) => void` | Updates the SVG of the outline element with a notch calculated based off of the notchWidth. The notch will appear left justified, unless isRtl is true.


Could we change this so there are two methods
notch(notchWidth: number, isRtl: boolean) => void
and
closeNotch() => void

lynnmercier · 2018-03-21T16:33:49Z

packages/mdc-textfield/README.md

@@ -248,7 +248,7 @@ Method Signature | Description
 `isFocused() => boolean` | Returns whether the input is focused
 `isRtl() => boolean` | Returns whether the direction of the root element is set to RTL
 `hasOutline() => boolean` | Returns whether there is an outline element
-`updateOutlinePath(labelWidth: number, isRtl: boolean) => void` | Updates the outline path to create a notch for the label element
+`notchOutline(labelWidth: number, isRtl: boolean) => void` | Updates the outline path to create a notch for the label element



Do you also need a closeOutlineNotch method in MDCTextFieldAdapter?

lynnmercier · 2018-03-21T16:37:14Z

packages/mdc-textfield/foundation.js

+    if (activateNotch) {
+      this.adapter_.notchOutline(labelWidth, isRtl);
+    } else {
+      this.adapter_.notchOutline(0);


setting the labelWidth = 0 in order to simulate "closing the notch" is unintuitive. Just add another method that explicitly closes the notch

…undation

lynnmercier · 2018-03-21T17:41:00Z

packages/mdc-textfield/foundation.js

@@ -204,7 +207,11 @@ class MDCTextFieldFoundation extends MDCFoundation {
    const labelScale = isDense ? numbers.DENSE_LABEL_SCALE : numbers.LABEL_SCALE;
    const labelWidth = this.adapter_.getLabelWidth() * labelScale;
    const isRtl = this.adapter_.isRtl();


calculate all this labelWidth and isRTL within the if statement for opening the notch.

lynnmercier · 2018-03-21T17:41:16Z

packages/mdc-textfield/foundation.js

@@ -193,9 +195,10 @@ class MDCTextFieldFoundation extends MDCFoundation {
  }

  /**
-   * Updates the focus outline for outlined text fields.
+   * Activates/deactivates the notched outline.
+   * @param {boolean} activateNotch


nit: open notch? instead of activate?

Change as per material-components-web v0.34.1. reference: material-components/material-components-web#2401

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

Matty Goo added 7 commits March 15, 2018 10:31

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

Merge branch 'chore/text-field/decouple-outline' into refactor/notche…

e49a03b

…d-outline/update-api

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

142a142

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

0ec3a8f

Merge branch 'chore/text-field/decouple-outline' into refactor/notche…

74fc299

…d-outline/update-api

moog16 changed the base branch from chore/text-field/decouple-outline to master March 16, 2018 05:18

Merge branch 'master' into refactor/notched-outline/update-api

b32fc7b

Matt Goo added 2 commits March 16, 2018 13:43

Merge branch 'master' into refactor/notched-outline/update-api

61e3d88

fix(notched-outline): added more test coverage

0656a9e

moog16 assigned williamernest Mar 16, 2018

Merge branch 'master' into refactor/notched-outline/update-api

cc391d2

williamernest approved these changes Mar 20, 2018

View reviewed changes

Matty Goo added 3 commits March 20, 2018 11:33

Merge branch 'master' into refactor/notched-outline/update-api

f2e604b

fix(notched-outline): updated readme

4d8dc0e

Merge branch 'refactor/notched-outline/update-api' of github.com:mate…

509e530

…rial-components/material-components-web into refactor/notched-outline/update-api

moog16 assigned lynnmercier and unassigned williamernest Mar 20, 2018

Merge branch 'master' into refactor/notched-outline/update-api

d334297

lynnmercier suggested changes Mar 21, 2018

View reviewed changes

Matt Goo added 2 commits March 21, 2018 09:39

Merge branch 'master' into refactor/notched-outline/update-api

a630847

feat(notched-outline): added closeOutline method on notchedoutline fo…

0634366

…undation

lynnmercier approved these changes Mar 21, 2018

View reviewed changes

fix(notched-outline): updated readme and verbiage for open/close notch

c015302

moog16 merged commit 4f83757 into master Mar 21, 2018

moog16 deleted the refactor/notched-outline/update-api branch March 21, 2018 20:24

trimox mentioned this pull request Apr 8, 2018

feat(notched-outline): Add new foundation methods trimox/angular-mdc-web#836

Merged

trimox added a commit to trimox/angular-mdc-web that referenced this pull request Apr 9, 2018

feat(notched-outline): Add MDC Foundation methods (#836)

8b87e14

Change as per material-components-web v0.34.1. reference: material-components/material-components-web#2401

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(notched-outline): remove text-field notched outline styles and coupling #2401

refactor(notched-outline): remove text-field notched outline styles and coupling #2401

moog16 commented Mar 14, 2018

codecov-io commented Mar 16, 2018 •

edited

williamernest left a comment

williamernest Mar 19, 2018

williamernest Mar 20, 2018

williamernest Mar 20, 2018

moog16 Mar 20, 2018

lynnmercier Mar 21, 2018

lynnmercier Mar 21, 2018

lynnmercier Mar 21, 2018

lynnmercier Mar 21, 2018

lynnmercier Mar 21, 2018

refactor(notched-outline): remove text-field notched outline styles and coupling #2401

refactor(notched-outline): remove text-field notched outline styles and coupling #2401

Conversation

moog16 commented Mar 14, 2018

codecov-io commented Mar 16, 2018 • edited

Codecov Report

williamernest 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

Choose a reason for hiding this comment

codecov-io commented Mar 16, 2018 •

edited