From 25ab4647c5bb49749f8a1941d4fb83b354dcca52 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 3 Mar 2020 16:01:41 -0800 Subject: [PATCH] Revert "fix(animations): process shorthand `margin` and `padding` styles correctly (#35701)" (#35847) This reverts commit 35c9f0dc2f3665d4f9d9ece328cee4559bbec9c6, breaks internal tests PR Close #35847 --- aio/scripts/_payload-limits.json | 4 +- .../css_keyframes/css_keyframes_driver.ts | 6 +- .../css_keyframes/css_keyframes_player.ts | 2 +- .../animations/browser/src/render/shared.ts | 34 ------------ .../web_animations/web_animations_driver.ts | 4 +- .../web_animations/web_animations_player.ts | 2 +- packages/animations/browser/src/util.ts | 7 ++- .../element_animation_style_handler_spec.ts | 2 + .../browser/test/render/shared_spec.ts | 55 ------------------- 9 files changed, 16 insertions(+), 100 deletions(-) delete mode 100644 packages/animations/browser/test/render/shared_spec.ts diff --git a/aio/scripts/_payload-limits.json b/aio/scripts/_payload-limits.json index 04d5de641c..e83f14b8a1 100755 --- a/aio/scripts/_payload-limits.json +++ b/aio/scripts/_payload-limits.json @@ -12,7 +12,7 @@ "master": { "uncompressed": { "runtime-es2015": 2987, - "main-es2015": 451552, + "main-es2015": 451469, "polyfills-es2015": 52195 } } @@ -21,7 +21,7 @@ "master": { "uncompressed": { "runtime-es2015": 3097, - "main-es2015": 429313, + "main-es2015": 429230, "polyfills-es2015": 52195 } } diff --git a/packages/animations/browser/src/render/css_keyframes/css_keyframes_driver.ts b/packages/animations/browser/src/render/css_keyframes/css_keyframes_driver.ts index 30aafcb60e..c69ee499c9 100644 --- a/packages/animations/browser/src/render/css_keyframes/css_keyframes_driver.ts +++ b/packages/animations/browser/src/render/css_keyframes/css_keyframes_driver.ts @@ -7,9 +7,9 @@ */ import {AnimationPlayer, ɵStyleData} from '@angular/animations'; -import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes} from '../../util'; +import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes, computeStyle} from '../../util'; import {AnimationDriver} from '../animation_driver'; -import {computeStyle, containsElement, hypenatePropsObject, invokeQuery, matchesElement, validateStyleProperty} from '../shared'; +import {containsElement, hypenatePropsObject, invokeQuery, matchesElement, validateStyleProperty} from '../shared'; import {packageNonAnimatableStyles} from '../special_cased_styles'; import {CssKeyframesPlayer} from './css_keyframes_player'; @@ -36,7 +36,7 @@ export class CssKeyframesDriver implements AnimationDriver { } computeStyle(element: any, prop: string, defaultValue?: string): string { - return computeStyle(element, prop); + return (window.getComputedStyle(element) as any)[prop] as string; } buildKeyframeElement(element: any, name: string, keyframes: {[key: string]: any}[]): any { diff --git a/packages/animations/browser/src/render/css_keyframes/css_keyframes_player.ts b/packages/animations/browser/src/render/css_keyframes/css_keyframes_player.ts index f14e7c315b..1a6f302b04 100644 --- a/packages/animations/browser/src/render/css_keyframes/css_keyframes_player.ts +++ b/packages/animations/browser/src/render/css_keyframes/css_keyframes_player.ts @@ -7,7 +7,7 @@ */ import {AnimationPlayer} from '@angular/animations'; -import {computeStyle} from '../shared'; +import {computeStyle} from '../../util'; import {SpecialCasedStyles} from '../special_cased_styles'; import {ElementAnimationStyleHandler} from './element_animation_style_handler'; diff --git a/packages/animations/browser/src/render/shared.ts b/packages/animations/browser/src/render/shared.ts index 6ae40fd625..e3d6119fad 100644 --- a/packages/animations/browser/src/render/shared.ts +++ b/packages/animations/browser/src/render/shared.ts @@ -237,37 +237,3 @@ export function hypenatePropsObject(object: {[key: string]: any}): {[key: string }); return newObj; } - - -/** - * Returns the computed style for the provided property on the provided element. - * - * This function uses `window.getComputedStyle` internally to determine the - * style value for the element. Firefox doesn't support reading the shorthand - * forms of margin/padding and for this reason this function needs to account - * for that. - */ -export function computeStyle(element: HTMLElement, prop: string): string { - const gcs = window.getComputedStyle(element); - - // this is casted to any because the `CSSStyleDeclaration` type is a fixed - // set of properties and `prop` is a dynamic reference to a property within - // the `CSSStyleDeclaration` list. - let value = gcs[prop as any]; - - // Firefox returns empty string values for `margin` and `padding` properties - // when extracted using getComputedStyle (see similar issue here: - // https://github.com/jquery/jquery/issues/3383). In this situation - // we want to emulate the value that is returned by creating the top, - // right, bottom and left properties as individual style lookups. - if (value.length === 0 && (prop === 'margin' || prop === 'padding')) { - // reconstruct the padding/margin value as `top right bottom left` - const propTop = (prop + 'Top') as 'marginTop' | 'paddingTop'; - const propRight = (prop + 'Right') as 'marginRight' | 'paddingRight'; - const propBottom = (prop + 'Bottom') as 'marginBottom' | 'paddingBottom'; - const propLeft = (prop + 'Left') as 'marginLeft' | 'paddingLeft'; - value = `${gcs[propTop]} ${gcs[propRight]} ${gcs[propBottom]} ${gcs[propLeft]}`; - } - - return value; -} diff --git a/packages/animations/browser/src/render/web_animations/web_animations_driver.ts b/packages/animations/browser/src/render/web_animations/web_animations_driver.ts index 8e7ee84e39..29cd26e376 100644 --- a/packages/animations/browser/src/render/web_animations/web_animations_driver.ts +++ b/packages/animations/browser/src/render/web_animations/web_animations_driver.ts @@ -10,7 +10,7 @@ import {AnimationPlayer, ɵStyleData} from '@angular/animations'; import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes, copyStyles} from '../../util'; import {AnimationDriver} from '../animation_driver'; import {CssKeyframesDriver} from '../css_keyframes/css_keyframes_driver'; -import {computeStyle, containsElement, invokeQuery, isBrowser, matchesElement, validateStyleProperty} from '../shared'; +import {containsElement, invokeQuery, isBrowser, matchesElement, validateStyleProperty} from '../shared'; import {packageNonAnimatableStyles} from '../special_cased_styles'; import {WebAnimationsPlayer} from './web_animations_player'; @@ -32,7 +32,7 @@ export class WebAnimationsDriver implements AnimationDriver { } computeStyle(element: any, prop: string, defaultValue?: string): string { - return computeStyle(element, prop); + return (window.getComputedStyle(element) as any)[prop] as string; } overrideWebAnimationsSupport(supported: boolean) { this._isNativeImpl = supported; } diff --git a/packages/animations/browser/src/render/web_animations/web_animations_player.ts b/packages/animations/browser/src/render/web_animations/web_animations_player.ts index e0e0d0c3d8..76c2cd44be 100644 --- a/packages/animations/browser/src/render/web_animations/web_animations_player.ts +++ b/packages/animations/browser/src/render/web_animations/web_animations_player.ts @@ -7,7 +7,7 @@ */ import {AnimationPlayer} from '@angular/animations'; -import {computeStyle} from '../shared'; +import {computeStyle} from '../../util'; import {SpecialCasedStyles} from '../special_cased_styles'; import {DOMAnimation} from './dom_animation'; diff --git a/packages/animations/browser/src/util.ts b/packages/animations/browser/src/util.ts index 45c26b2cd1..ffcc2a7567 100644 --- a/packages/animations/browser/src/util.ts +++ b/packages/animations/browser/src/util.ts @@ -6,10 +6,9 @@ * found in the LICENSE file at https://angular.io/license */ 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 {computeStyle, isNode} from './render/shared'; +import {isNode} from './render/shared'; export const ONE_SECOND = 1000; @@ -341,3 +340,7 @@ export function visitDslNode(visitor: any, node: any, context: any): any { throw new Error(`Unable to resolve animation metadata node #${node.type}`); } } + +export function computeStyle(element: any, prop: string): string { + return (window.getComputedStyle(element))[prop]; +} diff --git a/packages/animations/browser/test/render/css_keyframes/element_animation_style_handler_spec.ts b/packages/animations/browser/test/render/css_keyframes/element_animation_style_handler_spec.ts index e252781ee2..d5b64b5758 100644 --- a/packages/animations/browser/test/render/css_keyframes/element_animation_style_handler_spec.ts +++ b/packages/animations/browser/test/render/css_keyframes/element_animation_style_handler_spec.ts @@ -6,6 +6,8 @@ * found in the LICENSE file at https://angular.io/license */ import {ElementAnimationStyleHandler} from '../../../src/render/css_keyframes/element_animation_style_handler'; +import {computeStyle} from '../../../src/util'; + import {assertStyle, createElement, makeAnimationEvent, supportsAnimationEventCreation} from './shared'; const EMPTY_FN = () => {}; diff --git a/packages/animations/browser/test/render/shared_spec.ts b/packages/animations/browser/test/render/shared_spec.ts deleted file mode 100644 index a181cd7a25..0000000000 --- a/packages/animations/browser/test/render/shared_spec.ts +++ /dev/null @@ -1,55 +0,0 @@ -/** - * @license - * Copyright Google Inc. All Rights Reserved. - * - * Use of this source code is governed by an MIT-style license that can be - * found in the LICENSE file at https://angular.io/license - */ -import {computeStyle} from '../../src/render/shared'; - -describe('shared animations code', () => { - if (isNode) return; - - describe('computeStyle', () => { - it('should compute the margin style into the form top,right,bottom,left', () => { - const div = buildActualElement(); - - div.style.setProperty('margin', '1px 2px 3px 4px'); - expect(computeStyle(div, 'margin')).toEqual('1px 2px 3px 4px'); - - div.style.setProperty('margin', '0px'); - div.style.setProperty('margin-top', '10px'); - div.style.setProperty('margin-right', '20px'); - div.style.setProperty('margin-bottom', '30px'); - div.style.setProperty('margin-left', '40px'); - expect(computeStyle(div, 'margin')).toEqual('10px 20px 30px 40px'); - }); - - it('should compute the padding style into the form top,right,bottom,left', () => { - const div = buildActualElement(); - - div.style.setProperty('padding', '1px 2px 3px 4px'); - expect(computeStyle(div, 'padding')).toEqual('1px 2px 3px 4px'); - - div.style.setProperty('padding', '0px'); - div.style.setProperty('padding-top', '10px'); - div.style.setProperty('padding-right', '20px'); - div.style.setProperty('padding-bottom', '30px'); - div.style.setProperty('padding-left', '40px'); - expect(computeStyle(div, 'padding')).toEqual('10px 20px 30px 40px'); - }); - }); -}); - -/** - * Returns a div element that's attached to the body. - * - * The reason why this function exists is because in order to - * compute style values on an element is must be attached within - * the body of a webpage. - */ -function buildActualElement() { - const div = document.createElement('div'); - document.body.appendChild(div); - return div; -}