Skip to content

fix(slider): refactor the slider to use percent values for the track … #1663

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

Merged
merged 11 commits into from
Nov 8, 2016
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
"run-sequence": "^1.2.2",
"sass": "^0.5.0",
"strip-ansi": "^3.0.0",
"stylelint": "^6.9.0",
"stylelint": "^7.5.0",
"symlink-or-copy": "^1.0.1",
"ts-node": "^0.7.3",
"tslint": "^3.13.0",
Expand Down
6 changes: 6 additions & 0 deletions src/demo-app/slider/slider-demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,9 @@ <h1>Slider with one-way binding</h1>
<h1>Slider with two-way binding</h1>
<md-slider [(ngModel)]="demo" step="40"></md-slider>
<input [(ngModel)]="demo">

<md-tab-group>
<md-tab label="One">
<md-slider min="1" max="5" value="3"></md-slider>
</md-tab>
</md-tab-group>
76 changes: 76 additions & 0 deletions src/lib/core/coersion/number-property.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import {coerceNumberProperty} from './number-property';


describe('coerceNumberProperty', () => {
it('should coerce undefined to 0 or default', () => {
expect(coerceNumberProperty(undefined)).toBe(0);
expect(coerceNumberProperty(undefined, 111)).toBe(111);
});

it('should coerce null to 0 or default', () => {
expect(coerceNumberProperty(null)).toBe(0);
expect(coerceNumberProperty(null, 111)).toBe(111);
});

it('should coerce true to 0 or default', () => {
expect(coerceNumberProperty(true)).toBe(0);
expect(coerceNumberProperty(true, 111)).toBe(111);
});

it('should coerce false to 0 or default', () => {
expect(coerceNumberProperty(false)).toBe(0);
expect(coerceNumberProperty(false, 111)).toBe(111);

});

it('should coerce the empty string to 0 or default', () => {
expect(coerceNumberProperty('')).toBe(0);
expect(coerceNumberProperty('', 111)).toBe(111);

});

it('should coerce the string "1" to 1', () => {
expect(coerceNumberProperty('1')).toBe(1);
expect(coerceNumberProperty('1', 111)).toBe(1);
});

it('should coerce the string "123.456" to 123.456', () => {
expect(coerceNumberProperty('123.456')).toBe(123.456);
expect(coerceNumberProperty('123.456', 111)).toBe(123.456);
});

it('should coerce the string "-123.456" to -123.456', () => {
expect(coerceNumberProperty('-123.456')).toBe(-123.456);
expect(coerceNumberProperty('-123.456', 111)).toBe(-123.456);
});

it('should coerce an arbitrary string to 0 or default', () => {
expect(coerceNumberProperty('pink')).toBe(0);
expect(coerceNumberProperty('pink', 111)).toBe(111);
});

it('should coerce the number 1 to 1', () => {
expect(coerceNumberProperty(1)).toBe(1);
expect(coerceNumberProperty(1, 111)).toBe(1);
});

it('should coerce the number 123.456 to 123.456', () => {
expect(coerceNumberProperty(123.456)).toBe(123.456);
expect(coerceNumberProperty(123.456, 111)).toBe(123.456);
});

it('should coerce the number -123.456 to -123.456', () => {
expect(coerceNumberProperty(-123.456)).toBe(-123.456);
expect(coerceNumberProperty(-123.456, 111)).toBe(-123.456);
});

it('should coerce an object to 0 or default', () => {
expect(coerceNumberProperty({})).toBe(0);
expect(coerceNumberProperty({}, 111)).toBe(111);
});

it('should coerce an array to 0 or default', () => {
expect(coerceNumberProperty([])).toBe(0);
expect(coerceNumberProperty([], 111)).toBe(111);
});
});
4 changes: 4 additions & 0 deletions src/lib/core/coersion/number-property.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/** Coerces a data-bound value (typically a string) to a number. */
export function coerceNumberProperty(value: any, fallbackValue = 0) {
return isNaN(parseFloat(value as any)) ? fallbackValue : Number(value);
Copy link
Member

@jelbourn jelbourn Nov 7, 2016

Choose a reason for hiding this comment

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

On second look I realize that parseFloat(value) and Number(value) are subtly different in their handling of non-numeric characters. E.g.,

parseFloat('123 boats') == 123
Number('123 boats') === NaN

We should just use one of these- probably Number:

let numberValue = Number(value);
return isNaN(numberValue) ? fallbackValue : numberValue;

}
1 change: 1 addition & 0 deletions src/lib/core/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export * from './animation/animation';

// Coersion
export {coerceBooleanProperty} from './coersion/boolean-property';
export {coerceNumberProperty} from './coersion/number-property';


@NgModule({
Expand Down
3 changes: 1 addition & 2 deletions src/lib/slider/_slider-theme.scss
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@
background-color: md-color($accent);
}

.md-slider-thumb::after {
.md-slider-thumb {
background-color: md-color($accent);
border-color: md-color($accent);
}

.md-slider-thumb-label {
Expand Down
30 changes: 11 additions & 19 deletions src/lib/slider/slider.html
Original file line number Diff line number Diff line change
@@ -1,21 +1,13 @@
<div class="md-slider-wrapper">
<div class="md-slider-container"
[class.md-slider-sliding]="isSliding"
[class.md-slider-active]="isActive"
[class.md-slider-thumb-label-showing]="thumbLabel">
<div class="md-slider-track-container">
<div class="md-slider-track"></div>
<div class="md-slider-track md-slider-track-fill"></div>
<div class="md-slider-tick-container"></div>
<div class="md-slider-last-tick-container"></div>
</div>
<div class="md-slider-thumb-container">
<div class="md-slider-thumb-position">
<div class="md-slider-thumb"></div>
<div class="md-slider-thumb-label">
<span class="md-slider-thumb-label-text">{{value}}</span>
</div>
</div>
<div class="md-slider-track">
<div class="md-slider-track-fill" [style.flexBasis]="trackFillFlexBasis"></div>
<div class="md-slider-ticks-container" [style.marginLeft]="ticksContainerMarginLeft">
<div class="md-slider-ticks" [style.marginLeft]="ticksMarginLeft"
[style.backgroundSize]="ticksBackgroundSize"></div>
</div>
<div class="md-slider-thumb-container">
<div class="md-slider-thumb"></div>
<div class="md-slider-thumb-label">
<span class="md-slider-thumb-label-text">{{value}}</span>
</div>
</div>
</div>
</div>
Loading