Skip to content

Commit

Permalink
fix(slider): default values to between min and max to better match na…
Browse files Browse the repository at this point in the history 
…tive input

PiperOrigin-RevId: 538701031
  • Loading branch information
@material-web-copybara @copybara-github
material-web-copybara authored and copybara-github committed Jun 8, 2023
1 parent fbed917 commit 2ef3606
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 38 deletions.
4 changes: 2 additions & 2 deletions slider/demo/stories.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ const customStyling: MaterialStoryInit<StoryKnobs> = {
const target = event.target as MdSlider;
const {min, max, valueStart, valueEnd} = target;
const range = max - min;
const fractionStart = valueStart / range;
const fractionEnd = valueEnd / range;
const fractionStart = valueStart! / range;
const fractionEnd = valueEnd! / range;
target.valueStartLabel = labelFor(fractionStart);
target.valueEndLabel = labelFor(fractionEnd);
}
Expand Down
74 changes: 42 additions & 32 deletions slider/lib/slider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ import {MdRipple} from '../../ripple/ripple.js';
// Disable warning for classMap with destructuring
// tslint:disable:quoted-properties-on-dictionary

/** The default value for a continuous slider. */
const DEFAULT_VALUE = 50;
/** The default start value for a range slider. */
const DEFAULT_VALUE_START = 25;
/** The default end value for a range slider. */
const DEFAULT_VALUE_END = 75;

/**
* Slider component.
Expand Down Expand Up @@ -61,19 +55,17 @@ export class Slider extends LitElement {
/**
* The slider value displayed when range is false.
*/
@property({type: Number}) value = DEFAULT_VALUE;
@property({type: Number}) value?: number;

/**
* The slider start value displayed when range is true.
*/
@property({type: Number, attribute: 'value-start'})
valueStart = DEFAULT_VALUE_START;
@property({type: Number, attribute: 'value-start'}) valueStart?: number;

/**
* The slider end value displayed when range is true.
*/
@property({type: Number, attribute: 'value-end'})
valueEnd = DEFAULT_VALUE_END;
@property({type: Number, attribute: 'value-end'}) valueEnd?: number;

/**
* An optional label for the slider's value displayed when range is
Expand Down Expand Up @@ -195,8 +187,8 @@ export class Slider extends LitElement {
@state() private startOnTop = false;
@state() private handlesOverlapping = false;

@state() private renderValueStart = 0;
@state() private renderValueEnd = 0;
@state() private renderValueStart?: number;
@state() private renderValueEnd?: number;

// used in synthetic events generated to control ripple hover state.
private ripplePointerId = 1;
Expand Down Expand Up @@ -229,12 +221,12 @@ export class Slider extends LitElement {
protected override willUpdate(changed: PropertyValues) {
this.renderValueStart = changed.has('valueStart') ?
this.valueStart :
(this.inputStart?.valueAsNumber ?? 0);
this.inputStart?.valueAsNumber;
const endValueChanged =
(changed.has('valueEnd') && this.range) || changed.has('value');
this.renderValueEnd = endValueChanged ?
(this.range ? this.valueEnd : this.value) :
(this.inputEnd?.valueAsNumber ?? 0);
this.inputEnd?.valueAsNumber;
// manually handle ripple hover state since the handle is pointer events
// none.
if (changed.get('handleStartHover') !== undefined) {
Expand Down Expand Up @@ -269,6 +261,26 @@ export class Slider extends LitElement {
this.renderValueStart = this.inputStart!.valueAsNumber;
}
this.renderValueEnd = this.inputEnd!.valueAsNumber;
// update values if they are unset
// when using a range, default to equi-distant between
// min - valueStart - valueEnd - max
if (this.range) {
const segment = (this.max - this.min) / 3;
if (this.valueStart === undefined) {
this.inputStart!.valueAsNumber = this.min + segment;
// read actual value from input
const v = this.inputStart!.valueAsNumber;
this.valueStart = this.renderValueStart = v;
}
if (this.valueEnd === undefined) {
this.inputEnd!.valueAsNumber = this.min + 2 * segment;
// read actual value from input
const v = this.inputEnd!.valueAsNumber;
this.valueEnd = this.renderValueEnd = v;
}
} else {
this.value ??= this.renderValueEnd;
}
if (changed.has('range') || changed.has('renderValueStart') ||
changed.has('renderValueEnd') || this.isUpdatePending) {
this.handlesOverlapping = isOverlapping(this.handleStart, this.handleEnd);
Expand All @@ -281,9 +293,10 @@ export class Slider extends LitElement {
protected override render() {
const step = this.step === 0 ? 1 : this.step;
const range = Math.max(this.max - this.min, step);
const startFraction =
this.range ? ((this.renderValueStart - this.min) / range) : 0;
const endFraction = (this.renderValueEnd - this.min) / range;
const startFraction = this.range ?
(((this.renderValueStart ?? this.min) - this.min) / range) :
0;
const endFraction = ((this.renderValueEnd ?? this.min) - this.min) / range;
const containerStyles = {
// for clipping inputs and active track.
'--slider-start-fraction': String(startFraction),
Expand Down Expand Up @@ -375,11 +388,8 @@ export class Slider extends LitElement {
</div>`;
}

private renderInput({start, value, label}: {
start: boolean,
value: number,
label: string,
}) {
private renderInput({start, value, label}:
{start: boolean; value?: number; label: string;}) {
const name = start ? `start` : `end`;
// when ranged, ensure announcement includes value info.
// Needed for closure conformance
Expand Down Expand Up @@ -619,27 +629,27 @@ export class Slider extends LitElement {
/** @private */
formResetCallback() {
if (this.range) {
this.valueStart =
Number(this.getAttribute('value-start') ?? DEFAULT_VALUE_START);
this.valueEnd =
Number(this.getAttribute('value-end') ?? DEFAULT_VALUE_END);
const valueStart = this.getAttribute('value-start');
this.valueStart = valueStart !== null ? Number(valueStart) : undefined;
const valueEnd = this.getAttribute('value-end');
this.valueEnd = valueEnd !== null ? Number(valueEnd) : undefined;
return;
}

this.value = Number(this.getAttribute('value') ?? DEFAULT_VALUE);
const value = this.getAttribute('value');
this.value = value !== null ? Number(value) : undefined;
}

