Skip to content

Commit 5a734ce

Browse files
committed
Addressed comments.
1 parent c1489bd commit 5a734ce

9 files changed

+125
-95
lines changed

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@
8181
"run-sequence": "^1.2.2",
8282
"sass": "^0.5.0",
8383
"strip-ansi": "^3.0.0",
84-
"stylelint": "^6.9.0",
84+
"stylelint": "^7.5.0",
8585
"symlink-or-copy": "^1.0.1",
8686
"ts-node": "^0.7.3",
8787
"tslint": "^3.13.0",
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import {coerceNumberProperty} from './number-property';
2+
3+
4+
describe('coerceNumberProperty', () => {
5+
it('should coerce undefined to 0 or default', () => {
6+
expect(coerceNumberProperty(undefined)).toBe(0);
7+
expect(coerceNumberProperty(undefined, 111)).toBe(111);
8+
});
9+
10+
it('should coerce null to 0 or default', () => {
11+
expect(coerceNumberProperty(null)).toBe(0);
12+
expect(coerceNumberProperty(null, 111)).toBe(111);
13+
});
14+
15+
it('should coerce true to 0 or default', () => {
16+
expect(coerceNumberProperty(true)).toBe(0);
17+
expect(coerceNumberProperty(true, 111)).toBe(111);
18+
});
19+
20+
it('should coerce false to 0 or default', () => {
21+
expect(coerceNumberProperty(false)).toBe(0);
22+
expect(coerceNumberProperty(false, 111)).toBe(111);
23+
24+
});
25+
26+
it('should coerce the empty string to 0 or default', () => {
27+
expect(coerceNumberProperty('')).toBe(0);
28+
expect(coerceNumberProperty('', 111)).toBe(111);
29+
30+
});
31+
32+
it('should coerce the string "1" to 1', () => {
33+
expect(coerceNumberProperty('1')).toBe(1);
34+
expect(coerceNumberProperty('1', 111)).toBe(1);
35+
});
36+
37+
it('should coerce the string "123.456" to 123.456', () => {
38+
expect(coerceNumberProperty('123.456')).toBe(123.456);
39+
expect(coerceNumberProperty('123.456', 111)).toBe(123.456);
40+
});
41+
42+
it('should coerce the string "-123.456" to -123.456', () => {
43+
expect(coerceNumberProperty('-123.456')).toBe(-123.456);
44+
expect(coerceNumberProperty('-123.456', 111)).toBe(-123.456);
45+
});
46+
47+
it('should coerce an arbitrary string to 0 or default', () => {
48+
expect(coerceNumberProperty('pink')).toBe(0);
49+
expect(coerceNumberProperty('pink', 111)).toBe(111);
50+
});
51+
52+
it('should coerce the number 1 to 1', () => {
53+
expect(coerceNumberProperty(1)).toBe(1);
54+
expect(coerceNumberProperty(1, 111)).toBe(1);
55+
});
56+
57+
it('should coerce the number 123.456 to 123.456', () => {
58+
expect(coerceNumberProperty(123.456)).toBe(123.456);
59+
expect(coerceNumberProperty(123.456, 111)).toBe(123.456);
60+
});
61+
62+
it('should coerce the number -123.456 to -123.456', () => {
63+
expect(coerceNumberProperty(-123.456)).toBe(-123.456);
64+
expect(coerceNumberProperty(-123.456, 111)).toBe(-123.456);
65+
});
66+
67+
it('should coerce an object to 0 or default', () => {
68+
expect(coerceNumberProperty({})).toBe(0);
69+
expect(coerceNumberProperty({}, 111)).toBe(111);
70+
});
71+
72+
it('should coerce an array to 0 or default', () => {
73+
expect(coerceNumberProperty([])).toBe(0);
74+
expect(coerceNumberProperty([], 111)).toBe(111);
75+
});
76+
});
+4
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
/** Coerces a data-bound value (typically a string) to a number. */
2+
export function coerceNumberProperty(value: any, fallbackValue = 0) {
3+
return isNaN(parseFloat(value as any)) ? fallbackValue : Number(value);
4+
}

src/lib/core/core.ts

+1
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ export * from './animation/animation';
8383

