From 6e20e0aac8fb5aa0a44725ee16b69bd204c1a164 Mon Sep 17 00:00:00 2001 From: Vikram Subramanian Date: Thu, 21 Jun 2018 22:55:47 -0700 Subject: [PATCH] fix(animations): set animations styles properly on platform-server (#24624) Animations styles weren't getting properly set on platform-server because of erroneous checks and absence of reflection of style property to attribute on the server. The fix corrects the check for platform and explicitly reflects the style property to the attribute. PR Close #24624 --- .../animations/browser/src/render/shared.ts | 11 ++++- packages/animations/browser/src/util.ts | 44 +++++++++++++++++++ .../animations/browser/tsconfig-build.json | 1 + .../platform-server/test/integration_spec.ts | 20 +++++++-- 4 files changed, 71 insertions(+), 5 deletions(-) diff --git a/packages/animations/browser/src/render/shared.ts b/packages/animations/browser/src/render/shared.ts index 7cb2f7a033..7027805b46 100644 --- a/packages/animations/browser/src/render/shared.ts +++ b/packages/animations/browser/src/render/shared.ts @@ -14,6 +14,10 @@ export function isBrowser() { return (typeof window !== 'undefined' && typeof window.document !== 'undefined'); } +export function isNode() { + return (typeof process !== 'undefined'); +} + export function optimizeGroupPlayer(players: AnimationPlayer[]): AnimationPlayer { switch (players.length) { case 0: @@ -142,11 +146,14 @@ let _query: (element: any, selector: string, multi: boolean) => any[] = return []; }; -if (isBrowser()) { +// Define utility methods for browsers and platform-server(domino) where Element +// and utility methods exist. +const _isNode = isNode(); +if (_isNode || typeof Element !== 'undefined') { // this is well supported in all browsers _contains = (elm1: any, elm2: any) => { return elm1.contains(elm2) as boolean; }; - if (Element.prototype.matches) { + if (_isNode || Element.prototype.matches) { _matches = (element: any, selector: string) => element.matches(selector); } else { const proto = Element.prototype as any; diff --git a/packages/animations/browser/src/util.ts b/packages/animations/browser/src/util.ts index 5b5d8a3587..0fe31caf4f 100644 --- a/packages/animations/browser/src/util.ts +++ b/packages/animations/browser/src/util.ts @@ -8,6 +8,7 @@ import {AnimateTimings, AnimationMetadata, AnimationMetadataType, AnimationOptions, sequence, ɵStyleData} from '@angular/animations'; import {Ast as AnimationAst, AstVisitor as AnimationAstVisitor} from './dsl/animation_ast'; import {AnimationDslVisitor} from './dsl/animation_dsl_visitor'; +import {isNode} from './render/shared'; export const ONE_SECOND = 1000; @@ -125,12 +126,47 @@ export function copyStyles( return destination; } +function getStyleAttributeString(element: any, key: string, value: string) { + // Return the key-value pair string to be added to the style attribute for the + // given CSS style key. + if (value) { + return key + ':' + value + ';'; + } else { + return ''; + } +} + +function writeStyleAttribute(element: any) { + // Read the style property of the element and manually reflect it to the + // style attribute. This is needed because Domino on platform-server doesn't + // understand the full set of allowed CSS properties and doesn't reflect some + // of them automatically. + let styleAttrValue = ''; + for (let i = 0; i < element.style.length; i++) { + const key = element.style.item(i); + styleAttrValue += getStyleAttributeString(element, key, element.style.getPropertyValue(key)); + } + for (const key in element.style) { + // Skip internal Domino properties that don't need to be reflected. + if (!element.style.hasOwnProperty(key) || key.startsWith('_')) { + continue; + } + const dashKey = camelCaseToDashCase(key); + styleAttrValue += getStyleAttributeString(element, dashKey, element.style[key]); + } + element.setAttribute('style', styleAttrValue); +} + export function setStyles(element: any, styles: ɵStyleData) { if (element['style']) { Object.keys(styles).forEach(prop => { const camelProp = dashCaseToCamelCase(prop); element.style[camelProp] = styles[prop]; }); + // On the server set the 'style' attribute since it's not automatically reflected. + if (isNode()) { + writeStyleAttribute(element); + } } } @@ -140,6 +176,10 @@ export function eraseStyles(element: any, styles: ɵStyleData) { const camelProp = dashCaseToCamelCase(prop); element.style[camelProp] = ''; }); + // On the server set the 'style' attribute since it's not automatically reflected. + if (isNode()) { + writeStyleAttribute(element); + } } } @@ -231,6 +271,10 @@ export function dashCaseToCamelCase(input: string): string { return input.replace(DASH_CASE_REGEXP, (...m: any[]) => m[1].toUpperCase()); } +function camelCaseToDashCase(input: string): string { + return input.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase(); +} + export function allowPreviousPlayerStylesMerge(duration: number, delay: number) { return duration === 0 || delay === 0; } diff --git a/packages/animations/browser/tsconfig-build.json b/packages/animations/browser/tsconfig-build.json index 4e4dfedb12..963bc8dfa2 100644 --- a/packages/animations/browser/tsconfig-build.json +++ b/packages/animations/browser/tsconfig-build.json @@ -13,6 +13,7 @@ "files": [ "public_api.ts", + "../../../node_modules/@types/node/index.d.ts", "../../../node_modules/zone.js/dist/zone.js.d.ts", "../../system.d.ts" ], diff --git a/packages/platform-server/test/integration_spec.ts b/packages/platform-server/test/integration_spec.ts index 3e7e8b2006..1b77bab5e6 100644 --- a/packages/platform-server/test/integration_spec.ts +++ b/packages/platform-server/test/integration_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AnimationBuilder, animate, style, transition, trigger} from '@angular/animations'; +import {AnimationBuilder, animate, state, style, transition, trigger} from '@angular/animations'; import {APP_BASE_HREF, PlatformLocation, isPlatformServer} from '@angular/common'; import {HTTP_INTERCEPTORS, HttpClient, HttpClientModule, HttpEvent, HttpHandler, HttpInterceptor, HttpRequest} from '@angular/common/http'; import {HttpClientTestingModule, HttpTestingController} from '@angular/common/http/testing'; @@ -136,12 +136,21 @@ class SVGServerModule { @Component({ selector: 'app', - template: '
{{text}}
', + template: `
{{text}}
`, animations: [trigger( 'myAnimation', - [transition('void => *', [style({'opacity': '0'}), animate(500, style({'opacity': '1'}))])])], + [ + state('void', style({'opacity': '0'})), + state('active', style({ + 'opacity': '1', // simple supported property + 'font-weight': 'bold', // property with dashed name + 'transform': 'translate3d(0, 0, 0)', // not natively supported by Domino + })), + transition('void => *', [animate('0ms')]), + ], )] }) class MyAnimationApp { + state = 'active'; constructor(private builder: AnimationBuilder) {} text = 'Works!'; @@ -524,6 +533,8 @@ class HiddenModule { // PlatformConfig takes in a parsed document so that it can be cached across requests. doc = ''; called = false; + (global as any)['window'] = undefined; + (global as any)['document'] = undefined; }); afterEach(() => { expect(called).toBe(true); }); @@ -576,6 +587,9 @@ class HiddenModule { renderModule(AnimationServerModule, {document: doc}).then(output => { expect(output).toContain('Works!'); expect(output).toContain('ng-trigger-myAnimation'); + expect(output).toContain('opacity:1;'); + expect(output).toContain('transform:translate3d(0, 0, 0);'); + expect(output).toContain('font-weight:bold;'); called = true; }); }));