Commit b94fc330 authored by Esmael El-Moslimany's avatar Esmael El-Moslimany Committed by Commit Bot

WebUI: cr-slider, remove position transitions for knob, bar and label

The position transitions cause a couple issues.

1. When the slider value is set after being retrieved asynchronously
even if the retrieval is quick, the movement will transition from 0
to the retrieved value. This is a minor issue noted in
https://crbug.com/896625.

2. Multiple arrow keyboard events, including repetition from holding
down and arrow key, results in a delay in updating the position causing
erratic movement of the knob.

Bug: 910054
Change-Id: Ibc98638f1c9bda6531a8dea2648654e41a91e37f
Reviewed-on: https://chromium-review.googlesource.com/c/1355760
Commit-Queue: Esmael El-Moslimany <aee@chromium.org>
Reviewed-by: default avatarDemetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612508}
parent e87ed71f
...@@ -316,4 +316,52 @@ suite('cr-slider', function() { ...@@ -316,4 +316,52 @@ suite('cr-slider', function() {
assertEquals(50, crSlider.value); assertEquals(50, crSlider.value);
}); });
}); });
test('smooth position transition only on pointerdown', () => {
const assertNoTransition = () => {
const expected = 'all 0s ease 0s';
assertEquals(expected, getComputedStyle(crSlider.$.knob).transition);
assertEquals(expected, getComputedStyle(crSlider.$.bar).transition);
assertEquals(expected, getComputedStyle(crSlider.$.label).transition);
};
const assertTransition = () => {
const getValue = propName => `${propName} 0.08s ease 0s`;
assertEquals(
getValue('margin-inline-start'),
getComputedStyle(crSlider.$.knob).transition);
assertEquals(
getValue('width'), getComputedStyle(crSlider.$.bar).transition);
assertEquals(
getValue('margin-inline-start'),
getComputedStyle(crSlider.$.label).transition);
};
assertNoTransition();
pointerDown(.5);
assertTransition();
return test_util.eventToPromise('transitionend', crSlider.$.knob)
.then(() => {
assertNoTransition();
// Other operations that change the value do not have transitions.
pointerMove(0);
assertNoTransition();
assertEquals(0, crSlider.value);
pointerUp();
pressArrowRight();
assertNoTransition();
assertEquals(1, crSlider.value);
crSlider.value = 50;
assertNoTransition();
// Check that the slider is not stuck with a transition when the value
// does not change.
crSlider.value = 0;
pointerDown(0);
assertTransition();
return test_util.eventToPromise('transitionend', crSlider.$.knob);
})
.then(() => {
assertNoTransition();
});
});
}); });
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
<style include="cr-hidden-style"> <style include="cr-hidden-style">
:host { :host {
--cr-slider-bar-height: 2px; --cr-slider-bar-height: 2px;
--cr-slider-position-transition: 80ms ease;
-webkit-tap-highlight-color: rgba(0, 0, 0, 0); -webkit-tap-highlight-color: rgba(0, 0, 0, 0);
cursor: default; cursor: default;
user-select: none; user-select: none;
...@@ -52,10 +53,13 @@ ...@@ -52,10 +53,13 @@
border-top-color: var(--cr-slider-active-color, var(--google-blue-600)); border-top-color: var(--cr-slider-active-color, var(--google-blue-600));
left: 0; left: 0;
position: absolute; position: absolute;
transition: width 80ms ease;
width: 0; width: 0;
} }
:host([transiting_]) #bar {
transition: width var(--cr-slider-position-transition);
}
:host([is-rtl_]) #bar { :host([is-rtl_]) #bar {
left: initial; left: initial;
right: 0; right: 0;
...@@ -77,10 +81,13 @@ ...@@ -77,10 +81,13 @@
margin-inline-start: 0; margin-inline-start: 0;
outline: none; outline: none;
position: absolute; position: absolute;
transition: margin-inline-start 80ms ease;
width: 10px; width: 10px;
} }
:host([transiting_]) #knob {
transition: margin-inline-start var(--cr-slider-position-transition);
}
paper-ripple { paper-ripple {
color: var(--google-blue-600); color: var(--google-blue-600);
height: 32px; height: 32px;
...@@ -154,10 +161,13 @@ ...@@ -154,10 +161,13 @@
line-height: 1.5em; line-height: 1.5em;
padding: 0 8px; padding: 0 8px;
position: absolute; position: absolute;
transition: margin-inline-start 80ms ease;
white-space: nowrap; white-space: nowrap;
} }
:host([transiting_]) #label {
transition: margin-inline-start var(--cr-slider-position-transition);
}
:host([disabled_]) { :host([disabled_]) {
pointer-events: none; pointer-events: none;
} }
...@@ -196,7 +206,8 @@ ...@@ -196,7 +206,8 @@
</div> </div>
</div> </div>
<div id="knobContainer"> <div id="knobContainer">
<div id="knob" tabindex="0"></div> <div id="knob" tabindex="0" on-transitionend="onKnobTransitionEnd_">
</div>
</div> </div>
<div id="labelContainer" aria-label="[[label_]]"> <div id="labelContainer" aria-label="[[label_]]">
<div id="label">[[label_]]</div> <div id="label">[[label_]]</div>
......
...@@ -153,6 +153,20 @@ cr_slider.SliderTick; ...@@ -153,6 +153,20 @@ cr_slider.SliderTick;
value: false, value: false,
reflectToAttribute: true, reflectToAttribute: true,
}, },
/**
* |transiting_| is set to true when bar is touched or clicked. This
* triggers a single position transition effect to take place for the
* knob, bar and label. When the transition is complete, |transiting_| is
* set to false resulting in no transition effect during dragging, manual
* value updates and keyboard events.
* @private
*/
transiting_: {
type: Boolean,
value: false,
reflectToAttribute: true,
},
}, },
hostAttributes: { hostAttributes: {
...@@ -317,6 +331,11 @@ cr_slider.SliderTick; ...@@ -317,6 +331,11 @@ cr_slider.SliderTick;
} }
}, },
/** @private */
onKnobTransitionEnd_: function() {
this.transiting_ = false;
},
/** /**
* When the left-mouse button is pressed, the knob location is updated and * When the left-mouse button is pressed, the knob location is updated and
* dragging starts. * dragging starts.
...@@ -328,6 +347,7 @@ cr_slider.SliderTick; ...@@ -328,6 +347,7 @@ cr_slider.SliderTick;
return; return;
this.dragging = true; this.dragging = true;
this.transiting_ = true;
this.updateValueFromClientX_(event.clientX); this.updateValueFromClientX_(event.clientX);
// If there is a ripple animation in progress, setTimeout will hold off on // If there is a ripple animation in progress, setTimeout will hold off on
// updating |holdDown_|. // updating |holdDown_|.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment