Skip to content

Commit cd25296

Browse files
committed
fix(material-experimental/mdc-slider): code review changes
* make ripple refs private and check DOM directly in unit tests
1 parent a55ceca commit cd25296

File tree

2 files changed

+46
-47
lines changed

2 files changed

+46
-47
lines changed

src/material-experimental/mdc-slider/slider.spec.ts

Lines changed: 43 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@
88

99
import {dispatchMouseEvent, dispatchPointerEvent} from '@angular/cdk/testing/private';
1010
import {Component, DebugElement, Type} from '@angular/core';
11-
import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing';
12-
import {RippleRef, RippleState} from '@angular/material/core';
11+
import {ComponentFixture, fakeAsync, TestBed, tick, waitForAsync} from '@angular/core/testing';
1312
import {By} from '@angular/platform-browser';
1413
import {Thumb} from '@material/slider';
1514
import {MatSliderModule} from './module';
@@ -165,9 +164,9 @@ describe('MDC-based MatSlider' , () => {
165164
thumbY = thumbDimensions.top - (thumbDimensions.height / 2);
166165
}));
167166

168-
function isRippleVisible(rippleRef: RippleRef) {
169-
return rippleRef?.state === RippleState.FADING_IN
170-
|| rippleRef?.state === RippleState.VISIBLE;
167+
function isRippleVisible(selector: string) {
168+
tick(500);
169+
return !!document.querySelector(`.mat-mdc-slider-${selector}-ripple`);
171170
}
172171

173172
function blur() {
@@ -190,83 +189,83 @@ describe('MDC-based MatSlider' , () => {
190189
dispatchPointerEvent(thumbElement, 'pointerup', thumbX, thumbY);
191190
}
192191

193-
it('should show the hover ripple on mouseenter', () => {
194-
expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(false);
192+
it('should show the hover ripple on mouseenter', fakeAsync(() => {
193+
expect(isRippleVisible('hover')).toBe(false);
195194
mouseenter();
196-
expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(true);
197-
});
195+
expect(isRippleVisible('hover')).toBe(true);
196+
}));
198197

199-
it('should hide the hover ripple on mouseleave', () => {
198+
it('should hide the hover ripple on mouseleave', fakeAsync(() => {
200199
mouseenter();
201200
mouseleave();
202-
expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(false);
203-
});
201+
expect(isRippleVisible('hover')).toBe(false);
202+
}));
204203

205-
it('should show the focus ripple on pointerdown', () => {
206-
expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(false);
204+
it('should show the focus ripple on pointerdown', fakeAsync(() => {
205+
expect(isRippleVisible('focus')).toBe(false);
207206
pointerdown();
208-
expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(true);
209-
});
207+
expect(isRippleVisible('focus')).toBe(true);
208+
}));
210209

211-
it('should continue to show the focus ripple on pointerup', () => {
210+
it('should continue to show the focus ripple on pointerup', fakeAsync(() => {
212211
pointerdown();
213212
pointerup();
214-
expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(true);
215-
});
213+
expect(isRippleVisible('focus')).toBe(true);
214+
}));
216215

217-
it('should hide the focus ripple on blur', () => {
216+
it('should hide the focus ripple on blur', fakeAsync(() => {
218217
pointerdown();
219218
pointerup();
220219
blur();
221-
expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(false);
222-
});
220+
expect(isRippleVisible('focus')).toBe(false);
221+
}));
223222

224-
it('should show the active ripple on pointerdown', () => {
225-
expect(isRippleVisible(thumbInstance._activeRippleRef)).toBe(false);
223+
it('should show the active ripple on pointerdown', fakeAsync(() => {
224+
expect(isRippleVisible('active')).toBe(false);
226225
pointerdown();
227-
expect(isRippleVisible(thumbInstance._activeRippleRef)).toBe(true);
228-
});
226+
expect(isRippleVisible('active')).toBe(true);
227+
}));
229228

230-
it('should hide the active ripple on pointerup', () => {
229+
it('should hide the active ripple on pointerup', fakeAsync(() => {
231230
pointerdown();
232231
pointerup();
233-
expect(isRippleVisible(thumbInstance._activeRippleRef)).toBe(false);
234-
});
232+
expect(isRippleVisible('active')).toBe(false);
233+
}));
235234

236235
// Edge cases.
237236

238-
it('should not show the hover ripple if the thumb is already focused', () => {
237+
it('should not show the hover ripple if the thumb is already focused', fakeAsync(() => {
239238
pointerdown();
240239
mouseenter();
241-
expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(false);
242-
});
240+
expect(isRippleVisible('hover')).toBe(false);
241+
}));
243242

244-
it('should hide the hover ripple if the thumb is focused', () => {
243+
it('should hide the hover ripple if the thumb is focused', fakeAsync(() => {
245244
mouseenter();
246245
pointerdown();
247-
expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(false);
248-
});
246+
expect(isRippleVisible('hover')).toBe(false);
247+
}));
249248

250-
it('should not hide the focus ripple if the thumb is pressed', () => {
249+
it('should not hide the focus ripple if the thumb is pressed', fakeAsync(() => {
251250
pointerdown();
252251
blur();
253-
expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(true);
254-
});
252+
expect(isRippleVisible('focus')).toBe(true);
253+
}));
255254

256-
it('should not hide the hover ripple on blur if the cursor is thumb being hovered', () => {
255+
it('should not hide the hover ripple on blur if the thumb is hovered', fakeAsync(() => {
257256
mouseenter();
258257
pointerdown();
259258
pointerup();
260259
blur();
261-
expect(isRippleVisible(thumbInstance._hoverRippleRef)).toBe(true);
262-
});
260+
expect(isRippleVisible('hover')).toBe(true);
261+
}));
263262

264-
it('should hide the focus ripple on drag end if the thumb already lost focus', () => {
263+
it('should hide the focus ripple on drag end if the thumb already lost focus', fakeAsync(() => {
265264
pointerdown();
266265
blur();
267266
pointerup();
268-
expect(isRippleVisible(thumbInstance._focusRippleRef)).toBe(false);
269-
});
267+
expect(isRippleVisible('focus')).toBe(false);
268+
}));
270269
});
271270
});
272271

src/material-experimental/mdc-slider/slider.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,13 @@ export class MatSliderVisualThumb implements AfterViewInit, OnDestroy {
9595
private _sliderInput: MatSliderThumb;
9696

9797
/** The RippleRef for the slider thumbs hover state. */
98-
_hoverRippleRef: RippleRef;
98+
private _hoverRippleRef: RippleRef;
9999

100100
/** The RippleRef for the slider thumbs focus state. */
101-
_focusRippleRef: RippleRef;
101+
private _focusRippleRef: RippleRef;
102102

103103
/** The RippleRef for the slider thumbs active state. */
104-
_activeRippleRef: RippleRef;
104+
private _activeRippleRef: RippleRef;
105105

106106
/** Whether the slider thumb is currently being pressed. */
107107
private _isActive: boolean = false;

0 commit comments

Comments
 (0)