Skip to content

Commit

Permalink
fix(slider): truncate long decimal values
Browse files Browse the repository at this point in the history
When a slider has a decimal step, we can end up with a value being assigned to the model that looks like 33.300000000000004. These changes ensure that the value is rounded to the same amount of decimals as the step.

Fixes angular#10951.
  • Loading branch information
crisbeto committed Apr 23, 2018
1 parent a0c77e2 commit 326bdd6
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 4 deletions.
10 changes: 10 additions & 0 deletions src/lib/slider/slider.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,16 @@ describe('MatSlider', () => {

expect(sliderDebugElement.componentInstance.displayValue).toBe(100);
});

it('should truncate long decimal values when using a decimal step', () => {
fixture.componentInstance.step = 0.1;
fixture.detectChanges();

dispatchSlideEventSequence(sliderNativeElement, 0, 0.333333, gestureConfig);

expect(sliderInstance.value).toBe(33.3);
});

});

describe('slider with auto ticks', () => {
Expand Down
14 changes: 10 additions & 4 deletions src/lib/slider/slider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ export class MatSlider extends _MatSliderMixinBase
this._step = coerceNumberProperty(v, this._step);

if (this._step % 1 !== 0) {
this._roundLabelTo = this._step.toString().split('.').pop()!.length;
this._roundToDecimal = this._step.toString().split('.').pop()!.length;
}

// Since this could modify the label, we need to notify the change detection.
Expand Down Expand Up @@ -260,8 +260,8 @@ export class MatSlider extends _MatSliderMixinBase
// 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 && this.value % 1 !== 0) {
return this.value.toFixed(this._roundLabelTo);
if (this._roundToDecimal && this.value && this.value % 1 !== 0) {
return this.value.toFixed(this._roundToDecimal);
}

return this.value || 0;
Expand Down Expand Up @@ -403,7 +403,7 @@ export class MatSlider extends _MatSliderMixinBase
private _controlValueAccessorChangeFn: (value: any) => void = () => {};

/** Decimal places to round to, based on the step amount. */
private _roundLabelTo: number;
private _roundToDecimal: number;

/** Subscription to the Directionality change EventEmitter. */
private _dirChangeSubscription = Subscription.EMPTY;
Expand Down Expand Up @@ -639,6 +639,12 @@ export class MatSlider extends _MatSliderMixinBase
// whole number divisible by the step relative to the min.
let closestValue = Math.round((exactValue - this.min) / this.step) * this.step + this.min;

// If we've got a step with a decimal, we may end up with something like 33.300000000000004.
// Truncate the value to ensure that it matches the label and to make it easier to work with.
if (this._roundToDecimal) {
closestValue = parseFloat(closestValue.toFixed(this._roundToDecimal));
}

// The value needs to snap to the min and max.
this.value = this._clamp(closestValue, this.min, this.max);
}
Expand Down

0 comments on commit 326bdd6

Please sign in to comment.