Skip to content

Commit 5c7674a

Browse files
authored
fix(material/slider): slider tx imprecision (#28283)
* Fixes a bug where the slider's min and max value could go beyond the first and last tick marks. This caused the slider thumb to never truly line up with the tick marks except at the exact center of the slider track.
1 parent 1fe1f69 commit 5c7674a

File tree

5 files changed

+44
-41
lines changed

5 files changed

+44
-41
lines changed

src/material/slider/slider-input.ts

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA
130130
*/
131131
get translateX(): number {
132132
if (this._slider.min >= this._slider.max) {
133-
this._translateX = 0;
133+
this._translateX = this._tickMarkOffset;
134134
return this._translateX;
135135
}
136136
if (this._translateX === undefined) {
@@ -216,6 +216,9 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA
216216
/** The radius of a native html slider's knob. */
217217
_knobRadius: number = 8;
218218

219+
/** The distance in px from the start of the slider track to the first tick mark. */
220+
_tickMarkOffset = 3;
221+
219222
/** Whether user's cursor is currently in a mouse down state on the input. */
220223
_isActive: boolean = false;
221224

@@ -490,15 +493,22 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA
490493
}
491494

492495
_clamp(v: number): number {
493-
return Math.max(Math.min(v, this._slider._cachedWidth), 0);
496+
const min = this._tickMarkOffset;
497+
const max = this._slider._cachedWidth - this._tickMarkOffset;
498+
return Math.max(Math.min(v, max), min);
494499
}
495500

496501
_calcTranslateXByValue(): number {
497502
if (this._slider._isRtl) {
498-
return (1 - this.percentage) * this._slider._cachedWidth;
503+
return (
504+
(1 - this.percentage) * (this._slider._cachedWidth - this._tickMarkOffset * 2) +
505+
this._tickMarkOffset
506+
);
499507
}
500-
const tickMarkOffset = 3; // The spaces before & after the start & end tick marks.
501-
return this.percentage * (this._slider._cachedWidth - tickMarkOffset * 2) + tickMarkOffset;
508+
return (
509+
this.percentage * (this._slider._cachedWidth - this._tickMarkOffset * 2) +
510+
this._tickMarkOffset
511+
);
502512
}
503513

504514
_calcTranslateXByPointerEvent(event: PointerEvent): number {
@@ -509,19 +519,18 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA
509519
* Used to set the slider width to the correct
510520
* dimensions while the user is dragging.
511521
*/
512-
_updateWidthActive(): void {
513-
this._hostElement.style.padding = `0 ${this._slider._inputPadding}px`;
514-
this._hostElement.style.width = `calc(100% + ${this._slider._inputPadding}px)`;
515-
}
522+
_updateWidthActive(): void {}
516523

517524
/**
518525
* Sets the slider input to disproportionate dimensions to allow for touch
519526
* events to be captured on touch devices.
520527
*/
521528
_updateWidthInactive(): void {
522-
this._hostElement.style.padding = '0px';
523-
this._hostElement.style.width = 'calc(100% + 48px)';
524-
this._hostElement.style.left = '-24px';
529+
this._hostElement.style.padding = `0 ${this._slider._inputPadding}px`;
530+
this._hostElement.style.width = `calc(100% + ${
531+
this._slider._inputPadding - this._tickMarkOffset * 2
532+
}px)`;
533+
this._hostElement.style.left = `-${this._slider._rippleRadius - this._tickMarkOffset}px`;
525534
}
526535

527536
_updateThumbUIByValue(options?: {withAnimation: boolean}): void {
@@ -617,7 +626,7 @@ export class MatSliderRangeThumb extends MatSliderThumb implements _MatSliderRan
617626
if (!this._isLeftThumb && sibling) {
618627
return sibling.translateX;
619628
}
620-
return 0;
629+
return this._tickMarkOffset;
621630
}
622631

623632
/**
@@ -629,7 +638,7 @@ export class MatSliderRangeThumb extends MatSliderThumb implements _MatSliderRan
629638
if (this._isLeftThumb && sibling) {
630639
return sibling.translateX;
631640
}
632-
return this._slider._cachedWidth;
641+
return this._slider._cachedWidth - this._tickMarkOffset;
633642
}
634643

635644
_setIsLeftThumb(): void {
@@ -725,7 +734,8 @@ export class MatSliderRangeThumb extends MatSliderThumb implements _MatSliderRan
725734

726735
override _updateWidthActive(): void {
727736
const minWidth = this._slider._rippleRadius * 2 - this._slider._inputPadding * 2;
728-
const maxWidth = this._slider._cachedWidth + this._slider._inputPadding - minWidth;
737+
const maxWidth =
738+
this._slider._cachedWidth + this._slider._inputPadding - minWidth - this._tickMarkOffset * 2;
729739
const percentage =
730740
this._slider.min < this._slider.max
731741
? (this.max - this.min) / (this._slider.max - this._slider.min)
@@ -740,7 +750,7 @@ export class MatSliderRangeThumb extends MatSliderThumb implements _MatSliderRan
740750
if (!sibling) {
741751
return;
742752
}
743-
const maxWidth = this._slider._cachedWidth;
753+
const maxWidth = this._slider._cachedWidth - this._tickMarkOffset * 2;
744754
const midValue = this._isEndThumb
745755
? this.value - (this.value - sibling.value) / 2
746756
: this.value + (sibling.value - this.value) / 2;
@@ -768,11 +778,11 @@ export class MatSliderRangeThumb extends MatSliderThumb implements _MatSliderRan
768778
this._hostElement.style.padding = '0px';
769779

770780
if (this._isLeftThumb) {
771-
this._hostElement.style.left = '-24px';
781+
this._hostElement.style.left = `-${this._slider._rippleRadius - this._tickMarkOffset}px`;
772782
this._hostElement.style.right = 'auto';
773783
} else {
774784
this._hostElement.style.left = 'auto';
775-
this._hostElement.style.right = '-24px';
785+
this._hostElement.style.right = `-${this._slider._rippleRadius - this._tickMarkOffset}px`;
776786
}
777787
}
778788

src/material/slider/slider-interface.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -121,18 +121,6 @@ export interface _MatSlider {
121121
*/
122122
_inputPadding: number;
123123

124-
/**
125-
* The offset represents left most translateX of the slider knob. Inversely,
126-
* (slider width - offset) = the right most translateX of the slider knob.
127-
*
128-
* Note:
129-
* * The native slider knob differs from the visual slider. It's knob cannot slide past
130-
* the end of the track AT ALL.
131-
* * The visual slider knob CAN slide past the end of the track slightly. It's knob can slide
132-
* past the end of the track such that it's center lines up with the end of the track.
133-
*/
134-
_inputOffset: number;
135-
136124
/** The radius of the visual slider's ripple. */
137125
_rippleRadius: number;
138126

src/material/slider/slider.spec.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,23 @@ describe('MDC-based MatSlider', () => {
6060
expect(input.max).withContext('max').toBe(max);
6161
expect(input.value).withContext('value').toBe(value);
6262

63-
// Note: This ±6 is here to account for the slight shift of the slider
64-
// thumb caused by the tick marks being 3px away from the track start
65-
// and end.
63+
// The discrepancy between the "ideal" and "actual" translateX comes from
64+
// the 3px offset from the start & end of the slider track to the first
65+
// and last tick marks.
6666
//
67-
// This check is meant to ensure the "ideal" estimate is within 3px of the
68-
// actual slider thumb position.
69-
expect(input.translateX - 6 < translateX && input.translateX + 6 > translateX)
67+
// The "actual" translateX is calculated based on a slider that is 6px
68+
// smaller than the width of the slider. Using this "actual" translateX in
69+
// tests would make it even more difficult than it already is to tell if
70+
// the translateX is off, so we abstract things in here so tests can be
71+
// more intuitive.
72+
//
73+
// The most clear way to compare the two tx's is to just turn them into
74+
// percentages by dividing by their (total height) / 100.
75+
const idealTXPercentage = Math.round(translateX / 3);
76+
const actualTXPercentage = Math.round((input.translateX - 3) / 2.94);
77+
expect(actualTXPercentage)
7078
.withContext(`translateX: ${input.translateX} should be close to ${translateX}`)
71-
.toBeTrue();
79+
.toBe(idealTXPercentage);
7280
if (step !== undefined) {
7381
expect(input.step).withContext('step').toBe(step);
7482
}

src/material/slider/slider.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,6 @@ export class MatSlider implements AfterViewInit, OnDestroy, _MatSlider {
408408
_knobRadius: number = 8;
409409

410410
_inputPadding: number;
411-
_inputOffset: number;
412411

413412
ngAfterViewInit(): void {
414413
if (this._platform.isBrowser) {
@@ -431,7 +430,6 @@ export class MatSlider implements AfterViewInit, OnDestroy, _MatSlider {
431430
const thumb = this._getThumb(_MatThumb.END);
432431
this._rippleRadius = thumb._ripple.radius;
433432
this._inputPadding = this._rippleRadius - this._knobRadius;
434-
this._inputOffset = this._knobRadius;
435433

436434
this._isRange
437435
? this._initUIRange(eInput as _MatSliderRangeThumb, sInput as _MatSliderRangeThumb)

tools/public_api_guard/material/slider.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,6 @@ export class MatSlider implements AfterViewInit, OnDestroy, _MatSlider {
5252
_hasAnimation: boolean;
5353
_input: _MatSliderThumb;
5454
// (undocumented)
55-
_inputOffset: number;
56-
// (undocumented)
5755
_inputPadding: number;
5856
_inputs: QueryList<_MatSliderRangeThumb>;
5957
_isCursorOnSliderThumb(event: PointerEvent, rect: DOMRect): boolean;
@@ -271,6 +269,7 @@ export class MatSliderThumb implements _MatSliderThumb, OnDestroy, ControlValueA
271269
get step(): number;
272270
set step(v: number);
273271
thumbPosition: _MatThumb;
272+
_tickMarkOffset: number;
274273
get translateX(): number;
275274
set translateX(v: number);
276275
// (undocumented)

0 commit comments

Comments
 (0)