fix(slider): round decimals in the thumb label #2527
Conversation
googlebot
added
the
cla: yes
| /** 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); |
There was a problem hiding this comment.
Using one decimal place is too prescriptive, since having a step of, say 0.05 between 0 and 1 is valid. It would be nice to have the slider intelligently figure out the appropriate number of decimal places to show based on the step value.
There was a problem hiding this comment.
That's what I was thinking initially, however anything longer than 3 characters doesn't look right with the current setup.
There was a problem hiding this comment.
I would round it based on the step and consider the 3 characters thing a separate bug (we'll still have issues with 1000 even with no decimals)
|
cc @mmalerba |
|
@mmalerba I've switched it over to determine the rounding based on the |
| /** The value to be used for display purposes. */ | ||
| get displayValue(): string|number { | ||
| if (this._roundLabelTo && this.value % 1 !== 0) { | ||
| return this.value.toFixed(this._roundLabelTo); |
There was a problem hiding this comment.
I think there's a couple cases this misses:
.999 to 2 places
.899 to 2 places
.001 to 2 places
There was a problem hiding this comment.
I wanted to keep this as simple as possible, because it gets called on every change detection.
There was a problem hiding this comment.
Ok, well at least leave a note so that if this issue pops up we know that it was an intentional choice
crisbeto
force-pushed
the
2511/slider-decimal-display
branch
from
b47c9cd
to
b78426e
Compare
| @@ -226,6 +235,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 0.99 or | |||
There was a problem hiding this comment.
.999 rounds to 1, .899 rounds to .9 (when rounding to 2 decimal places)
There was a problem hiding this comment.
Sorry, apparently I forgot how rounding works. Fixed now.
|
lgtm, lint seems to be failing but i don't think its related to this PR |
Rounds down the thumb label value to a maximum of one decimal place, in order to avoid displaying weird rounding errors. Fixes angular#2511.
crisbeto
force-pushed
the
2511/slider-decimal-display
branch
from
06b0f48
to
2740172
Compare
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Rounds down the thumb label value to a maximum of one decimal place, in order to avoid displaying weird rounding errors.
Fixes #2511.