/** @private */
formStateRestoreCallback(state: string|Array<[string, string]>|null) {
if (Array.isArray(state)) {
const [[, valueStart], [, valueEnd]] = state;
this.valueStart = Number(valueStart ?? DEFAULT_VALUE_START);
this.valueEnd = Number(valueEnd ?? DEFAULT_VALUE_START);
this.valueStart = Number(valueStart);
this.valueEnd = Number(valueEnd);
this.range = true;
return;
}

this.value = Number(state ?? DEFAULT_VALUE);
this.value = Number(state);
this.range = false;
}
}
Expand Down
52 changes: 48 additions & 4 deletions slider/slider_test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ function getSliderTemplate(props?: SliderTestProps) {
return html`
<md-slider
.range=${props?.range ?? false}
.value=${props?.value ?? 0}
.valueStart=${props?.valueStart ?? 0}
.valueEnd=${props?.valueEnd ?? 0}
.value=${props?.value}
.valueStart=${props?.valueStart}
.valueEnd=${props?.valueEnd}
.step=${props?.step ?? 1}
.min=${props?.min ?? 0}
.max=${props?.max ?? 100}
Expand Down Expand Up @@ -147,7 +147,6 @@ describe('<md-slider>', () => {
it('update via interaction', async () => {
const props = {range: true, valueStart: 2, valueEnd: 6};
const {harness} = await setupTest(props);
expect(harness.element.value).toEqual(0);
const [endInput, startInput] = harness.getInputs();
await harness.simulateValueInteraction(7, endInput);
expect(harness.element.valueStart).toEqual(2);
Expand Down Expand Up @@ -346,6 +345,21 @@ describe('<md-slider>', () => {
});
});

describe('default values', () => {
it('defaults value to midway between min/max', async () => {
const {harness} = await setupTest({min: -100, max: -40});
await harness.element.updateComplete;
expect(harness.element.value).toBe(-70);
});

it('defaults valueStart/End to equidistant between min/max', async () => {
const {harness} = await setupTest({range: true, min: 80, max: 100});
await harness.element.updateComplete;
expect(harness.element.valueStart).toBe(87);
expect(harness.element.valueEnd).toBe(93);
});
});

describe('forms', () => {
createFormTests({
queryControl: root => root.querySelector('md-slider'),
Expand Down Expand Up @@ -383,6 +397,36 @@ describe('<md-slider>', () => {
expect(formData.get('slider-end')).toBe('10');
}
},
{
name: 'single default value',
render: () => html`<md-slider name="slider"></md-slider>`,
assertValue(formData) {
expect(formData.get('slider')).toBe('50');
}
},
{
name: 'single default value with min/max',
render: () =>
html`<md-slider name="slider" min="100" max="300"></md-slider>`,
assertValue(formData) {
expect(formData.get('slider')).toBe('200');
}
},
{
name: 'multiple default values',
render: () => html`<md-slider range name="slider"></md-slider>`,
assertValue(formData) {
expect(formData.getAll('slider')).toEqual(['33', '67']);
}
},
{
name: 'multiple default values with min/max',
render: () =>
html`<md-slider range name="slider" min="100" max="300"></md-slider>`,
assertValue(formData) {
expect(formData.getAll('slider')).toEqual(['167', '233']);
}
},
{
name: 'disabled',
render: () =>
Expand Down

0 comments on commit 2ef3606

Please sign in to comment.