From 4d51158b1ac2320d775355165ddb080107838e1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Wed, 8 Jun 2016 22:50:52 -0700 Subject: [PATCH] fix(animations): ensure the web-animations driver converts style props to camel-case The web animations API now requires that all styles are converted to camel case. Chrome has already made this breaking change and hyphenated styles are not functional anymore. Closes #9111 Closes #9112 --- .../src/dom/web_animations_driver.ts | 94 ++++++++++--------- .../test/dom/web_animations_driver_spec.ts | 78 +++++++++++++++ .../test/dom/web_animations_player_spec.ts | 35 +------ .../testing/mock_dom_animate_player.ts | 35 +++++++ 4 files changed, 166 insertions(+), 76 deletions(-) create mode 100644 modules/@angular/platform-browser/test/dom/web_animations_driver_spec.ts create mode 100644 modules/@angular/platform-browser/testing/mock_dom_animate_player.ts diff --git a/modules/@angular/platform-browser/src/dom/web_animations_driver.ts b/modules/@angular/platform-browser/src/dom/web_animations_driver.ts index 216f8814fa..f86ac2291a 100644 --- a/modules/@angular/platform-browser/src/dom/web_animations_driver.ts +++ b/modules/@angular/platform-browser/src/dom/web_animations_driver.ts @@ -5,6 +5,8 @@ import {StringMapWrapper} from '../facade/collection'; import {StringWrapper, isNumber, isPresent} from '../facade/lang'; import {getDOM} from './dom_adapter'; +import {DomAnimatePlayer} from './dom_animate_player'; +import {dashCaseToCamelCase} from './util'; import {WebAnimationsPlayer} from './web_animations_player'; export class WebAnimationsDriver implements AnimationDriver { @@ -13,7 +15,7 @@ export class WebAnimationsDriver implements AnimationDriver { duration: number, delay: number, easing: string): AnimationPlayer { var anyElm = element; - var formattedSteps: any[] /** TODO #9100 */ = []; + var formattedSteps: {[key: string]: string | number}[] = []; var startingStyleLookup: {[key: string]: string | number} = {}; if (isPresent(startingStyles) && startingStyles.styles.length > 0) { startingStyleLookup = _populateStyles(anyElm, startingStyles, {}); @@ -23,7 +25,7 @@ export class WebAnimationsDriver implements AnimationDriver { keyframes.forEach((keyframe: AnimationKeyframe) => { let data = _populateStyles(anyElm, keyframe.styles, startingStyleLookup); - (data as any /** TODO #9100 */)['offset'] = keyframe.offset; + data['offset'] = keyframe.offset; formattedSteps.push(data); }); @@ -33,44 +35,52 @@ export class WebAnimationsDriver implements AnimationDriver { // start/end values is suitable enough for the web-animations API if (formattedSteps.length == 1) { var start = formattedSteps[0]; - start.offset = null; + start['offset'] = null; formattedSteps = [start, start]; } - var player = anyElm.animate( - formattedSteps, + var player = this._triggerWebAnimation( + anyElm, formattedSteps, {'duration': duration, 'delay': delay, 'easing': easing, 'fill': 'forwards'}); return new WebAnimationsPlayer(player, duration); } + + /** @internal */ + _triggerWebAnimation(elm: any, keyframes: any[], options: any): DomAnimatePlayer { + return elm.animate(keyframes, options); + } } function _populateStyles( - element: any, styles: AnimationStyles, defaultStyles: {[key: string]: string | number}) { - var data = {}; + element: any, styles: AnimationStyles, + defaultStyles: {[key: string]: string | number}): {[key: string]: string | number} { + var data: {[key: string]: string | number} = {}; styles.styles.forEach((entry) => { - StringMapWrapper.forEach(entry, (val: any /** TODO #9100 */, prop: any /** TODO #9100 */) => { - (data as any /** TODO #9100 */)[prop] = val == AUTO_STYLE ? - _computeStyle(element, prop) : - val.toString() + _resolveStyleUnit(val, prop); + StringMapWrapper.forEach(entry, (val: any, prop: string) => { + var formattedProp = dashCaseToCamelCase(prop); + data[formattedProp] = val == AUTO_STYLE ? + _computeStyle(element, formattedProp) : + val.toString() + _resolveStyleUnit(val, prop, formattedProp); }); }); - StringMapWrapper.forEach( - defaultStyles, (value: any /** TODO #9100 */, prop: any /** TODO #9100 */) => { - if (!isPresent((data as any /** TODO #9100 */)[prop])) { - (data as any /** TODO #9100 */)[prop] = value; - } - }); + StringMapWrapper.forEach(defaultStyles, (value: string, prop: string) => { + if (!isPresent(data[prop])) { + data[prop] = value; + } + }); return data; } -function _resolveStyleUnit(val: string | number, prop: string): string { +function _resolveStyleUnit( + val: string | number, userProvidedProp: string, formattedProp: string): string { var unit = ''; - if (_isPixelDimensionStyle(prop) && val != 0 && val != '0') { + if (_isPixelDimensionStyle(formattedProp) && val != 0 && val != '0') { if (isNumber(val)) { unit = 'px'; } else if (_findDimensionalSuffix(val.toString()).length == 0) { - throw new BaseException('Please provide a CSS unit value for ' + prop + ':' + val); + throw new BaseException( + 'Please provide a CSS unit value for ' + userProvidedProp + ':' + val); } } return unit; @@ -93,32 +103,32 @@ function _isPixelDimensionStyle(prop: string): boolean { switch (prop) { case 'width': case 'height': - case 'min-width': - case 'min-height': - case 'max-width': - case 'max-height': + case 'minWidth': + case 'minHeight': + case 'maxWidth': + case 'maxHeight': case 'left': case 'top': case 'bottom': case 'right': - case 'font-size': - case 'outline-width': - case 'outline-offset': - case 'padding-top': - case 'padding-left': - case 'padding-bottom': - case 'padding-right': - case 'margin-top': - case 'margin-left': - case 'margin-bottom': - case 'margin-right': - case 'border-radius': - case 'border-width': - case 'border-top-width': - case 'border-left-width': - case 'border-right-width': - case 'border-bottom-width': - case 'text-indent': + case 'fontSize': + case 'outlineWidth': + case 'outlineOffset': + case 'paddingTop': + case 'paddingLeft': + case 'paddingBottom': + case 'paddingRight': + case 'marginTop': + case 'marginLeft': + case 'marginBottom': + case 'marginRight': + case 'borderRadius': + case 'borderWidth': + case 'borderTopWidth': + case 'borderLeftWidth': + case 'borderRightWidth': + case 'borderBottomWidth': + case 'textIndent': return true; default: diff --git a/modules/@angular/platform-browser/test/dom/web_animations_driver_spec.ts b/modules/@angular/platform-browser/test/dom/web_animations_driver_spec.ts new file mode 100644 index 0000000000..d88651e409 --- /dev/null +++ b/modules/@angular/platform-browser/test/dom/web_animations_driver_spec.ts @@ -0,0 +1,78 @@ +import {AsyncTestCompleter, beforeEach, beforeEachProviders, ddescribe, describe, expect, iit, inject, it, xdescribe, xit} from '@angular/core/testing/testing_internal'; +import {el} from '@angular/platform-browser/testing'; + +import {AnimationKeyframe, AnimationStyles} from '../../core_private'; +import {DomAnimatePlayer} from '../../src/dom/dom_animate_player'; +import {WebAnimationsDriver} from '../../src/dom/web_animations_driver'; +import {MockDomAnimatePlayer} from '../../testing/mock_dom_animate_player'; + +class ExtendedWebAnimationsDriver extends WebAnimationsDriver { + public log: {[key: string]: any}[] = []; + + constructor() { super(); } + + _triggerWebAnimation(elm: any, keyframes: any[], options: any): DomAnimatePlayer { + this.log.push({'elm': elm, 'keyframes': keyframes, 'options': options}); + return new MockDomAnimatePlayer(); + } +} + +function _makeStyles(styles: {[key: string]: string | number}): AnimationStyles { + return new AnimationStyles([styles]); +} + +function _makeKeyframe( + offset: number, styles: {[key: string]: string | number}): AnimationKeyframe { + return new AnimationKeyframe(offset, _makeStyles(styles)); +} + +export function main() { + describe('WebAnimationsDriver', () => { + var driver: ExtendedWebAnimationsDriver; + var elm: HTMLElement; + beforeEach(() => { + driver = new ExtendedWebAnimationsDriver(); + elm = el('
'); + }); + + it('should convert all styles to camelcase', () => { + var startingStyles = _makeStyles({'border-top-right': '40px'}); + var styles = [ + _makeKeyframe(0, {'max-width': '100px', 'height': '200px'}), + _makeKeyframe(1, {'font-size': '555px'}) + ]; + + driver.animate(elm, startingStyles, styles, 0, 0, 'linear'); + var details = driver.log.pop(); + var startKeyframe = details['keyframes'][0]; + var firstKeyframe = details['keyframes'][1]; + var lastKeyframe = details['keyframes'][2]; + + expect(startKeyframe['borderTopRight']).toEqual('40px'); + + expect(firstKeyframe['maxWidth']).toEqual('100px'); + expect(firstKeyframe['max-width']).toBeFalsy(); + expect(firstKeyframe['height']).toEqual('200px'); + + expect(lastKeyframe['fontSize']).toEqual('555px'); + expect(lastKeyframe['font-size']).toBeFalsy(); + }); + + it('should auto prefix numeric properties with a `px` value', () => { + var startingStyles = _makeStyles({'borderTopWidth': 40}); + var styles = [_makeKeyframe(0, {'font-size': 100}), _makeKeyframe(1, {'height': '555em'})]; + + driver.animate(elm, startingStyles, styles, 0, 0, 'linear'); + var details = driver.log.pop(); + var startKeyframe = details['keyframes'][0]; + var firstKeyframe = details['keyframes'][1]; + var lastKeyframe = details['keyframes'][2]; + + expect(startKeyframe['borderTopWidth']).toEqual('40px'); + + expect(firstKeyframe['fontSize']).toEqual('100px'); + + expect(lastKeyframe['height']).toEqual('555em'); + }); + }); +} diff --git a/modules/@angular/platform-browser/test/dom/web_animations_player_spec.ts b/modules/@angular/platform-browser/test/dom/web_animations_player_spec.ts index e1a96cd7e9..0c42c97215 100644 --- a/modules/@angular/platform-browser/test/dom/web_animations_player_spec.ts +++ b/modules/@angular/platform-browser/test/dom/web_animations_player_spec.ts @@ -1,40 +1,7 @@ import {AsyncTestCompleter, MockAnimationPlayer, beforeEach, beforeEachProviders, ddescribe, describe, expect, iit, inject, it, xdescribe, xit} from '@angular/core/testing/testing_internal'; -import {DomAnimatePlayer} from '../../src/dom/dom_animate_player'; import {WebAnimationsPlayer} from '../../src/dom/web_animations_player'; -import {isPresent} from '../../src/facade/lang'; - -export class MockDomAnimatePlayer implements DomAnimatePlayer { - public captures: {[key: string]: any[]} = {}; - private _position: number = 0; - private _onfinish = () => {}; - public currentTime: number; - - _capture(method: string, data: any) { - if (!isPresent(this.captures[method])) { - this.captures[method] = []; - } - this.captures[method].push(data); - } - - cancel() { this._capture('cancel', null); } - play() { this._capture('play', null); } - pause() { this._capture('pause', null); } - finish() { - this._capture('finish', null); - this._onfinish(); - } - set onfinish(fn) { - this._capture('onfinish', fn); - this._onfinish = fn; - } - get onfinish() { return this._onfinish; } - set position(val) { - this._capture('position', val); - this._position = val; - } - get position() { return this._position; } -} +import {MockDomAnimatePlayer} from '../../testing/mock_dom_animate_player'; export function main() { function makePlayer(): {[key: string]: any} { diff --git a/modules/@angular/platform-browser/testing/mock_dom_animate_player.ts b/modules/@angular/platform-browser/testing/mock_dom_animate_player.ts new file mode 100644 index 0000000000..d750eac3a5 --- /dev/null +++ b/modules/@angular/platform-browser/testing/mock_dom_animate_player.ts @@ -0,0 +1,35 @@ +import {DomAnimatePlayer} from '../src/dom/dom_animate_player'; +import {isPresent} from '../src/facade/lang'; + +export class MockDomAnimatePlayer implements DomAnimatePlayer { + public captures: {[key: string]: any[]} = {}; + private _position: number = 0; + private _onfinish: Function = () => {}; + public currentTime: number; + + /** @internal */ + _capture(method: string, data: any): void { + if (!isPresent(this.captures[method])) { + this.captures[method] = []; + } + this.captures[method].push(data); + } + + cancel(): void { this._capture('cancel', null); } + play(): void { this._capture('play', null); } + pause(): void { this._capture('pause', null); } + finish(): void { + this._capture('finish', null); + this._onfinish(); + } + set onfinish(fn: Function) { + this._capture('onfinish', fn); + this._onfinish = fn; + } + get onfinish(): Function { return this._onfinish; } + set position(val: number) { + this._capture('position', val); + this._position = val; + } + get position(): number { return this._position; } +}