feat(form-field): support for different spec variants #9366

mmalerba · 2018-01-13T00:01:07Z

Add support for variants and create 3 to start with: legacy (same as current master), standard (same as current master + a couple tweaks to be more in line with the spec), box (the box style from the spec).

Demos:
https://mmalerba-demo1.firebaseapp.com - current master
https://mmalerba-demo2.firebaseapp.com - default variant = legacy (should be identical to master)
https://mmalerba-demo3.firebaseapp.com - default variant = standard
https://mmalerba-demo4.firebaseapp.com - default variant = box

Couple notes:

legacy is the actual default I used, so nothing should change for anyone unless they explicitly set a variant.
I didn't add the background ripple shown in the box spec yet.
Non-floating labels really don't work well with the box variant because of how the label is aligned (see the "first name" input at https://mmalerba-demo4.firebaseapp.com/input). Should we allow it to look broken? Ignore the floatLabel="never" option?
Prefixes & suffixes look funky in the box variant. The reason is that we're using prefix/suffix for 2 different purposes:
1. Icons/buttons such as a password visibility toggle (e.g. https://storage.googleapis.com/material-design/publish/material_v_12/assets/0B5ZSepuCX1xOdEc4cHZWV3ZzTzg/password1.png)
2. Things like putting a '$' symbol in front of the user-inputted value. (e.g. https://storage.googleapis.com/material-design/publish/material_v_12/assets/0B5ZSepuCX1xOdVBQcERBbTJpSm8/prefix1.png)

I think (ii) is actually a misuse of the prefix/suffix. It should behave more like a placeholder that only appears once the label floats up. The spec provides more examples of this kind of thing here: https://material.io/guidelines/components/text-fields.html#text-fields-input-types. I think we should probably add support for formats to MatInput to handle this case rather than relying on the form-field's prefix/suffix. If you guys agree with this I'll go ahead and make the prefix/suffix behave like the icon case in the spec for standard and box and leave the legacy behavior as is.

googlebot · 2018-01-13T00:01:21Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

crisbeto

Some more notes:

Text inside the box variant looks fuzzy. I'm guessing it could be because of the perspective, because I'm not seeing it on IE. See https://i.imgur.com/6zg8Uz0.png
For the box variant + non-floating placeholder we could log a warning in dev mode that it's unsupported. You can use the MATERIAL_SANITY_CHECKS token so users are able to disable the warning.
I noticed a weird issue on Firefox where only the ellipsis inside an invalid field is white. It doesn't seem to be in master. See https://i.imgur.com/9T63rIb.png
Regarding prefixes and suffixes, I agree with assuming that they're icons. The text-based prefixes should be handled using a mask inside the input.

crisbeto · 2018-01-13T13:12:30Z

src/lib/form-field/form-field-box.scss

+@import '../core/style/vendor-prefixes';
+
+
+// Styles that only apply to the standard variant of the form-field.


Inaccurate comment.

crisbeto · 2018-01-13T13:14:59Z

src/lib/form-field/form-field-box.scss

+
+  .mat-form-field-label {
+    // The perspective helps smooth out animations on Chrome and Firefox but isn't needed on IE.
+    transform: translateY(-50%) perspective(100px);


The 100px seems arbitrary. Add a comment about where it comes from?

This was just trial and error to balance jumpiness and blurriness issues across different browsers. I'm going to try switching to filter and backface-visibility to fix some of these issues. It makes the blurriness better and improves the jumpiness in chrome, but makes firefox a little more jumpy. Overall I think its a little bit better solution

crisbeto · 2018-01-13T13:15:28Z

src/lib/form-field/form-field-box.scss

+  .mat-form-field-label {
+    // The perspective helps smooth out animations on Chrome and Firefox but isn't needed on IE.
+    transform: translateY(-50%) perspective(100px);
+    -ms-transform: translateY(-50%);


Does it get in the way of IE? If the outcome is the same we might as well keep the single declaration.

crisbeto · 2018-01-13T13:17:59Z

src/lib/form-field/form-field.ts

@@ -86,6 +95,9 @@ let nextUniqueId = 0;
 export class MatFormField implements AfterViewInit, AfterContentInit, AfterContentChecked {
  private _labelOptions: LabelOptions;

+  /** The form-field style variant. */
+  @Input() variant: 'legacy' | 'standard' | 'box' = 'legacy';


Turn this into a type so users can import it?

crisbeto · 2018-01-13T13:18:54Z

src/lib/form-field/form-field.ts

@@ -86,6 +95,9 @@ let nextUniqueId = 0;
 export class MatFormField implements AfterViewInit, AfterContentInit, AfterContentChecked {
  private _labelOptions: LabelOptions;

+  /** The form-field style variant. */
+  @Input() variant: 'legacy' | 'standard' | 'box' = 'legacy';
+


Sidenote: maybe we should add an injection token that allows you to specify the default variant?

mmalerba · 2018-01-16T20:08:52Z

I took a look at the firefox ellipsis issue, it looks like its caused by the combination of the transform text-overflow and filter properties. I'm not really sure what I can do about it...

crisbeto · 2018-01-17T18:17:47Z

src/lib/form-field/form-field-legacy.scss

+  }
+
+  .mat-form-field-label {
+    backface-visibility: initial;


IE doesn't support the initial value, we should have a lint rule for it as well. You can default it to visible instead.

jelbourn · 2018-01-17T19:40:21Z

I also see the blurriness in the box style

jelbourn · 2018-01-17T21:48:17Z

src/lib/form-field/_form-field-legacy-theme.scss

+
+  $underline-color: mat-color($foreground, divider, if($is-dark-theme, 0.7, 0.42));
+
+  .mat-form-field-variant-legacy {


This increases the specificity of these rules, so anyone with an override might find their styles no longer apply. We'll have to look through peoples' styles to gauge the extent of this.

Yeah if this isn't feasible we can add all the legacy styles to the base styles and then override the ones we don't want in standard, box, and outline

jelbourn · 2018-01-17T22:09:16Z

src/lib/form-field/form-field.scss

@@ -108,6 +108,9 @@ $mat-form-field-default-infix-width: 180px !default;
  // Hide the label initially, and only show it when it's floating or the control is empty.
  display: none;

+  // Helps prevent text jumpiness when transforming the label.


Can you provide any background on why this is so?

Changed some things so this is no longer needed

jelbourn · 2018-01-17T22:09:22Z

src/lib/form-field/form-field.scss

@@ -77,6 +78,9 @@ $mat-form-field-default-infix-width: 180px !default;
  height: 100%;
  overflow: hidden;
  pointer-events: none;  // We shouldn't catch mouse events (let them through).
+
+  // Helps prevent text jumpiness when transforming the label.


Can you provide any background on why this is so?

Changed some things so this is no longer needed

jelbourn · 2018-01-17T22:09:46Z

src/lib/form-field/form-field.scss

+    transform: scaleX(1);
+    transition: transform 300ms $swift-ease-out-timing-function,
+    opacity 100ms $swift-ease-out-timing-function,
+    background-color 300ms $swift-ease-out-timing-function;


Indent overflow lines?

jelbourn · 2018-01-17T22:13:12Z

src/lib/select/select.scss

@@ -100,5 +105,7 @@ $mat-select-placeholder-arrow-space: 2 * ($mat-select-arrow-size + $mat-select-a
    // Remove the transition to prevent the placeholder
    // from overlapping when the label comes back down.
    transition: none;
+    // Prevents the '...' from showing on the parent element.
+    display: block;


Wouldn't the parent element just not have overflow: ellipses?

The overflow: ellipses on the parent is also what gives the value its ... so we need to do it this way even though its a little strange

jelbourn · 2018-01-17T22:13:55Z

src/lib/select/select.scss

+
+  // When used in a box variant form-field the arrow should be centered in the box.
+  .mat-form-field-variant-box & {
+    transform: translateY(-50%);


Transforming by a percentage value like this can lead to blurriness due to subpixel rendering. Generally it's better to center via flex styles

jelbourn · 2018-01-17T22:14:31Z

src/lib/form-field/form-field.ts

@@ -86,6 +98,9 @@ let nextUniqueId = 0;
 export class MatFormField implements AfterViewInit, AfterContentInit, AfterContentChecked {
  private _labelOptions: LabelOptions;

+  /** The form-field style variant. */
+  @Input() variant: MatFormFieldVariant = 'legacy';


Not necessarily in this PR, but we'll also need a provider to set the default appearance as well

Yep, will add this later

mmalerba · 2018-01-17T23:06:15Z

I've updated all of the demos with the latest changes (you may need to clear your cache). I also sent an email to the material design team hoping that they can clarify the prefix/suffix thing for us.

mmalerba · 2018-01-18T20:03:53Z

I've updated the standard and box appearances so that they no longer promote the placeholder to a label and they ignore floatLabel="never" instead treating it like floatLabel="always". The legacy appearance still behaves like it does in current master.

Remaining issues I plan to address in future PRs:

How to treat prefix and suffix in the new appearances
Allowing user to set default appearance via a provider
Adding a background ripple for the box appearance

mmalerba · 2018-01-18T21:47:39Z

resolved the bazel failure, PTAL

[Based on the spec](https://material.io/guidelines/components/text-fields.html#text-fields-states) form fields should have a hover state where the underline gets darkened while the user is hovering and gets replaced by the theme color after the focus the input.

jelbourn

LGTM

* feat(form-field): implement hover state [Based on the spec](https://material.io/guidelines/components/text-fields.html#text-fields-states) form fields should have a hover state where the underline gets darkened while the user is hovering and gets replaced by the theme color after the focus the input. * branch css logic * extract underline css into standard variant * box variant underline * box variant spacing * add legacy variant * fix select ellipsis * move hover state changes out of legacy variant * add variants section to input demo * use filter/backface-visibility to address label jumpiness * address comments * fix box variant text fuzziness * remove bluriness fixes that aren't needed anymore * address comments * remove the floatLabel=never option in the new variants * variant --> appearance * add tests for new label & placeholder behavior * update demo to use mat-label * fix bazel build

…ter (#9762) * feat(form-field): support for different spec variants (#9366) * feat(form-field): implement hover state [Based on the spec](https://material.io/guidelines/components/text-fields.html#text-fields-states) form fields should have a hover state where the underline gets darkened while the user is hovering and gets replaced by the theme color after the focus the input. * branch css logic * extract underline css into standard variant * box variant underline * box variant spacing * add legacy variant * fix select ellipsis * move hover state changes out of legacy variant * add variants section to input demo * use filter/backface-visibility to address label jumpiness * address comments * fix box variant text fuzziness * remove bluriness fixes that aren't needed anymore * address comments * remove the floatLabel=never option in the new variants * variant --> appearance * add tests for new label & placeholder behavior * update demo to use mat-label * fix bazel build * feat(chips): Add chip avatar and chip trailing icon (#9557) * feat(chips): Add chip avatar and chip trailing icon * Removed MatBasicChip and MatStandardChip * Add mat-chip-trailing-icon style to MatChipRemove and add examples in demo * fix(form-field): rename box to fill and tweak the styles a bit (#9636) * make some tweaks to the box appearance * rename 'box' appearance to 'fill' * feat(form-field): add outline style (#9705) * remove datepicker reliance on form-field's underlineRef * add spacing and alignment rules for outline variant * outline color & thickness * style tweaks * correctly position and size the gap * address comments * fix(form-field): correct prefix & suffix icons as well as select arrow for various form field appearances (#9743) * feat(input): add utilities for custom styling and monitoring state of input autofill (#9719) * add utility for monitoring input autofill * add scss mixin for styling input autofill colors * tests * move everything from cdk to MatInputModule * address comments * add doc comments * fix(form-field): fixes for outline appearance (#9759) * use the `AutofillMonitor` in `MatInput` * Make `updateOutlineGap` public so users can call it if needed * feat(chips): add ripple to chips (#9761) * fix(form-field, chips): fix tests & lint (#9767) * undo change that caused darkened color for legacy form field * fix change detection * fix(chips): remove margin for chip list (#9793) * add terminateOnPointerUp to ripple config

talamaska · 2018-02-16T00:46:51Z

I don't get it. Why so drastic change in the behavior of the non floating label? I liked that feature and I'm using it in production. Can't we not place that mat-label and have a placeholder like thing, but with the required asterisk, you can't put a colored asterisk in the placeholder attribute normally. With that drastic change I'll be forced to stick to 5.2.1. So is it possible to make 1 more component mat-placeholder to act like a placeholder, but have a required mark and be possible to not use mat-label for the purpose of a placeholder

mmalerba · 2018-02-16T03:53:20Z

@talamaska I've upped the priority on #7645 for adding text mask support to the input. I'm not 100% sure our text masking solution will be able to cover this use case, but I'll keep it in mind while designing.

The never-float placeholder stuff was really just an artifact of the fact that the spec didn't used to distinguish between label and placeholder and we therefore needed some way to emulate standard placeholder behavior with the floating label. The spec is now much more clear about the fact that the label and placeholder are two different things and the required * really belongs with the label

talamaska · 2018-02-20T22:49:14Z

Having a required mark in the placeholder doen't have anything to do with the text masking. It is a totaly separated feature. I see the spec have changed and you think it was wrong to provide that feature, which is adopted by a lot of people. The reality is that forms with just placeholders and without labels have been designed and used long before any of angular or material existed, and will exist in the future so basically by making this decision you again deliver a breaking change, something that happens too often for a mature framework. Maybe that's the point? To make people not count on any of your products and do their own ui framework? Probably this is something I'm going to do - fork the material inputs and hope they will work in future angular versions.

angular-automatic-lock-bot · 2019-09-09T08:03:24Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

mmalerba added the pr: needs review label Jan 13, 2018

mmalerba assigned jelbourn and crisbeto Jan 13, 2018

mmalerba requested review from jelbourn and crisbeto January 13, 2018 00:01

mmalerba requested a review from kara as a code owner January 13, 2018 00:01

googlebot added the cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla label Jan 13, 2018

mmalerba mentioned this pull request Jan 13, 2018

feat(form-field): implement hover state #9215

Closed

mmalerba added cla: yes PR author has agreed to Google's Contributor License Agreement and removed cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla labels Jan 13, 2018

crisbeto reviewed Jan 13, 2018

View reviewed changes

crisbeto reviewed Jan 17, 2018

View reviewed changes

jelbourn reviewed Jan 17, 2018

View reviewed changes

mmalerba force-pushed the ff-var branch from 67e9308 to a9a1b12 Compare January 17, 2018 22:30

mmalerba requested review from andrewseguin, devversion, josephperrott and tinayuangao as code owners January 18, 2018 19:44

mmalerba force-pushed the ff-var branch from 1ac5dc0 to f865dd3 Compare January 18, 2018 20:16

mmalerba force-pushed the ff-var branch from 7b1dffe to a970692 Compare January 19, 2018 22:01

crisbeto mentioned this pull request Jan 20, 2018

MatInput with MatPrefix does not conform to the Material spec for icons and text #9488

Closed

mmalerba changed the base branch from master to input-chip January 22, 2018 23:42

crisbeto and others added 2 commits January 22, 2018 15:44

branch css logic

19feeb0

mmalerba added 11 commits January 22, 2018 15:44

add variants section to input demo

024a070

use filter/backface-visibility to address label jumpiness

af21801

address comments

ec76aa5

fix box variant text fuzziness

bef4ded

remove bluriness fixes that aren't needed anymore

d42ea81

address comments

a1deead

remove the floatLabel=never option in the new variants

f81123f

variant --> appearance

72d74b8

add tests for new label & placeholder behavior

b29ea34

update demo to use mat-label

b864285

fix bazel build

43b00d8

mmalerba force-pushed the ff-var branch from a970692 to 43b00d8 Compare January 22, 2018 23:57

jelbourn approved these changes Jan 25, 2018

View reviewed changes

jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels Jan 25, 2018

mmalerba merged commit 826ddb0 into angular:input-chip Jan 26, 2018

jelbourn added the target: minor This PR is targeted for the next minor release label Jan 29, 2018

EdricChan03 mentioned this pull request May 7, 2018

Text field boxes are missing ripple effects #11199

Closed

mmalerba deleted the ff-var branch July 31, 2018 21:56

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 9, 2019

		@import '../core/style/vendor-prefixes';


		// Styles that only apply to the standard variant of the form-field.


		$underline-color: mat-color($foreground, divider, if($is-dark-theme, 0.7, 0.42));

		.mat-form-field-variant-legacy {

feat(form-field): support for different spec variants #9366

feat(form-field): support for different spec variants #9366

Uh oh!

Conversation

mmalerba commented Jan 13, 2018

Uh oh!

googlebot commented Jan 13, 2018

Uh oh!

crisbeto left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmalerba commented Jan 16, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jelbourn commented Jan 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmalerba commented Jan 17, 2018

Uh oh!

mmalerba commented Jan 18, 2018

Uh oh!

mmalerba commented Jan 18, 2018

Uh oh!

jelbourn left a comment

Choose a reason for hiding this comment

Uh oh!

talamaska commented Feb 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmalerba commented Feb 16, 2018

Uh oh!

talamaska commented Feb 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

angular-automatic-lock-bot bot commented Sep 9, 2019

Uh oh!

Uh oh!

talamaska commented Feb 16, 2018 •

edited

Loading

talamaska commented Feb 20, 2018 •

edited

Loading