From 4faf85161da9c7dc8599f45847fc6abb707090a9 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Wed, 4 Jan 2017 22:21:14 +0100 Subject: [PATCH 1/5] fix(slider): round decimals in the thumb label Rounds down the thumb label value to a maximum of one decimal place, in order to avoid displaying weird rounding errors. Fixes #2511. --- src/lib/slider/slider.html | 2 +- src/lib/slider/slider.spec.ts | 24 ++++++++++++++++++++++-- src/lib/slider/slider.ts | 6 ++++++ 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/lib/slider/slider.html b/src/lib/slider/slider.html index a50837a8e39c..a18361956483 100644 --- a/src/lib/slider/slider.html +++ b/src/lib/slider/slider.html @@ -9,7 +9,7 @@
- {{value}} + {{displayValue}}
diff --git a/src/lib/slider/slider.spec.ts b/src/lib/slider/slider.spec.ts index e7acc53f8eed..09deb828b883 100644 --- a/src/lib/slider/slider.spec.ts +++ b/src/lib/slider/slider.spec.ts @@ -431,6 +431,24 @@ describe('MdSlider', () => { // The closest snap is at the end of the slider. expect(trackFillElement.style.transform).toContain('scaleX(1)'); }); + + it('should round the value inside the label to one decimal place', () => { + fixture.componentInstance.step = 0.1; + fixture.detectChanges(); + + dispatchSlideEventSequence(sliderNativeElement, 0, 0.333333, gestureConfig); + + expect(sliderDebugElement.componentInstance.displayValue).toBe('33.3'); + }); + + it('should not add decimals to the value if it is a whole number', () => { + fixture.componentInstance.step = 0.1; + fixture.detectChanges(); + + dispatchSlideEventSequence(sliderNativeElement, 0, 1, gestureConfig); + + expect(sliderDebugElement.componentInstance.displayValue).toBe(100); + }); }); describe('slider with auto ticks', () => { @@ -1162,10 +1180,12 @@ class SliderWithMinAndMax { class SliderWithValue { } @Component({ - template: ``, + template: ``, styles: [styles], }) -class SliderWithStep { } +class SliderWithStep { + step = 25; +} @Component({ template: ``, diff --git a/src/lib/slider/slider.ts b/src/lib/slider/slider.ts index 45935eccf562..e789219981d3 100644 --- a/src/lib/slider/slider.ts +++ b/src/lib/slider/slider.ts @@ -238,6 +238,12 @@ export class MdSlider implements ControlValueAccessor { set vertical(value: any) { this._vertical = coerceBooleanProperty(value); } private _vertical = false; + /** The value to be used for display purposes. */ + get displayValue(): string|number { + // Skip adding the decimal part if the number is whole. + return this.value % 1 === 0 ? this.value : this.value.toFixed(1); + } + /** * Whether the axis of the slider is inverted. * (i.e. whether moving the thumb in the positive x or y direction decreases the slider's value). From 6e4db8cda43047efc4d3183cc49355ac5bf2e21f Mon Sep 17 00:00:00 2001 From: crisbeto Date: Mon, 16 Jan 2017 22:04:10 +0100 Subject: [PATCH 2/5] Determine the rounding, based on the the provided step. --- src/lib/slider/slider.spec.ts | 19 ++++++++++++------- src/lib/slider/slider.ts | 18 +++++++++++++++--- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/lib/slider/slider.spec.ts b/src/lib/slider/slider.spec.ts index 09deb828b883..18017694679e 100644 --- a/src/lib/slider/slider.spec.ts +++ b/src/lib/slider/slider.spec.ts @@ -432,13 +432,18 @@ describe('MdSlider', () => { expect(trackFillElement.style.transform).toContain('scaleX(1)'); }); - it('should round the value inside the label to one decimal place', () => { - fixture.componentInstance.step = 0.1; - fixture.detectChanges(); - - dispatchSlideEventSequence(sliderNativeElement, 0, 0.333333, gestureConfig); - - expect(sliderDebugElement.componentInstance.displayValue).toBe('33.3'); + it('should round the value inside the label based on the provided step', () => { + let testStep = (step: number, expected: string) => { + fixture.componentInstance.step = step; + fixture.detectChanges(); + dispatchSlideEventSequence(sliderNativeElement, 0, 0.333333, gestureConfig); + expect(sliderDebugElement.componentInstance.displayValue.toString()).toBe(expected); + }; + + testStep(1, '33'); + testStep(0.1, '33.3'); + testStep(0.01, '33.33'); + testStep(0.001, '33.333'); }); it('should not add decimals to the value if it is a whole number', () => { diff --git a/src/lib/slider/slider.ts b/src/lib/slider/slider.ts index e789219981d3..c7d73073a5aa 100644 --- a/src/lib/slider/slider.ts +++ b/src/lib/slider/slider.ts @@ -147,12 +147,21 @@ export class MdSlider implements ControlValueAccessor { */ _isActive: boolean = false; + /** Decimal places to round to, based on the step amount. */ + private _roundTo: number; + private _step: number = 1; /** The values at which the thumb will snap. */ @Input() get step() { return this._step; } - set step(v) { this._step = coerceNumberProperty(v, this._step); } + set step(v) { + this._step = coerceNumberProperty(v, this._step); + + if (this._step % 1 !== 0) { + this._roundTo = this._step.toString().split('.').pop().length; + } + } private _tickInterval: 'auto' | number = 0; @@ -240,8 +249,11 @@ export class MdSlider implements ControlValueAccessor { /** The value to be used for display purposes. */ get displayValue(): string|number { - // Skip adding the decimal part if the number is whole. - return this.value % 1 === 0 ? this.value : this.value.toFixed(1); + if (this._roundTo && this.value % 1 !== 0) { + return this.value.toFixed(this._roundTo); + } + + return this.value; } /** From 2d2d7b868852358cb989fcacc092d2dd34b849ca Mon Sep 17 00:00:00 2001 From: crisbeto Date: Mon, 16 Jan 2017 22:05:41 +0100 Subject: [PATCH 3/5] More descriptive property name. --- src/lib/slider/slider.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/lib/slider/slider.ts b/src/lib/slider/slider.ts index c7d73073a5aa..1e7f0ef0bfec 100644 --- a/src/lib/slider/slider.ts +++ b/src/lib/slider/slider.ts @@ -148,7 +148,7 @@ export class MdSlider implements ControlValueAccessor { _isActive: boolean = false; /** Decimal places to round to, based on the step amount. */ - private _roundTo: number; + private _roundLabelTo: number; private _step: number = 1; @@ -159,7 +159,7 @@ export class MdSlider implements ControlValueAccessor { this._step = coerceNumberProperty(v, this._step); if (this._step % 1 !== 0) { - this._roundTo = this._step.toString().split('.').pop().length; + this._roundLabelTo = this._step.toString().split('.').pop().length; } } @@ -249,8 +249,8 @@ export class MdSlider implements ControlValueAccessor { /** The value to be used for display purposes. */ get displayValue(): string|number { - if (this._roundTo && this.value % 1 !== 0) { - return this.value.toFixed(this._roundTo); + if (this._roundLabelTo && this.value % 1 !== 0) { + return this.value.toFixed(this._roundLabelTo); } return this.value; From 1890734f59880cdc99b34b6db6a9dc4bd59344cc Mon Sep 17 00:00:00 2001 From: crisbeto Date: Tue, 17 Jan 2017 20:02:14 +0100 Subject: [PATCH 4/5] Add note on displayLabel performance. --- src/lib/slider/slider.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/lib/slider/slider.ts b/src/lib/slider/slider.ts index 1e7f0ef0bfec..0f7835d2c319 100644 --- a/src/lib/slider/slider.ts +++ b/src/lib/slider/slider.ts @@ -249,6 +249,9 @@ export class MdSlider implements ControlValueAccessor { /** The value to be used for display purposes. */ get displayValue(): string|number { + // Note that this could be improved further by rounding something like 0.999 to 0.99 or + // 0.899 to 0.89, however it is very performance sensitive, because it gets called on + // every change detection cycle. if (this._roundLabelTo && this.value % 1 !== 0) { return this.value.toFixed(this._roundLabelTo); } From 2740172d6d4f530fdd7668a5fd93992e0c1300fb Mon Sep 17 00:00:00 2001 From: crisbeto Date: Tue, 17 Jan 2017 23:21:22 +0100 Subject: [PATCH 5/5] Fix wrong comment. --- src/lib/slider/slider.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/slider/slider.ts b/src/lib/slider/slider.ts index 0f7835d2c319..e874791180bc 100644 --- a/src/lib/slider/slider.ts +++ b/src/lib/slider/slider.ts @@ -249,8 +249,8 @@ export class MdSlider implements ControlValueAccessor { /** The value to be used for display purposes. */ get displayValue(): string|number { - // Note that this could be improved further by rounding something like 0.999 to 0.99 or - // 0.899 to 0.89, however it is very performance sensitive, because it gets called on + // Note that this could be improved further by rounding something like 0.999 to 1 or + // 0.899 to 0.9, however it is very performance sensitive, because it gets called on // every change detection cycle. if (this._roundLabelTo && this.value % 1 !== 0) { return this.value.toFixed(this._roundLabelTo);