feat(text-field): Move bottom-line to separate package #2037

williamernest · 2018-01-23T00:46:56Z

BREAKING CHANGE: Moves the text-field bottom-line element to a new package, so we can reuse it in other components. The HTML class name for the bottom-line element has changed from mdc-text-field__bottom-line to mdc-bottom-line.
refs: #1981

…ge.json

….json

codecov-io · 2018-01-23T02:08:38Z

Codecov Report

Merging #2037 into master will decrease coverage by 0.07%.
The diff coverage is 94.23%.

@@            Coverage Diff             @@
##           master    #2037      +/-   ##
==========================================
- Coverage   99.33%   99.25%   -0.08%     
==========================================
  Files          84       84              
  Lines        3769     3768       -1     
  Branches      490      489       -1     
==========================================
- Hits         3744     3740       -4     
- Misses         25       28       +3

Impacted Files	Coverage Δ
packages/mdc-textfield/constants.js	`100% <ø> (ø)`	⬆️
packages/mdc-line-ripple/constants.js	`100% <ø> (ø)`
packages/mdc-line-ripple/index.js	`100% <100%> (ø)`
packages/mdc-textfield/foundation.js	`98.75% <100%> (-0.04%)`	⬇️
packages/mdc-textfield/index.js	`96.49% <100%> (+0.06%)`	⬆️
packages/mdc-line-ripple/foundation.js	`90.32% <76.92%> (ø)`

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 4f4e36b...edde073. Read the comment docs.

lynnmercier · 2018-01-24T19:19:44Z

packages/material-components-web/package.json

@@ -16,6 +16,7 @@
    "@material/animation": "^0.25.0",
    "@material/auto-init": "^0.29.0",
    "@material/base": "^0.29.0",
+    "@material/bottom-line": "^0.29.0",


I think you might need to create a 0.0.0 version first? And then let lerna upgrade it for you when we release? Check with @kfranqueiro

First commit of selection-control, for reference: da9c7a9

lynnmercier · 2018-01-24T19:20:06Z

packages/mdc-bottom-line/README.md

@@ -9,7 +9,7 @@ path: /catalog/input-controls/text-field/bottom-line/

 # Text Field Bottom Line


Remove "Text Field" from this

And you'll want to change the docs comment above to something like



