Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(slider): round decimals in the thumb label #2527

Merged
merged 5 commits into from
Jan 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/lib/slider/slider.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<div class="md-slider-thumb-container" [ngStyle]="thumbContainerStyles">
<div class="md-slider-thumb"></div>
<div class="md-slider-thumb-label">
<span class="md-slider-thumb-label-text">{{value}}</span>
<span class="md-slider-thumb-label-text">{{displayValue}}</span>
</div>
</div>
</div>
29 changes: 27 additions & 2 deletions src/lib/slider/slider.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,29 @@ 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 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', () => {
fixture.componentInstance.step = 0.1;
fixture.detectChanges();

dispatchSlideEventSequence(sliderNativeElement, 0, 1, gestureConfig);

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

describe('slider with auto ticks', () => {
Expand Down Expand Up @@ -1162,10 +1185,12 @@ class SliderWithMinAndMax {
class SliderWithValue { }

@Component({
template: `<md-slider step="25"></md-slider>`,
template: `<md-slider [step]="step"></md-slider>`,
styles: [styles],
})
class SliderWithStep { }
class SliderWithStep {
step = 25;
}

@Component({
template: `<md-slider step="5" tickInterval="auto"></md-slider>`,
Expand Down
23 changes: 22 additions & 1 deletion src/lib/slider/slider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,21 @@ export class MdSlider implements ControlValueAccessor {
*/
_isActive: boolean = false;

/** Decimal places to round to, based on the step amount. */
private _roundLabelTo: 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._roundLabelTo = this._step.toString().split('.').pop().length;
}
}

private _tickInterval: 'auto' | number = 0;

Expand Down Expand Up @@ -238,6 +247,18 @@ 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 {
// 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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's a couple cases this misses:
.999 to 2 places
.899 to 2 places
.001 to 2 places

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to keep this as simple as possible, because it gets called on every change detection.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, well at least leave a note so that if this issue pops up we know that it was an intentional choice

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}

return this.value;
}

/**
* 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).
Expand Down