8484
// Coersion
8585
export {coerceBooleanProperty} from './coersion/boolean-property';
86+
export {coerceNumberProperty} from './coersion/number-property';
8687

8788

8889
@NgModule({

src/lib/slider/slider.html

+4-4
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
<div class="md-slider-track">
2-
<div class="md-slider-track-fill" [style.flexBasis]="percent * 100 + '%'"></div>
3-
<div class="md-slider-ticks-container" [style.marginLeft]="-tickIntervalPercent / 2 * 100 + '%'">
4-
<div class="md-slider-ticks" [style.marginLeft]="tickIntervalPercent / 2 * 100 + '%'"
5-
[style.backgroundSize]="tickIntervalPercent * 100 + '% 2px'"></div>
2+
<div class="md-slider-track-fill" [style.flexBasis]="percent | percent"></div>
3+
<div class="md-slider-ticks-container" [style.marginLeft]="-halfTickIntervalPercent | percent">
4+
<div class="md-slider-ticks" [style.marginLeft]="halfTickIntervalPercent | percent"
5+
[style.backgroundSize]="(tickIntervalPercent | percent) + ' 2px'"></div>
66
</div>
77
<div class="md-slider-thumb-container">
88
<div class="md-slider-thumb"></div>

src/lib/slider/slider.scss

+2-2
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ md-slider {
6969
.md-slider-ticks {
7070
background: repeating-linear-gradient(to right, $md-slider-tick-color,
7171
$md-slider-tick-color $md-slider-tick-size, transparent 0, transparent) repeat;
72-
/* Firefox doesn't draw the gradient correctly with 'to right'
73-
(see https://bugzilla.mozilla.org/show_bug.cgi?id=1314319). */
72+
// Firefox doesn't draw the gradient correctly with 'to right'
73+
// (see https://bugzilla.mozilla.org/show_bug.cgi?id=1314319).
7474
background: -moz-repeating-linear-gradient(0.0001deg, $md-slider-tick-color,
7575
$md-slider-tick-color $md-slider-tick-size, transparent 0, transparent) repeat;
7676
height: $md-slider-track-thickness;

src/lib/slider/slider.spec.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ describe('MdSlider', () => {
126126
expect(sliderNativeElement.classList).toContain('md-slider-active');
127127

128128
// Call the `onBlur` handler directly because we cannot simulate a focus event in unit tests.
129-
sliderInstance.onBlur();
129+
sliderInstance._onBlur();
130130
fixture.detectChanges();
131131

132132
expect(sliderNativeElement.classList).not.toContain('md-slider-active');

src/lib/slider/slider.ts

+35-86
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@ import {
99
} from '@angular/core';
1010
import {NG_VALUE_ACCESSOR, ControlValueAccessor, FormsModule} from '@angular/forms';
1111
import {HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
12-
import {MdGestureConfig, coerceBooleanProperty} from '../core';
12+
import {MdGestureConfig, coerceBooleanProperty, coerceNumberProperty} from '../core';
1313
import {Input as HammerInput} from 'hammerjs';
14+
import {CommonModule} from '@angular/common';
1415

1516
/**
1617
* Visually, a 30px separation between tick marks looks best. This is very subjective but it is
@@ -33,25 +34,21 @@ export const MD_SLIDER_VALUE_ACCESSOR: any = {
3334
selector: 'md-slider',
3435
providers: [MD_SLIDER_VALUE_ACCESSOR],
3536
host: {
36-
'(blur)': 'onBlur()',
37-
'(click)': 'onClick($event)',
38-
'(mouseenter)': 'onMouseenter()',
39-
'(slide)': 'onSlide($event)',
40-
'(slideend)': 'onSlideEnd()',
41-
'(slidestart)': 'onSlideStart($event)',
42-
'(window:resize)': 'onResize()',
43-
37+
'(blur)': '_onBlur()',
38+
'(click)': '_onClick($event)',
39+
'(mouseenter)': '_onMouseenter()',
40+
'(slide)': '_onSlide($event)',
41+
'(slideend)': '_onSlideEnd()',
42+
'(slidestart)': '_onSlideStart($event)',
4443
'tabindex': '0',
45-
4644
'[attr.aria-disabled]': 'disabled',
4745
'[attr.aria-valuemax]': 'max',
4846
'[attr.aria-valuemin]': 'min',
4947
'[attr.aria-valuenow]': 'value',
50-
51-
'[class.md-slider-active]': 'isActive',
48+
'[class.md-slider-active]': '_isActive',
5249
'[class.md-slider-disabled]': 'disabled',
5350
'[class.md-slider-has-ticks]': 'tickInterval',
54-
'[class.md-slider-sliding]': 'isSliding',
51+
'[class.md-slider-sliding]': '_isSliding',
5552
'[class.md-slider-thumb-label-showing]': 'thumbLabel',
5653
},
5754
templateUrl: 'slider.html',
@@ -87,32 +84,21 @@ export class MdSlider implements ControlValueAccessor {
8784
/**
8885
* Whether or not the thumb is sliding.
8986
* Used to determine if there should be a transition for the thumb and fill track.
90-
* TODO: internal
9187
*/
92-
isSliding: boolean = false;
88+
_isSliding: boolean = false;
9389

9490
/**
9591
* Whether or not the slider is active (clicked or sliding).
9692
* Used to shrink and grow the thumb as according to the Material Design spec.
97-
* TODO: internal
9893
*/
99-
isActive: boolean = false;
94+
_isActive: boolean = false;
10095

10196
/** The values at which the thumb will snap. */
10297
private _step: number = 1;
10398

10499
@Input()
105-
get step() {
106-
return this._step;
107-
}
108-
set step(v) {
109-
// Only set the value to a valid number. v is casted to an any as we know it will come in as a
110-
// string but it is labeled as a number which causes parseFloat to not accept it.
111-
if (isNaN(parseFloat(<any> v))) {
112-
return;
113-
}
114-
this._step = Number(v);
115-
}
100+
get step() { return this._step; }
101+
set step(v) { this._step = coerceNumberProperty(v, this._step); }
116102

117103
/**
118104
* How often to show ticks. Relative to the step so that a tick always appears on a step.
@@ -122,23 +108,15 @@ export class MdSlider implements ControlValueAccessor {
122108

123109
@Input('tick-interval')
124110
get tickInterval() { return this._tickInterval; }
125-
set tickInterval(v: 'auto' | number) {
126-
// Only set the tickInterval to a valid number. v is casted to an any as we know it will come
127-
// in as a string but it is labeled as a number which causes parseInt to not accept it.
128-
if (v == 'auto') {
129-
this._tickInterval = v;
130-
} else {
131-
let intV = parseInt(<any> v);
132-
if (!isNaN(intV)) {
133-
this._tickInterval = intV;
134-
}
135-
}
111+
set tickInterval(v) {
112+
this._tickInterval = (v == 'auto') ? v : coerceNumberProperty(v, <number>this._tickInterval);
136113
}
137114

138115
/** The size of a tick interval as a percentage of the size of the track. */
139116
private _tickIntervalPercent: number = 0;
140117

141118
get tickIntervalPercent() { return this._tickIntervalPercent; }
119+
get halfTickIntervalPercent() { return this._tickIntervalPercent / 2; }
142120

143121
/** The percentage of the slider that coincides with the value. */
144122
private _percent: number = 0;
@@ -157,13 +135,7 @@ export class MdSlider implements ControlValueAccessor {
157135
return this._value;
158136
}
159137
set value(v: number) {
160-
// Only set the value to a valid number. v is casted to an any as we know it will come in as a
161-
// string but it is labeled as a number which causes parseFloat to not accept it.
162-
if (isNaN(parseFloat(<any> v))) {
163-
return;
164-
}
165-
166-
this._value = Number(v);
138+
this._value = coerceNumberProperty(v, this._value);
167139
this._percent = this._calculatePercentage(this._value);
168140
this._controlValueAccessorChangeFn(this._value);
169141
}
@@ -176,14 +148,7 @@ export class MdSlider implements ControlValueAccessor {
176148
return this._min;
177149
}
178150
set min(v: number) {
179-
// Only set the value to a valid number. v is casted to an any as we know it will come in as a
180-
// string but it is labeled as a number which causes parseFloat to not accept it.
181-
if (isNaN(parseFloat(<any> v))) {
182-
return;
183-
}
184-
185-
// This has to be forced as a number to handle the math later.
186-
this._min = Number(v);
151+
this._min = coerceNumberProperty(v, this._min);
187152

188153
// If the value wasn't explicitly set by the user, set it to the min.
189154
if (this._value === null) {
@@ -200,38 +165,37 @@ export class MdSlider implements ControlValueAccessor {
200165
return this._max;
201166
}
202167
set max(v: number) {
203-
this._max = Number(v);
168+
this._max = coerceNumberProperty(v, this._max);
204169
this._percent = this._calculatePercentage(this.value);
205170
}
206171

207172
constructor(elementRef: ElementRef) {
208173
this._renderer = new SliderRenderer(elementRef);
209174
}
210175

211-
/** TODO: internal */
212-
onMouseenter() {
176+
_onMouseenter() {
213177
if (this.disabled) {
214178
return;
215179
}
216180

181+
// We save the dimensions of the slider here so we can use them to update the spacing of the
182+
// ticks and determine where on the slider click and slide events happen.
217183
this._sliderDimensions = this._renderer.getSliderDimensions();
218184
this._updateTickIntervalPercent();
219185
}
220186

221-
/** TODO: internal */
222-
onClick(event: MouseEvent) {
187+
_onClick(event: MouseEvent) {
223188
if (this.disabled) {
224189
return;
225190
}
226191

227-
this.isActive = true;
228-
this.isSliding = false;
192+
this._isActive = true;
193+
this._isSliding = false;
229194
this._renderer.addFocus();
230195
this._updateValueFromPosition(event.clientX);
231196
}
232197

233-
/** TODO: internal */
234-
onSlide(event: HammerInput) {
198+
_onSlide(event: HammerInput) {
235199
if (this.disabled) {
236200
return;
237201
}
@@ -241,36 +205,24 @@ export class MdSlider implements ControlValueAccessor {
241205
this._updateValueFromPosition(event.center.x);
242206
}
243207

244-
/** TODO: internal */
245-
onSlideStart(event: HammerInput) {
208+
_onSlideStart(event: HammerInput) {
246209
if (this.disabled) {
247210
return;
248211
}
249212

250213
event.preventDefault();
251-
this.isSliding = true;
252-
this.isActive = true;
214+
this._isSliding = true;
215+
this._isActive = true;
253216
this._renderer.addFocus();
254217
this._updateValueFromPosition(event.center.x);
255218
}
256219

257-
/** TODO: internal */
258-
onSlideEnd() {
259-
this.isSliding = false;
260-
}
261-
262-
/** TODO: internal */
263-
onResize() {
264-
// Consider the slider to be sliding during resize to prevent animation.
265-
this.isSliding = true;
266-
setTimeout(() => {
267-
this.isSliding = false;
268-
}, 0);
220+
_onSlideEnd() {
221+
this._isSliding = false;
269222
}
270223

271-
/** TODO: internal */
272-
onBlur() {
273-
this.isActive = false;
224+
_onBlur() {
225+
this._isActive = false;
274226
this.onTouched();
275227
}
276228

@@ -337,23 +289,20 @@ export class MdSlider implements ControlValueAccessor {
337289

338290
/**
339291
* Implemented as part of ControlValueAccessor.
340-
* TODO: internal
341292
*/
342293
writeValue(value: any) {
343294
this.value = value;
344295
}
345296

346297
/**
347298
* Implemented as part of ControlValueAccessor.
348-
* TODO: internal
349299
*/
350300
registerOnChange(fn: (value: any) => void) {
351301
this._controlValueAccessorChangeFn = fn;
352302
}
353303

354304
/**
355305
* Implemented as part of ControlValueAccessor.
356-
* TODO: internal
357306
*/
358307
registerOnTouched(fn: any) {
359308
this.onTouched = fn;
@@ -398,7 +347,7 @@ export class SliderRenderer {
398347

399348

400349
@NgModule({
401-
imports: [FormsModule],
350+
imports: [FormsModule, CommonModule],
402351
exports: [MdSlider],
403352
declarations: [MdSlider],
404353
providers: [

0 commit comments

Comments
 (0)