(I think you can just drop the iconId, since we don't have an icon for this)

lynnmercier · 2018-01-24T19:25:23Z

packages/mdc-bottom-line/README.md


-##### `MDCTextFieldBottomLine.foundation`


I think, since we are making this a public component, we should not make the foundation a field of MDCBottomLine anymore. I think we should create proxy methods on MDCBottomLine which proxy to MDCBottomLineFoundation's activate, deactivate and setTransformOrigin

When it was internal only to text-field, it was easier not to create proxy methods. But I think with this becoming a public component we should protect access to the foundation class.

Of course, you'll also have to change the adapter for Text Field. I think Select has a good example of this here:
https://github.com/material-components/material-components-web/blob/master/packages/mdc-select/index.js#L112

It shows how the Select foundation interacts with a Menu through the Select adapter. I think we need to add methods like activateBottomLine to the Text Field adapter.

lynnmercier · 2018-01-24T19:26:56Z

packages/mdc-bottom-line/README.md


 Method Signature | Description
 --- | ---
 `activate() => void` | Activates the bottom line
-`deactivate => void` | Deactivates the bottom line
+`deactivate() => void` | Immediately deactivates the bottom line
+`deactivateFocus() => void` | Sets `isActive_` flag to false, allowing the animation to finish before deactivating. 


this description is an internal implementation detail. Be more general about what deactivateFocus is trying to accomplish. Do we still need deactivate? Shouldn't we always wait for the animation to finish before deactivating?

We could remove the deactivateFocus field and combine it's functionality into deactivate

lynnmercier · 2018-01-24T19:27:42Z

packages/mdc-bottom-line/_functions.scss

+// limitations under the License.
+//
+
+@function mdc-bottom-line-transition($property) {


transition-value...based of what we did for mdc-elevation

lynnmercier · 2018-01-24T19:32:43Z

packages/mdc-bottom-line/adapter.js

@@ -21,13 +21,13 @@
 * Adapter for MDC TextField Bottom Line.
 *
 * Defines the shape of the adapter expected by the foundation. Implement this
- * adapter to integrate the TextField bottom line into your framework. See
+ * adapter to integrate the TextField or Select bottom line into your framework. See


drop "the TextField or Select" and just go with bottom line

lynnmercier · 2018-01-24T19:33:49Z

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


-  &__bottom-line--active {
+  &--active {


drop the & syntax here

lynnmercier · 2018-01-24T19:34:29Z

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

+  width: 100%;
+  height: 2px;
+  transform: scaleX(0);
+  transition: mdc-text-field-transition(transform), mdc-text-field-transition(opacity);


we shouldnt have any references to text-field in this file. Why is mdc-text-field-transition here?

lynnmercier · 2018-01-24T19:34:55Z

packages/mdc-select/mdc-select.scss

@@ -22,6 +22,7 @@
 @import "@material/ripple/common";
 @import "@material/ripple/mixins";
 @import "@material/rtl/mixins";
+@import "@material/list/mixins";


this should not be in this PR

lynnmercier · 2018-01-24T19:36:04Z

packages/mdc-textfield/_mixins.scss

@@ -156,7 +167,7 @@
 @mixin mdc-text-field-invalid_ {
  @include mdc-text-field-bottom-line-color($mdc-text-field-error-on-light);
  @include mdc-text-field-hover-bottom-line-color($mdc-text-field-error-on-light);
-  @include mdc-text-field-focused-bottom-line-color($mdc-text-field-error-on-light);
+  @include mdc-bottom-line-color($mdc-text-field-error-on-light);


You're calling both mdc-text-field-bottom-line-color and mdc-bottom-line-color...Can you change mdc-text-field-bottom-line-color to call mdc-bottom-line-color internally?

@mixin mdc-text-field-bottom-line-color_($color) { @include mdc-bottom-line-color($color); .mdc-text-field__input { @include mdc-theme-prop(border-bottom-color, $color); } }

No, mdc-bottom-line-color and mdc-text-field-bottom-line-color are two different colors. The latter is referring to the idle state of the input bottom-border-color property.

We should probably change mdc-text-field-bottom-line-color to be more along the lines of mdc-text-field-bottom-border-color to help differentiate between the two.

lynnmercier · 2018-01-30T00:38:13Z

package-lock.json

@@ -12738,32 +12738,6 @@
        }
      }
    },
-    "stylelint-scss": {


This does not belong in this PR

This is from my merge with master.

lynnmercier · 2018-01-30T00:38:34Z

packages/material-components-web/index.js

@@ -14,29 +14,29 @@
 * limitations under the License.
 */

-import autoInit from '@material/auto-init';


This looks like a broken change in this PR

This is from my merge with master.

lynnmercier · 2018-01-30T00:41:25Z

packages/mdc-line-ripple/README.md

-excerpt: "The bottom line indicates where to enter text, displayed below the label"
-iconId: text_field
-path: /catalog/input-controls/text-field/bottom-line/
+excerpt: "The line ripple is used to indicate certain information, such as a line of text or a selected element"


The user specified text is highlighted above the line ripple.

How about that?

lynnmercier · 2018-01-30T00:41:49Z

packages/mdc-line-ripple/README.md


-The bottom line indicates the selected element or text, displayed below the label. When a text field/select is active, the line’s color and thickness vary.
+The line ripple is used to indicate the selected element or text. When a the line ripple is active, the line’s color and thickness vary.


Update this description too.

lynnmercier · 2018-01-30T00:45:00Z

packages/mdc-line-ripple/foundation.js

  }

  /**
   * Sets the transform origin given a user's click location.
-   * @param {!Event} evt
+   * @param {!number} normalizedX


maybe add some more description here about what "normalized" means

lynnmercier · 2018-01-30T00:47:03Z

packages/mdc-line-ripple/index.js

      setAttr: (attr, value) => this.root_.setAttribute(attr, value),
      registerEventHandler: (evtType, handler) => this.root_.addEventListener(evtType, handler),
      deregisterEventHandler: (evtType, handler) => this.root_.removeEventListener(evtType, handler),
      notifyAnimationEnd: () => {
-        this.emit(MDCBottomLineFoundation.strings.ANIMATION_END_EVENT, {});
+        this.emit(MDCLineRippleFoundation.strings.ANIMATION_END_EVENT, {});


Dont think we need this anymore, since Text Field isn't listening to it. So lets remove it.

lynnmercier · 2018-01-30T00:47:52Z

packages/mdc-select/foundation.js

@@ -251,7 +251,7 @@ export default class MDCSelectFoundation extends MDCFoundation {
  close_() {
    const {OPEN} = MDCSelectFoundation.cssClasses;
    this.adapter_.removeClass(OPEN);
-    this.adapter_.removeClassFromBottomLine(cssClasses.BOTTOM_LINE_ACTIVE);
+    this.adapter_.removeClassFromBottomLine(cssClasses.LINE_RIPPLE_ACTIVE);


This snuck into this PR I think

lynnmercier · 2018-01-30T00:47:56Z

packages/mdc-select/mdc-select.scss

@@ -21,7 +21,6 @@
 @import "@material/ripple/common";
 @import "@material/ripple/mixins";
 @import "@material/rtl/mixins";
-@import "@material/list/mixins";


This snuck into this PR I think

I deleted this, because I accidentally added it in one of my previous commits.

lynnmercier · 2018-01-30T00:48:26Z

packages/mdc-textfield/README.md

@@ -224,8 +224,8 @@ Method Signature | Description
 `deregisterTextFieldInteractionHandler(evtType: string, handler: EventListener)` => void | Deregisters an event handler on the root element for a given event
 `registerInputInteractionHandler(evtType: string, handler: EventListener)` => void | Registers an event listener on the native input element for a given event
 `deregisterInputInteractionHandler(evtType: string, handler: EventListener)` => void | Deregisters an event listener on the native input element for a given event
-`registerBottomLineEventHandler(evtType: string, handler: EventListener)` => void | Registers an event listener on the bottom line element for a given event
-`deregisterBottomLineEventHandler(evtType: string, handler: EventListener)` => void | Deregisters an event listener on the bottom line element for a given event
+`registerLineRippleEventHandler(evtType: string, handler: EventListener)` => void | Proxies to MDCLineRipple to register an event listener on the line ripple element for a given event


You can remove this too

…er methods.

…ckage

lynnmercier · 2018-01-31T21:42:34Z

packages/mdc-line-ripple/foundation.js

@@ -80,11 +80,11 @@ class MDCLineRippleFoundation extends MDCFoundation {

  /**
   * Sets the transform origin given a user's click location.


I think this should be: setRippleCenter(xCoordinate)

Because really "transform-origin" is an internal implementation detail to line-ripple CSS.

And the description should be: "Sets the center of the ripple animation to the given X coordinate"

...or something in that direction anyway.

lynnmercier · 2018-01-31T21:45:22Z

packages/mdc-line-ripple/index.js

      setAttr: (attr, value) => this.root_.setAttribute(attr, value),
      registerEventHandler: (evtType, handler) => this.root_.addEventListener(evtType, handler),
      deregisterEventHandler: (evtType, handler) => this.root_.removeEventListener(evtType, handler),
      notifyAnimationEnd: () => {
-        this.emit(MDCTextFieldBottomLineFoundation.strings.ANIMATION_END_EVENT, {});
+        this.emit(MDCLineRippleFoundation.strings.ANIMATION_END_EVENT, {});


I thought you were going to remove this ANIMATION_END_EVENT? It doesn't seem necessary.

lynnmercier

LGTM

lynnmercier · 2018-01-31T22:56:19Z

packages/mdc-line-ripple/README.md

@@ -50,7 +50,7 @@ CSS Class | Description

 `activate() => void` | Proxies to the foundations `activate()` method.
 `deactivate() => void` |  Proxies to the foundations `deactivate()` method.
-`setTransformOrigin(evt: Event) => void` | Proxies to the foundations `setTransformOrigin(evt: Event)` method.
+`setRippleCenter(evt: Event) => void` | Proxies to the foundations `setRippleCenter(evt: Event)` method.


number: xCoordinate (not evt:Event)

lynnmercier · 2018-01-31T22:56:26Z

packages/mdc-line-ripple/README.md


 ### `MDCLineRippleFoundation`

 Method Signature | Description
 --- | ---
 `activate() => void` | Activates the line ripple
 `deactivate() => void` |  Deactivates the line ripple
-`setTransformOrigin(evt: Event) => void` | Sets the transform origin given a user's click location
+`setRippleCenter(evt: Event) => void` | Sets the transform origin given a user's click location


number: xCoordinate (not evt:Event)

…ckage

williamernest added 4 commits January 22, 2018 15:30

WIP: Move bottom-line to separate package

e0677e8

feat(text-field): Update bottom-line transform. Add package to pacaka…

c7a66ec

…ge.json

feat(text-field): Move bottom line focus element to mixin

ad2c9e1

feat(text-field): Update tests. Update webpack config. Update package…

3bab8f6

….json

bonniezhou self-assigned this Jan 23, 2018

lynnmercier suggested changes Jan 24, 2018

View reviewed changes

lynnmercier assigned lynnmercier and unassigned bonniezhou Jan 25, 2018

williamernest added 2 commits January 26, 2018 10:37

WIP: Rename bottom-line to line-ripple

23bb243

WIP: Merge master

78a8286

williamernest force-pushed the feat/text-field/move-bottom-line-to-own-package branch from 39936e7 to 78a8286 Compare January 26, 2018 22:59

williamernest added 3 commits January 26, 2018 15:01

feat(text-field): Update tests

105ef89

feat(text-field): Merge with master.

357d29a

feat(text-field): Merge with master

a2c0636

lynnmercier suggested changes Jan 30, 2018

View reviewed changes

williamernest and others added 5 commits January 29, 2018 18:13

feat(text-field): Update for comments. Remove unused text-field adapt…

63d5eae

…er methods.

Merge branch 'master' into feat/text-field/move-bottom-line-to-own-pa…

07d6623

…ckage

feat(text-field): Undo changes to package-lock.json

9865bba

feat(text-field): Merge with master

4a7180f

Merge branch 'master' into feat/text-field/move-bottom-line-to-own-pa…

32368ee

…ckage

lynnmercier suggested changes Jan 31, 2018

View reviewed changes

feat(text-field): Update for comments. Remove event from line ripple

1f53574

lynnmercier approved these changes Jan 31, 2018

View reviewed changes

williamernest and others added 2 commits January 31, 2018 15:05

feat(text-field): Update readme/parameter names

5af8d74

Merge branch 'master' into feat/text-field/move-bottom-line-to-own-pa…

edde073

…ckage

williamernest merged commit 1dc0e85 into master Jan 31, 2018

williamernest deleted the feat/text-field/move-bottom-line-to-own-package branch January 31, 2018 23:45

trimox mentioned this pull request Feb 7, 2018

Text Field: Move MdcTextFieldBottomLine to separate package trimox/angular-mdc-web#620

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(text-field): Move bottom-line to separate package #2037

feat(text-field): Move bottom-line to separate package #2037

williamernest commented Jan 23, 2018

codecov-io commented Jan 23, 2018 •

edited

Loading

lynnmercier Jan 24, 2018

kfranqueiro Jan 24, 2018

lynnmercier Jan 24, 2018

lynnmercier Jan 24, 2018

lynnmercier Jan 24, 2018

williamernest Jan 25, 2018

lynnmercier Jan 24, 2018

lynnmercier Jan 24, 2018

lynnmercier Jan 24, 2018

lynnmercier Jan 24, 2018

lynnmercier Jan 24, 2018

lynnmercier Jan 24, 2018

williamernest Jan 25, 2018

lynnmercier Jan 30, 2018

williamernest Jan 30, 2018

lynnmercier Jan 30, 2018

williamernest Jan 30, 2018

lynnmercier Jan 30, 2018

lynnmercier Jan 30, 2018

lynnmercier Jan 30, 2018

lynnmercier Jan 30, 2018

lynnmercier Jan 30, 2018

lynnmercier Jan 30, 2018

williamernest Jan 30, 2018

lynnmercier Jan 30, 2018

lynnmercier Jan 31, 2018

lynnmercier Jan 31, 2018

lynnmercier left a comment

lynnmercier Jan 31, 2018

lynnmercier Jan 31, 2018

		@@ -9,7 +9,7 @@ path: /catalog/input-controls/text-field/bottom-line/

		# Text Field Bottom Line


		The bottom line indicates the selected element or text, displayed below the label. When a text field/select is active, the line’s color and thickness vary.
		The line ripple is used to indicate the selected element or text. When a the line ripple is active, the line’s color and thickness vary.

		@@ -80,11 +80,11 @@ class MDCLineRippleFoundation extends MDCFoundation {

		/**
		* Sets the transform origin given a user's click location.


		##### `MDCTextFieldBottomLine.foundation`

feat(text-field): Move bottom-line to separate package #2037

feat(text-field): Move bottom-line to separate package #2037

Conversation

williamernest commented Jan 23, 2018

codecov-io commented Jan 23, 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

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

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

Choose a reason for hiding this comment

codecov-io commented Jan 23, 2018 •

edited

Loading