Skip to content

Commit

Permalink
fix: broken min and max attributes in Slider and more
Browse files Browse the repository at this point in the history 
* simplified inner logic (less code, removed a few @State props that were triggering extra renders)
* min and max attributes works as expected
* added unit attribute to change the default "%" or leave it out
* added decimals attribute to control how many decimals to show (0, 1 or 2)
* added the scaleInput event, that fires on every input change, and changed scaleChange event to fire only when user input stops (native-like)
* updated tests and snapshots
  • Loading branch information
Arturo Castillo Delgado authored May 19, 2021
1 parent 06e8e2c commit 66543d0
Show file tree
Hide file tree
Showing 5 changed files with 183 additions and 120 deletions.
12 changes: 5 additions & 7 deletions packages/components/src/components/slider/__snapshots__/slider.spec.ts.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,12 @@ exports[`Slider should match snapshot 1`] = `
<div class="slider" part="slider">
<div class="slider__track-wrapper" part="track-wrapper">
<div class="slider__track" part="track">
<div class="slider__bar" part="bar" style="width: undefined%;"></div>
<div class="slider__thumb-wrapper" part="thumb-wrapper" style="left: undefined%;">
<div aria-labelledby="slider-0-label" aria-orientation="horizontal" aria-valuemax="100" aria-valuemin="0" aria-valuetext="undefined%" class="slider__thumb" id="slider-0" part="thumb" role="slider" tabindex="0"></div>
<div class="slider__bar" part="bar" style="width: 0%;"></div>
<div class="slider__thumb-wrapper" part="thumb-wrapper" style="left: 0%;">
<div aria-labelledby="slider-0-label" aria-orientation="horizontal" aria-valuemax="100" aria-valuemin="0" aria-valuetext="undefined" class="slider__thumb" id="slider-0" part="thumb" role="slider" tabindex="0"></div>
</div>
</div>
<div class="slider__display-value" part="display-value">
%
</div>
<div class="slider__display-value" part="display-value"></div>
</div>
</div>
</mock:shadow-root>
Expand All @@ -29,7 +27,7 @@ exports[`Slider should match snapshot 2`] = `
<div class="slider__track" part="track">
<div class="slider__bar" part="bar" style="width: 10%;"></div>
<div class="slider__thumb-wrapper" part="thumb-wrapper" style="left: 10%;">
<div aria-labelledby="slider-1-label" aria-orientation="horizontal" aria-valuemax="100" aria-valuemin="0" aria-valuenow="10" aria-valuetext="10%" class="slider__thumb" id="slider-1" part="thumb" role="slider" tabindex="0"></div>
<div aria-labelledby="slider-1-label" aria-orientation="horizontal" aria-valuemax="100" aria-valuemin="0" aria-valuenow="10" aria-valuetext="10" class="slider__thumb" id="slider-1" part="thumb" role="slider" tabindex="0"></div>
</div>
</div>
<div class="slider__display-value" part="display-value">
Expand Down
31 changes: 17 additions & 14 deletions packages/components/src/components/slider/readme.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,30 @@

## Properties

| Property | Attribute | Description | Type | Default |
| ------------- | -------------- | --------------------------------------------------------------------- | --------- | ----------- |
| `customColor` | `custom-color` | (optional) slider custom color | `string` | `''` |
| `disabled` | `disabled` | (optional) disabled | `boolean` | `false` |
| `label` | `label` | (optional) slider label | `string` | `undefined` |
| `max` | `max` | (optional) the maximal value of the slider | `number` | `100` |
| `min` | `min` | t(optional) he minimal value of the slider | `number` | `0` |
| `showValue` | `show-value` | (optional) slider display value | `boolean` | `true` |
| `sliderId` | `slider-id` | (optional) Slider id | `string` | `undefined` |
| `step` | `step` | (optional) the step size to increase or decrease when dragging slider | `number` | `1` |
| `styles` | `styles` | (optional) Injected CSS styles | `string` | `undefined` |
| `thumbLarge` | `thumb-large` | (optional) larger thumb | `boolean` | `false` |
| `trackSmall` | `track-small` | (optional) smaller track | `boolean` | `false` |
| `value` | `value` | (optional) the display value of the slider | `number` | `undefined` |
| Property | Attribute | Description | Type | Default |
| ------------- | -------------- | --------------------------------------------------------------------- | ------------- | ----------- |
| `customColor` | `custom-color` | (optional) slider custom color | `string` | `''` |
| `decimals` | `decimals` | (optional) number of decimal places | `0 \| 1 \| 2` | `0` |
| `disabled` | `disabled` | (optional) disabled | `boolean` | `false` |
| `label` | `label` | (optional) slider label | `string` | `undefined` |
| `max` | `max` | (optional) the maximal value of the slider | `number` | `100` |
| `min` | `min` | t(optional) he minimal value of the slider | `number` | `0` |
| `showValue` | `show-value` | (optional) slider display value | `boolean` | `true` |
| `sliderId` | `slider-id` | (optional) Slider id | `string` | `undefined` |
| `step` | `step` | (optional) the step size to increase or decrease when dragging slider | `number` | `1` |
| `styles` | `styles` | (optional) Injected CSS styles | `string` | `undefined` |
| `thumbLarge` | `thumb-large` | (optional) larger thumb | `boolean` | `false` |
| `trackSmall` | `track-small` | (optional) smaller track | `boolean` | `false` |
| `unit` | `unit` | (optional) slider value unit | `string` | `'%'` |
| `value` | `value` | (optional) the display value of the slider | `number` | `undefined` |


