fix(select): Arrow position/animation for appearance="standard" #12045

seritools · 2018-07-03T18:34:21Z

This commit aims to fix the arrow positioning of mat-selects
with appearance="standard". Since the label moves down whenever
the selected value is empty, the arrow looks out of place.
I added an animation that matches the speed of the label
animation:

Going from filled to empty transitions the arrow down so that it
sits level with the label.
Going from empty to filled snaps the arrow back up, matching the
label's behavior.

To test/compare the different appearances I added another card
to the demo-app with all of them.

Fixes #11925.

crisbeto · 2018-07-04T20:45:40Z

src/demo-app/select/select-demo.html

@@ -144,6 +144,59 @@
    </mat-card-content>
  </mat-card>

+  <mat-card class=".demo-appearance-comparison">


The dot in the class name looks like it shouldn't be there. Also it doesn't look like this class is being used anywhere.

crisbeto · 2018-07-04T20:48:32Z

src/lib/select/select-animations.ts

+  /**
+   * This animation moves the panel toggle arrow up whenever the select is not empty to match the
+   * visual middle. This affects only mat-selects with appearance='standard'.
+   */


Defining an animation just to move the arrow around is overkill and it makes it very hard for people to override the styles. Since the empty state is on the mat-select already, you can use it to toggle a class either on the select or the arrow itself which tells it where it's supposed to be positioned.

Using a normal CSS transition will make it easy to target the specific appearance as well, since the form field already has a class indicating the variant.

You can hook into the Angular Animations to disable this transition when Angular Animations are disabled. As an example see here: https://github.com/angular/material2/blob/master/src/lib/form-field/form-field.scss#L220

Yeah, using just simple CSS seems way better and easier to handle - I'm gonna update the PR on the weekend!

mmalerba · 2018-07-06T17:16:20Z

src/demo-app/select/select-demo.html

@@ -144,6 +144,59 @@
    </mat-card-content>
  </mat-card>

+  <mat-card class=".demo-appearance-comparison">


Can you add a button to this demo that allows me to toggle all of the selects to either have a value or be cleared? It's tough to see the animation when manually selecting the value

Sure, will do!

mmalerba · 2018-07-06T17:21:56Z

src/lib/select/select-animations.ts

+  /**
+   * This animation moves the panel toggle arrow up whenever the select is not empty to match the
+   * visual middle. This affects only mat-selects with appearance='standard'.
+   */


Using a normal CSS transition will make it easy to target the specific appearance as well, since the form field already has a class indicating the variant.

You can hook into the Angular Animations to disable this transition when Angular Animations are disabled. As an example see here: https://github.com/angular/material2/blob/master/src/lib/form-field/form-field.scss#L220

seritools · 2018-07-08T11:29:48Z

Alright! Switched over to a simple css animation and added a toggle button to the select-demo to quickly see the effect.

mmalerba

I think this feature looks good and is limited-scope enough to not interfere with adherence to the spec in other situations. Thanks for working on it!

mmalerba · 2018-07-09T22:58:43Z

src/lib/select/select.ts

@@ -202,6 +202,7 @@ export class MatSelectTrigger {}
    '[class.mat-select-disabled]': 'disabled',
    '[class.mat-select-invalid]': 'errorState',
    '[class.mat-select-required]': 'required',
+    '[class.mat-select-empty]': 'empty',


iirc, form-field has an empty class that you can probably use instead of adding a new one here

Hey, just checked this out, but the .mat-form-field-empty is only set on the floating label, not the mat-form-field itself. What would be the best change @mmalerba?

Oh ok, in that case we can just leave it as is

ngbot · 2018-07-16T15:45:58Z

I see that you just added the pr: merge ready label, but the following checks are still failing:
status "ci/circleci: build" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

…lar#11925) This commit aims to fix the arrow positioning of mat-selects with appearance="standard". Since the label moves down whenever the selected value is empty, the arrow looks out of place. I added an animation that matches the speed of the label animation: Going from filled to empty transitions the arrow down so that it sits level with the label. Going from empty to filled snaps the arrow back up, matching the label's behavior. To test/compare the different appearances I added another card to the demo-app with all of them.

seritools · 2018-07-21T12:00:05Z

Heh, rebased onto current master again to trigger a new CI run, now circleci passes but travis failed. Could you look into it once again @mmalerba?

mmalerba · 2018-07-24T17:31:15Z

failure looks like a flake, I restarted it

angular-automatic-lock-bot · 2019-09-09T12:01:40Z

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.}

seritools requested a review from crisbeto as a code owner July 3, 2018 18:34

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jul 3, 2018

seritools changed the title ~~fix(select): Arrow position/animation for appearance="standard" (#11925)~~ fix(select): Arrow position/animation for appearance="standard" Jul 3, 2018

crisbeto reviewed Jul 4, 2018

View reviewed changes

mmalerba self-assigned this Jul 6, 2018

mmalerba reviewed Jul 6, 2018

View reviewed changes

mmalerba reviewed Jul 9, 2018

View reviewed changes

mmalerba approved these changes Jul 16, 2018

View reviewed changes

mmalerba added pr: lgtm action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release labels Jul 16, 2018

seritools added 2 commits July 21, 2018 12:51

select-arrow: Replaced angular animation with simple css animation

4df3243

seritools force-pushed the select-standard-arrow-fix branch from 6362302 to 4df3243 Compare July 21, 2018 10:51

jelbourn merged commit 58f3c54 into angular:master Aug 23, 2018

ghost mentioned this pull request Dec 5, 2018

Mat-Select with appearance="standard": Arrow too high #11925

Closed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(select): Arrow position/animation for appearance="standard" #12045

fix(select): Arrow position/animation for appearance="standard" #12045

seritools commented Jul 3, 2018 •

edited

Loading

crisbeto Jul 4, 2018

crisbeto Jul 4, 2018

mmalerba Jul 6, 2018

seritools Jul 6, 2018

mmalerba Jul 6, 2018

seritools Jul 6, 2018

mmalerba Jul 6, 2018

seritools commented Jul 8, 2018

mmalerba left a comment

mmalerba Jul 9, 2018

seritools Jul 15, 2018

mmalerba Jul 16, 2018

ngbot bot commented Jul 16, 2018

seritools commented Jul 21, 2018

mmalerba commented Jul 24, 2018

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

fix(select): Arrow position/animation for appearance="standard" #12045

fix(select): Arrow position/animation for appearance="standard" #12045

Conversation

seritools commented Jul 3, 2018 • edited Loading

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

seritools commented Jul 8, 2018

mmalerba 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

ngbot bot commented Jul 16, 2018

seritools commented Jul 21, 2018

mmalerba commented Jul 24, 2018

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

seritools commented Jul 3, 2018 •

edited

Loading