Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

fix(floating-label): Update transition durations #2590

Merged
merged 5 commits into from
Apr 19, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/mdc-floating-label/_variables.scss
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@
//

$mdc-floating-label-position-y: 100%;
$mdc-floating-label-transition-duration: 150ms;
6 changes: 4 additions & 2 deletions packages/mdc-floating-label/mdc-floating-label.scss
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@
// limitations under the License.
//

@import "@material/animation/variables";
@import "@material/rtl/mixins";
@import "@material/theme/variables";
@import "@material/theme/mixins";
@import "@material/typography/mixins";
@import "./functions";
@import "./mixins";
@import "./variables";

Expand All @@ -30,7 +30,9 @@
bottom: 8px;
left: 0;
transform-origin: left top;
transition: mdc-floating-label-transition(transform), mdc-floating-label-transition(color);
transition:
transform $mdc-floating-label-transition-duration $mdc-animation-standard-curve-timing-function,
color $mdc-floating-label-transition-duration $mdc-animation-standard-curve-timing-function;
line-height: 1.15rem;
cursor: text;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//
// Copyright 2017 Google Inc. All Rights Reserved.
// Copyright 2018 Google Inc. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand All @@ -14,8 +14,4 @@
// limitations under the License.
//

@import "@material/animation/variables";

@function mdc-floating-label-transition($property) {
@return #{$property} 180ms $mdc-animation-standard-curve-timing-function;
}
$mdc-notched-outline-transition-duration: 150ms;
13 changes: 7 additions & 6 deletions packages/mdc-notched-outline/mdc-notched-outline.scss
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,21 @@
// limitations under the License.
//

@import "@material/animation/variables";
@import "@material/ripple/mixins";
@import "@material/ripple/variables";
@import "@material/textfield/functions";
@import "@material/theme/mixins";
@import "./mixins";
@import "./variables";

.mdc-notched-outline {
position: absolute;
top: 0;
left: 0;
width: calc(100% - 1px);
height: calc(100% - 2px);
transition: mdc-text-field-transition(opacity);
transition: opacity $mdc-notched-outline-transition-duration $mdc-animation-standard-curve-timing-function;
opacity: 0;
overflow: hidden;

Expand All @@ -45,19 +47,18 @@
left: 0;
width: calc(100% - 4px);
height: calc(100% - 4px);
transition: opacity 100ms ease;
transition: opacity $mdc-notched-outline-transition-duration $mdc-animation-standard-curve-timing-function,
border-color $mdc-notched-outline-transition-duration $mdc-animation-standard-curve-timing-function;
border: 1px solid;
opacity: 1;
}

.mdc-notched-outline__path {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if you want to change it here or another PR, but line 50 needs to be updated: change opacity 100ms ease to use the new duration variable and standard curve timing.
Also include a transition for border-color for the transition on hover.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the .mdc-notched-outline__idle selector (GitHub won't let me comment there)

Copy link
Contributor Author

@kfranqueiro kfranqueiro Apr 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hadn't touched this because I was suspecting it was intentionally set so that there isn't still visibly a line when the label text is moving into the gap.

100ms is significantly shorter than the other values of 180 / 260 elsewhere that I replaced, and is still shorter than the new value I'm replacing it with (150).

I can change it, but I'm not sure that changing the duration in particular is a good idea?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the branch with all of the suggested changes. Given the brevity of the overall duration, this seems like it might actually be fine (and even if I slow it down in dev tools it still doesn't seem like the half-faded line and the text don't end up intersecting too badly).

As much as I'd like to avoid transitioning on non-compositable properties, adding border-color fixes another issue I'd noticed, so thanks for pointing that out.

stroke-width: 1px;

// TODO(mattgoo): remove this text-field dep from notched-outline
transition:
mdc-text-field-transition(stroke),
mdc-text-field-transition(stroke-width),
mdc-text-field-transition(opacity);
stroke $mdc-notched-outline-transition-duration $mdc-animation-standard-curve-timing-function,
stroke-width $mdc-notched-outline-transition-duration $mdc-animation-standard-curve-timing-function;
fill: transparent;
}

Expand Down
1 change: 0 additions & 1 deletion packages/mdc-textfield/_mixins.scss
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,6 @@

position: absolute;
bottom: 20px;
transition: transform 260ms ease;
}

.mdc-text-field__icon {
Expand Down