fix(slider): use internal "input" for value sanitation

adobe · Mar 26, 2020 · dd588c9 · dd588c9
1 parent cab535e
commit dd588c9
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 57 deletions.
diff --git a/packages/slider/src/slider.ts b/packages/slider/src/slider.ts
@@ -40,12 +40,16 @@ export class Slider extends Focusable {
 
     public set value(value: number) {
         const oldValue = this.value;
+        if (this.input) {
+            this.input.value = String(value);
+        }
+        const newValue = this.input ? parseFloat(this.input.value) : value;
 
-        if (value === oldValue) {
+        if (newValue === oldValue) {
             return;
         }
 
-        this._value = value;
+        this._value = newValue;
         this.requestUpdate('value', oldValue);
     }
 
@@ -123,6 +127,7 @@ export class Slider extends Focusable {
 
     private supportsPointerEvent = 'setPointerCapture' in this;
     private currentMouseEvent?: MouseEvent;
+    private boundingClientRect?: DOMRect;
 
     public get focusElement(): HTMLElement {
         return this.input ? this.input : this;
@@ -139,7 +144,6 @@ export class Slider extends Focusable {
 
     protected updated(changedProperties: PropertyValues): void {
         if (changedProperties.has('value')) {
-            this.value = this.clampValue(this.value);
             if (this.dragging) {
                 this.dispatchInputEvent();
             }
@@ -299,6 +303,7 @@ export class Slider extends Focusable {
         if (this.disabled) {
             return;
         }
+        this.boundingClientRect = this.getBoundingClientRect();
         this.focus();
         this.dragging = true;
         this.handle.setPointerCapture(event.pointerId);
@@ -311,6 +316,7 @@ export class Slider extends Focusable {
         if (this.disabled) {
             return;
         }
+        this.boundingClientRect = this.getBoundingClientRect();
         document.addEventListener('mousemove', this.onMouseMove);
         document.addEventListener('mouseup', this.onMouseUp);
         this.focus();
@@ -373,6 +379,7 @@ export class Slider extends Focusable {
         if (event.target === this.handle || this.disabled) {
             return;
         }
+        this.boundingClientRect = this.getBoundingClientRect();
         this.dragging = true;
         this.handle.setPointerCapture(event.pointerId);
 
@@ -388,6 +395,7 @@ export class Slider extends Focusable {
         }
         document.addEventListener('mousemove', this.onMouseMove);
         document.addEventListener('mouseup', this.onMouseUp);
+        this.boundingClientRect = this.getBoundingClientRect();
         this.dragging = true;
         this.currentMouseEvent = event;
         this._trackMouseEvent();
@@ -398,8 +406,7 @@ export class Slider extends Focusable {
      */
     private onInputChange(): void {
         const inputValue = parseFloat(this.input.value);
-        this.value = this.clampValue(inputValue);
-        this.input.value = this.value.toString();
+        this.value = inputValue;
 
         this.dispatchChangeEvent();
     }
@@ -418,32 +425,20 @@ export class Slider extends Focusable {
      * @return: Slider value that correlates to the position under the pointer
      */
     private calculateHandlePosition(event: PointerEvent | MouseEvent): number {
-        const rect = this.getBoundingClientRect();
+        if (!this.boundingClientRect) {
+            return this.value;
+        }
+        const rect = this.boundingClientRect;
         const minOffset = rect.left;
         const offset = event.clientX;
         const size = rect.width;
 
         const percent = (offset - minOffset) / size;
-        let value = this.min + (this.max - this.min) * percent;
-
-        value = this.clampValue(value);
-
-        if (this.step) {
-            value = Math.round(value / this.step) * this.step;
-        }
+        const value = this.min + (this.max - this.min) * percent;
 
         return value;
     }
 
-    /**
-     * @param: value to be clamped
-     * @return: the original value if in range, this.max if over, and this.min if under
-     */
-    private clampValue(value: number): number {
-        const reducedValue = Math.min(value, this.max);
-        return Math.max(reducedValue, this.min);
-    }
-
     private dispatchInputEvent(): void {
         const inputEvent = new Event('input', {
             bubbles: true,

diff --git a/packages/slider/stories/slider.stories.ts b/packages/slider/stories/slider.stories.ts
@@ -9,18 +9,10 @@ the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTA
 OF ANY KIND, either express or implied. See the License for the specific language
 governing permissions and limitations under the License.
 */
-import {
-    html,
-    action,
-    text,
-    number,
-    select,
-    boolean,
-} from '@open-wc/demoing-storybook';
-import { ifDefined } from 'lit-html/directives/if-defined';
+import { html, action, text, number } from '@open-wc/demoing-storybook';
 
 import '../';
-import { variants, Slider } from '../';
+import { Slider } from '../';
 import { TemplateResult } from 'lit-html';
 
 export default {
@@ -29,38 +21,19 @@ export default {
 };
 
 export const Default = (): TemplateResult => {
-    const sliderVariants = ['', ...variants];
-    const value = number('Value', 50, {}, 'Element');
-    const min = number('Min', 0, {}, 'Element');
-    const max = number('Max', 100, {}, 'Element');
-    const step = number('Step', 1, {}, 'Element');
-    const tickStep = number('Tick Step', 10, {}, 'Element');
-    const label = text('Label', 'Opacity', 'Element');
-    const tickLabels = boolean('Tick Labels', false, 'Element');
-    const variant = select(
-        'Variant',
-        sliderVariants,
-        sliderVariants[0],
-        'Element'
-    );
     const handleEvent = (event: Event): void => {
         const target = event.target as Slider;
         action(event.type)(target.value);
     };
     return html`
         <div style="width: 500px; margin: 12px 20px;">
             <sp-slider
-                value="${value}"
-                step="${step}"
-                tick-step="${tickStep}"
-                min="${min}"
-                max="${max}"
-                label="${label}"
-                ?tick-labels="${tickLabels}"
-                id="opacity-slider"
-                variant=${ifDefined(variant || undefined)}
-                @sp-slider:input=${handleEvent}
-                @sp-slider:change=${handleEvent}
+                label="Opacity"
+                max="100"
+                min="0"
+                value="50"
+                @input=${handleEvent}
+                @change=${handleEvent}
             ></sp-slider>
         </div>
     `;