## Events

| Event | Description | Type |
| ------------- | ----------- | --------------------- |
| `scaleChange` | | `CustomEvent<number>` |
| `scaleInput` | | `CustomEvent<number>` |


## Shadow Parts
Expand Down
43 changes: 7 additions & 36 deletions packages/components/src/components/slider/slider.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ describe('Slider', () => {
expect(page.rootInstance.min).toBe(0);
expect(page.rootInstance.max).toBe(100);
expect(page.rootInstance.step).toBe(1);
expect(page.rootInstance.unit).toBe('%');
expect(page.rootInstance.decimals).toBe(0);
expect(page.rootInstance.showValue).toBe(true);
expect(page.rootInstance.customColor).toBe('');
expect(page.rootInstance.disabled).toBe(false);
Expand All @@ -102,8 +104,10 @@ describe('Slider', () => {
page.root.min = 10;
page.root.max = 90;
page.root.step = 2;
page.root.unit = '';
page.root.decimals = 2;
page.root.label = 'slider label';
page.root.showValue = 'false';
page.root.showValue = false;
page.root.customColor = 'magenta';
page.root.disabled = 'true';
page.root.trackSmall = 'true';
Expand All @@ -115,6 +119,8 @@ describe('Slider', () => {
expect(page.rootInstance.min).toBe(10);
expect(page.rootInstance.max).toBe(90);
expect(page.rootInstance.step).toBe(2);
expect(page.rootInstance.unit).toBe('');
expect(page.rootInstance.decimals).toBe(2);
expect(page.rootInstance.label).toBe('slider label');
expect(page.rootInstance.showValue).toBe(false);
expect(page.rootInstance.customColor).toBe('magenta');
Expand All @@ -126,41 +132,6 @@ describe('Slider', () => {
});
});

describe('functions', () => {
it('setPosition(-20) returns 0', () => {
const element = new Slider();
element.setPosition(-20);
expect(element.value).toBe(0);
});

it('setPosition(120) returns 100', () => {
const element = new Slider();
element.setPosition(120);
expect(element.value).toBe(100);
});

it('setPosition(50) returns 50', () => {
const element = new Slider();
element.setPosition(50);
expect(element.value).toBe(50);
});

it('currentPosition()', () => {
const element = new Slider();
element.value = 22;
element.max = 90;
element.min = 10;
expect(element.currentPosition()).toBe('15%');
});

it('onDragEnd()', () => {
const element = new Slider();
element.dragging = true;
element.onDragEnd();
expect(element.dragging).toBe(false);
});
});

it('keydown .slider__thumb with ArrowRight', async () => {
const page = await newSpecPage({
components: [Slider],
Expand Down
134 changes: 71 additions & 63 deletions packages/components/src/components/slider/slider.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,16 @@
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
*/

// ((input - min) * 100) / (max - min)

import {
Component,
h,
State,
Prop,
Host,
Event,
Watch,
EventEmitter,
} from '@stencil/core';
import classNames from 'classnames';
Expand All @@ -42,6 +45,10 @@ export class Slider {
@Prop() label?: string;
/** (optional) slider display value */
@Prop() showValue?: boolean = true;
/** (optional) slider value unit */
@Prop() unit?: string = '%';
/** (optional) number of decimal places */
@Prop() decimals?: 0 | 1 | 2 = 0;
/** (optional) slider custom color */
@Prop() customColor?: string = '';
/** (optional) disabled */
Expand All @@ -55,13 +62,14 @@ export class Slider {
/** (optional) Injected CSS styles */
@Prop() styles?: string;

@State() dragging: boolean;
@State() startX: number;
@State() currentX: number;
@State() startPosition: number;
@State() newPosition: number;
// The actual position in % of the slider thumb
@State() position: number;

@Event() scaleChange: EventEmitter<number>;
@Event() scaleInput: EventEmitter<number>;

private dragging: boolean;
private offsetLeft: number;

constructor() {
this.onDragging = this.onDragging.bind(this);
Expand All @@ -72,48 +80,79 @@ export class Slider {
if (this.sliderId == null) {
this.sliderId = 'slider-' + i++;
}
this.setPosition();
}

disconnectedCallback() {
this.removeGlobalListeners();
}

onButtonDown = (event: any) => {
onButtonDown = () => {
if (this.disabled) {
return;
}
this.onDragStart(event);
this.onDragStart();
this.addGlobalListeners();
};

onDragStart = (event: any) => {
onKeyDown = (event: KeyboardEvent) => {
let steps = 0;
if (['ArrowRight', 'ArrowLeft'].includes(event.key)) {
steps = event.key === 'ArrowRight' ? this.step : -this.step;
}
if (['ArrowUp', 'ArrowDown'].includes(event.key)) {
steps = event.key === 'ArrowUp' ? this.step * 10 : -this.step * 10;
}
this.setValue(this.value + steps);
};

onDragStart = () => {
this.dragging = true;
this.startX = this.handleTouchEvent(event).clientX;
this.startPosition = parseInt(this.currentPosition(), 10);
this.offsetLeft = this.sliderTrack.getBoundingClientRect().left;
};

onDragging = (event: any) => {
const { dragging, startX, startPosition } = this;
const { dragging, offsetLeft } = this;

if (dragging) {
this.currentX = this.handleTouchEvent(event).clientX;
const currentX = this.handleTouchEvent(event).clientX;
const position: number =
((currentX - offsetLeft) / this.sliderTrack.offsetWidth) * 100;
const nextValue = (position * (this.max - this.min)) / 100 + this.min;
// https://stackoverflow.com/q/14627566
const roundedNextValue = Math.ceil(nextValue / this.step) * this.step;
this.setValue(roundedNextValue);
}
};

let diff: number;
onDragEnd = () => {
this.dragging = false;
this.scaleChange.emit(this.value);
this.removeGlobalListeners();
};

diff = ((this.currentX - startX) / this.sliderTrack.offsetWidth) * 100;
handleTouchEvent(event: any): MouseEvent | Touch {
return event.type.indexOf('touch') === 0 ? event.touches[0] : event;
}

this.newPosition = startPosition + diff;
this.setPosition(this.newPosition);
}
setValue = (nextValue: number) => {
this.value = this.clamp(nextValue);
this.scaleInput.emit(this.value);
};

onDragEnd = () => {
const { dragging, newPosition } = this;
if (dragging) {
this.dragging = false;
@Watch('value')
handleValueChange() {
this.setPosition();
}

setPosition = () => {
if (!this.value) {
this.position = 0;
return;
}
this.setPosition(newPosition || this.startPosition);
this.removeGlobalListeners();
const clampedValue = this.clamp(this.value);
// https://stackoverflow.com/a/25835683
this.position = ((clampedValue - this.min) * 100) / (this.max - this.min);
};

addGlobalListeners() {
Expand All @@ -130,42 +169,6 @@ export class Slider {
window.removeEventListener('touchend', this.onDragEnd);
}

handleTouchEvent(event: any): MouseEvent | Touch {
return event.type.indexOf('touch') === 0 ? event.touches[0] : event;
}

onKeyDown = (event) => {
if (['ArrowRight', 'ArrowLeft'].includes(event.key)) {
this.setPosition(
this.value + (event.key === 'ArrowRight' ? this.step : -this.step)
);
}
if (['ArrowUp', 'ArrowDown'].includes(event.key)) {
this.setPosition(
this.value +
(event.key === 'ArrowUp' ? this.step * 10 : -this.step * 10)
);
}
};

setPosition = (newPosition: number) => {
if (newPosition < 0) {
newPosition = 0;
} else if (newPosition > 100) {
newPosition = 100;
}

const lengthPerStep = 100 / ((this.max - this.min) / this.step);
const steps = Math.round(newPosition / lengthPerStep);
this.value =
steps * lengthPerStep * (this.max - this.min) * 0.01 + this.min;
this.scaleChange.emit(Math.abs(this.value));
};

currentPosition(): string {
return `${((this.value - this.min) / (this.max - this.min)) * 100}%`;
}

render() {
return (
<Host>
Expand All @@ -192,14 +195,14 @@ export class Slider {
part="bar"
class="slider__bar"
style={{
width: `${this.value}%`,
width: `${this.position}%`,
backgroundColor: this.customColor,
}}
></div>
<div
part="thumb-wrapper"
class="slider__thumb-wrapper"
style={{ left: `${this.value}%` }}
style={{ left: `${this.position}%` }}
onMouseDown={this.onButtonDown}
onTouchStart={this.onButtonDown}
>
Expand All @@ -212,7 +215,7 @@ export class Slider {
aria-valuemin={this.min}
aria-valuenow={this.value}
aria-valuemax={this.max}
aria-valuetext={`${this.value}%`}
aria-valuetext={`${this.value}`}
aria-labelledby={`${this.sliderId}-label`}
aria-orientation="horizontal"
aria-disabled={this.disabled}
Expand All @@ -222,7 +225,8 @@ export class Slider {
</div>
{this.showValue && (
<div part="display-value" class="slider__display-value">
{this.value}%
{this.value != null && this.value.toFixed(this.decimals)}
{this.value != null && this.unit}
</div>
)}
</div>
Expand Down Expand Up @@ -250,4 +254,8 @@ export class Slider {
this.thumbLarge && `${prefix}thumb-large`
);
}

private clamp = (val: number) => {
return Math.min(Math.max(val, this.min), this.max);
};
}
Loading

0 comments on commit 66543d0

Please sign in to comment.