fix(animations): ensure `position` and `display` styles are handled outside of keyframes/web-animations (#28911)

When web-animations and/or CSS keyframes are used for animations certain
CSS style values (such as `display` and `position`) may be ignored by a
keyframe-based animation. Angular should special-case these styles to
ensure that they get applied as inline styles throughout the duration of
the animation.

Closes #24923
Closes #25635

Jira Issue: FW-1091
Jira Issue: FW-1092

PR Close #28911

packages/animations/browser/src/render/css_keyframes/css_keyframes_driver.ts

@@ -10,6 +10,7 @@ import {AnimationPlayer, ɵStyleData} from '@angular/animations';
 import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes, computeStyle} from '../../util';
 import {AnimationDriver} from '../animation_driver';
 import {containsElement, hypenatePropsObject, invokeQuery, matchesElement, validateStyleProperty} from '../shared';
+import {packageNonAnimatableStyles} from '../special_cased_styles';
 
 import {CssKeyframesPlayer} from './css_keyframes_player';
 import {DirectStylePlayer} from './direct_style_player';
@@ -105,8 +106,9 @@ export class CssKeyframesDriver implements AnimationDriver {
     const kfElm = this.buildKeyframeElement(element, animationName, keyframes);
     document.querySelector('head') !.appendChild(kfElm);
 
+    const specialStyles = packageNonAnimatableStyles(element, keyframes);
     const player = new CssKeyframesPlayer(
-        element, keyframes, animationName, duration, delay, easing, finalStyles);
+        element, keyframes, animationName, duration, delay, easing, finalStyles, specialStyles);
 
     player.onDestroy(() => removeElement(kfElm));
     return player;

packages/animations/browser/src/render/css_keyframes/css_keyframes_player.ts

@@ -8,12 +8,11 @@
 import {AnimationPlayer} from '@angular/animations';
 
 import {computeStyle} from '../../util';
-
+import {SpecialCasedStyles} from '../special_cased_styles';
 import {ElementAnimationStyleHandler} from './element_animation_style_handler';
 
 const DEFAULT_FILL_MODE = 'forwards';
 const DEFAULT_EASING = 'linear';
-const ANIMATION_END_EVENT = 'animationend';
 
 export const enum AnimatorControlState {INITIALIZED = 1, STARTED = 2, FINISHED = 3, DESTROYED = 4}
 
@@ -38,7 +37,8 @@ export class CssKeyframesPlayer implements AnimationPlayer {
       public readonly element: any, public readonly keyframes: {[key: string]: string | number}[],
       public readonly animationName: string, private readonly _duration: number,
       private readonly _delay: number, easing: string,
-      private readonly _finalStyles: {[key: string]: any}) {
+      private readonly _finalStyles: {[key: string]: any},
+      private readonly _specialStyles?: SpecialCasedStyles|null) {
     this.easing = easing || DEFAULT_EASING;
     this.totalTime = _duration + _delay;
     this._buildStyler();
@@ -57,6 +57,9 @@ export class CssKeyframesPlayer implements AnimationPlayer {
     this._styler.destroy();
     this._flushStartFns();
     this._flushDoneFns();
+    if (this._specialStyles) {
+      this._specialStyles.destroy();
+    }
     this._onDestroyFns.forEach(fn => fn());
     this._onDestroyFns = [];
   }
@@ -77,6 +80,9 @@ export class CssKeyframesPlayer implements AnimationPlayer {
     this._state = AnimatorControlState.FINISHED;
     this._styler.finish();
     this._flushStartFns();
+    if (this._specialStyles) {
+      this._specialStyles.finish();
+    }
     this._flushDoneFns();
   }
 
@@ -100,6 +106,9 @@ export class CssKeyframesPlayer implements AnimationPlayer {
     if (!this.hasStarted()) {
       this._flushStartFns();
       this._state = AnimatorControlState.STARTED;
+      if (this._specialStyles) {
+        this._specialStyles.start();
+      }
     }
     this._styler.resume();
   }

packages/animations/browser/src/render/special_cased_styles.ts

@@ -0,0 +1,133 @@
+/**
+ * @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 {eraseStyles, setStyles} from '../util';
+
+/**
+ * Returns an instance of `SpecialCasedStyles` if and when any special (non animateable) styles are
+ * detected.
+ *
+ * In CSS there exist properties that cannot be animated within a keyframe animation
+ * (whether it be via CSS keyframes or web-animations) and the animation implementation
+ * will ignore them. This function is designed to detect those special cased styles and
+ * return a container that will be executed at the start and end of the animation.
+ *
+ * @returns an instance of `SpecialCasedStyles` if any special styles are detected otherwise `null`
+ */
+export function packageNonAnimatableStyles(
+    element: any, styles: {[key: string]: any} | {[key: string]: any}[]): SpecialCasedStyles|null {
+  let startStyles: {[key: string]: any}|null = null;
+  let endStyles: {[key: string]: any}|null = null;
+  if (Array.isArray(styles) && styles.length) {
+    startStyles = filterNonAnimatableStyles(styles[0]);
+    if (styles.length > 1) {
+      endStyles = filterNonAnimatableStyles(styles[styles.length - 1]);
+    }
+  } else if (styles) {
+    startStyles = filterNonAnimatableStyles(styles);
+  }
+
+  return (startStyles || endStyles) ? new SpecialCasedStyles(element, startStyles, endStyles) :
+                                      null;
+}
+
+/**
+ * Designed to be executed during a keyframe-based animation to apply any special-cased styles.
+ *
+ * When started (when the `start()` method is run) then the provided `startStyles`
+ * will be applied. When finished (when the `finish()` method is called) the
+ * `endStyles` will be applied as well any any starting styles. Finally when
+ * `destroy()` is called then all styles will be removed.
+ */
+export class SpecialCasedStyles {
+  static initialStylesByElement = new WeakMap<any, {[key: string]: any}>();
+
+  private _state = SpecialCasedStylesState.Pending;
+  private _initialStyles !: {[key: string]: any};
+
+  constructor(
+      private _element: any, private _startStyles: {[key: string]: any}|null,
+      private _endStyles: {[key: string]: any}|null) {
+    let initialStyles = SpecialCasedStyles.initialStylesByElement.get(_element);
+    if (!initialStyles) {
+      SpecialCasedStyles.initialStylesByElement.set(_element, initialStyles = {});
+    }
+    this._initialStyles = initialStyles;
+  }
+
+  start() {
+    if (this._state < SpecialCasedStylesState.Started) {
+      if (this._startStyles) {
+        setStyles(this._element, this._startStyles, this._initialStyles);
+      }
+      this._state = SpecialCasedStylesState.Started;
+    }
+  }
+
+  finish() {
+    this.start();
+    if (this._state < SpecialCasedStylesState.Finished) {
+      setStyles(this._element, this._initialStyles);
+      if (this._endStyles) {
+        setStyles(this._element, this._endStyles);
+        this._endStyles = null;
+      }
+      this._state = SpecialCasedStylesState.Started;
+    }
+  }
+
+  destroy() {
+    this.finish();
+    if (this._state < SpecialCasedStylesState.Destroyed) {
+      SpecialCasedStyles.initialStylesByElement.delete(this._element);
+      if (this._startStyles) {
+        eraseStyles(this._element, this._startStyles);
+        this._endStyles = null;
+      }
+      if (this._endStyles) {
+        eraseStyles(this._element, this._endStyles);
+        this._endStyles = null;
+      }
+      setStyles(this._element, this._initialStyles);
+      this._state = SpecialCasedStylesState.Destroyed;
+    }
+  }
+}
+
+/**
+ * An enum of states reflective of what the status of `SpecialCasedStyles` is.
+ *
+ * Depending on how `SpecialCasedStyles` is interacted with, the start and end
+ * styles may not be applied in the same way. This enum ensures that if and when
+ * the ending styles are applied then the starting styles are applied. It is
+ * also used to reflect what the current status of the special cased styles are
+ * which helps prevent the starting/ending styles not be applied twice. It is
+ * also used to cleanup the styles once `SpecialCasedStyles` is destroyed.
+ */
+const enum SpecialCasedStylesState {
+  Pending = 0,
+  Started = 1,
+  Finished = 2,
+  Destroyed = 3,
+}
+
+function filterNonAnimatableStyles(styles: {[key: string]: any}) {
+  let result: {[key: string]: any}|null = null;
+  const props = Object.keys(styles);
+  for (let i = 0; i < props.length; i++) {
+    const prop = props[i];
+    if (isNonAnimatableStyle(prop)) {
+      result = result || {};
+      result[prop] = styles[prop];
+    }
+  }
+  return result;
+}
+
+function isNonAnimatableStyle(prop: string) {
+  return prop === 'display' || prop === 'position';
+}

packages/animations/browser/src/render/web_animations/web_animations_driver.ts

@@ -11,6 +11,7 @@ import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes, copy
 import {AnimationDriver} from '../animation_driver';
 import {CssKeyframesDriver} from '../css_keyframes/css_keyframes_driver';
 import {containsElement, invokeQuery, isBrowser, matchesElement, validateStyleProperty} from '../shared';
+import {packageNonAnimatableStyles} from '../special_cased_styles';
 
 import {WebAnimationsPlayer} from './web_animations_player';
 
@@ -66,7 +67,8 @@ export class WebAnimationsDriver implements AnimationDriver {
 
     keyframes = keyframes.map(styles => copyStyles(styles, false));
     keyframes = balancePreviousStylesIntoKeyframes(element, keyframes, previousStyles);
-    return new WebAnimationsPlayer(element, keyframes, playerOptions);
+    const specialStyles = packageNonAnimatableStyles(element, keyframes);
+    return new WebAnimationsPlayer(element, keyframes, playerOptions, specialStyles);
   }
 }
 

packages/animations/browser/src/render/web_animations/web_animations_player.ts

@@ -7,7 +7,8 @@
  */
 import {AnimationPlayer} from '@angular/animations';
 
-import {allowPreviousPlayerStylesMerge, balancePreviousStylesIntoKeyframes, computeStyle, copyStyles} from '../../util';
+import {computeStyle} from '../../util';
+import {SpecialCasedStyles} from '../special_cased_styles';
 
 import {DOMAnimation} from './dom_animation';
 
@@ -33,7 +34,8 @@ export class WebAnimationsPlayer implements AnimationPlayer {
 
   constructor(
       public element: any, public keyframes: {[key: string]: string | number}[],
-      public options: {[key: string]: string | number}) {
+      public options: {[key: string]: string | number},
+      private _specialStyles?: SpecialCasedStyles|null) {
     this._duration = <number>options['duration'];
     this._delay = <number>options['delay'] || 0;
     this.time = this._duration + this._delay;
@@ -91,6 +93,9 @@ export class WebAnimationsPlayer implements AnimationPlayer {
       this._onStartFns.forEach(fn => fn());
       this._onStartFns = [];
       this._started = true;
+      if (this._specialStyles) {
+        this._specialStyles.start();
+      }
     }
     this.domPlayer.play();
   }
@@ -102,6 +107,9 @@ export class WebAnimationsPlayer implements AnimationPlayer {
 
   finish(): void {
     this.init();
+    if (this._specialStyles) {
+      this._specialStyles.finish();
+    }
     this._onFinish();
     this.domPlayer.finish();
   }
@@ -131,6 +139,9 @@ export class WebAnimationsPlayer implements AnimationPlayer {
       this._destroyed = true;
       this._resetDomPlayerState();
       this._onFinish();
+      if (this._specialStyles) {
+        this._specialStyles.destroy();
+      }
       this._onDestroyFns.forEach(fn => fn());
       this._onDestroyFns = [];
     }

packages/animations/browser/src/util.ts

@@ -157,10 +157,13 @@ function writeStyleAttribute(element: any) {
   element.setAttribute('style', styleAttrValue);
 }
 
-export function setStyles(element: any, styles: ɵStyleData) {
+export function setStyles(element: any, styles: ɵStyleData, formerStyles?: {[key: string]: any}) {
   if (element['style']) {
     Object.keys(styles).forEach(prop => {
       const camelProp = dashCaseToCamelCase(prop);
+      if (formerStyles && !formerStyles.hasOwnProperty(prop)) {
+        formerStyles[prop] = element.style[camelProp];
+      }
       element.style[camelProp] = styles[prop];
     });
     // On the server set the 'style' attribute since it's not automatically reflected.

packages/core/test/animation/animations_with_css_keyframes_animations_integration_spec.ts

@@ -308,13 +308,65 @@ import {TestBed} from '../../testing';
 
          expect(foo.style.getPropertyValue('max-height')).toBeFalsy();
        });
+
+    it('should apply the `display` and `position` styles as regular inline styles for the duration of the animation',
+       () => {
+         @Component({
+           selector: 'ani-cmp',
+           template: `
+          <div #elm [@myAnimation]="myAnimationExp" style="display:table; position:fixed"></div>
+        `,
+           animations: [
+             trigger(
+                 'myAnimation',
+                 [
+                   state('go', style({display: 'inline-block'})),
+                   transition(
+                       '* => go',
+                       [
+                         style({display: 'inline', position: 'absolute', opacity: 0}),
+                         animate('1s', style({display: 'inline', opacity: 1, position: 'static'})),
+                         animate('1s', style({display: 'flexbox', opacity: 0})),
+                       ])
+                 ]),
+           ]
+         })
+         class Cmp {
+           @ViewChild('elm') public element: any;
+
+           public myAnimationExp = '';
+         }
+
+         TestBed.configureTestingModule({declarations: [Cmp]});
+
+         const engine = TestBed.get(AnimationEngine);
+         const fixture = TestBed.createComponent(Cmp);
+         const cmp = fixture.componentInstance;
+
+         // In Ivy, change detection needs to run before the ViewQuery for cmp.element will resolve.
+         // Keeping this test enabled since we still want to test the animation logic in Ivy.
+         if (ivyEnabled) fixture.detectChanges();
+
+         const elm = cmp.element.nativeElement;
+         expect(elm.style.getPropertyValue('display')).toEqual('table');
+         expect(elm.style.getPropertyValue('position')).toEqual('fixed');
+
+         cmp.myAnimationExp = 'go';
+         fixture.detectChanges();
+
+         expect(elm.style.getPropertyValue('display')).toEqual('inline');
+         expect(elm.style.getPropertyValue('position')).toEqual('absolute');
+
+         const player = engine.players.pop();
+         player.finish();
+         player.destroy();
+
+         expect(elm.style.getPropertyValue('display')).toEqual('inline-block');
+         expect(elm.style.getPropertyValue('position')).toEqual('fixed');
+       });
   });
 })();
 
-function approximate(value: number, target: number) {
-  return Math.abs(target - value) / value;
-}
-
 function getPlayer(engine: AnimationEngine, index = 0) {
   return (engine.players[index] as any) !.getRealPlayer();
 }

packages/core/test/animation/animations_with_web_animations_integration_spec.ts

@@ -9,11 +9,11 @@ import {animate, query, state, style, transition, trigger} from '@angular/animat
 import {AnimationDriver, ɵAnimationEngine, ɵWebAnimationsDriver, ɵWebAnimationsPlayer, ɵsupportsWebAnimations} from '@angular/animations/browser';
 import {TransitionAnimationPlayer} from '@angular/animations/browser/src/render/transition_animation_engine';
 import {AnimationGroupPlayer} from '@angular/animations/src/players/animation_group_player';
-import {Component} from '@angular/core';
+import {Component, ViewChild} from '@angular/core';
 import {TestBed} from '@angular/core/testing';
 import {BrowserAnimationsModule} from '@angular/platform-browser/animations';
 import {browserDetection} from '@angular/platform-browser/testing/src/browser_util';
-import {fixmeIvy} from '@angular/private/testing';
+import {ivyEnabled} from '@angular/private/testing';
 
 (function() {
   // these tests are only mean't to be run within the DOM (for now)
@@ -245,8 +245,8 @@ import {fixmeIvy} from '@angular/private/testing';
     it('should treat * styles as ! for queried items that are collected in a container that is being removed',
        () => {
          @Component({
-            selector: 'my-app',
+          selector: 'my-app',
-            styles: [`
+          styles: [`
               .list .outer {
                 overflow:hidden;
               }
@@ -254,7 +254,7 @@ import {fixmeIvy} from '@angular/private/testing';
                 line-height:50px;
               }
             `],
-            template: `
+          template: `
               <button (click)="empty()">Empty</button>
               <button (click)="middle()">Middle</button>
               <button (click)="full()">Full</button>
@@ -267,22 +267,22 @@ import {fixmeIvy} from '@angular/private/testing';
                 </div>
               </div>
             `,
-            animations: [
+          animations: [
-              trigger('list', [
+            trigger('list', [
-                transition(':enter', []),
+              transition(':enter', []),
-                transition('* => empty', [
+              transition('* => empty', [
-                  query(':leave', [
+                query(':leave', [
-                    animate(500, style({ height: '0px' }))
+                  animate(500, style({height: '0px'}))
-                  ])
+                ])
-                ]),
+              ]),
-                transition('* => full', [
+              transition('* => full', [
-                  query(':enter', [
+                query(':enter', [
-                    style({ height: '0px' }),
+                  style({height: '0px'}),
-                    animate(500, style({ height: '*' }))
+                  animate(500, style({height: '*'}))
-                  ])
+                ])
-                ]),
+              ]),
-              ])
+            ])
-            ]
+          ]
         })
         class Cmp {
            items: any[] = [];
@@ -455,6 +455,62 @@ import {fixmeIvy} from '@angular/private/testing';
                .toBeLessThan(0.05);
          }
        });
+
+    it('should apply the `display` and `position` styles as regular inline styles for the duration of the animation',
+       () => {
+         @Component({
+           selector: 'ani-cmp',
+           template: `
+          <div #elm [@myAnimation]="myAnimationExp" style="display:table; position:fixed"></div>
+        `,
+           animations: [
+             trigger(
+                 'myAnimation',
+                 [
+                   state('go', style({display: 'inline-block'})),
+                   transition(
+                       '* => go',
+                       [
+                         style({display: 'inline', position: 'absolute', opacity: 0}),
+                         animate('1s', style({display: 'inline', opacity: 1, position: 'static'})),
+                         animate('1s', style({display: 'flexbox', opacity: 0})),
+                       ])
+                 ]),
+           ]
+         })
+         class Cmp {
+           @ViewChild('elm') public element: any;
+
+           public myAnimationExp = '';
+         }
+
+         TestBed.configureTestingModule({declarations: [Cmp]});
+
+         const engine = TestBed.get(ɵAnimationEngine);
+         const fixture = TestBed.createComponent(Cmp);
+         const cmp = fixture.componentInstance;
+
+         // In Ivy, change detection needs to run before the ViewQuery for cmp.element will resolve.
+         // Keeping this test enabled since we still want to test the animation logic in Ivy.
+         if (ivyEnabled) fixture.detectChanges();
+
+         const elm = cmp.element.nativeElement;
+         expect(elm.style.getPropertyValue('display')).toEqual('table');
+         expect(elm.style.getPropertyValue('position')).toEqual('fixed');
+
+         cmp.myAnimationExp = 'go';
+         fixture.detectChanges();
+
+         expect(elm.style.getPropertyValue('display')).toEqual('inline');
+         expect(elm.style.getPropertyValue('position')).toEqual('absolute');
+
+         const player = engine.players.pop();
+         player.finish();
+         player.destroy();
+
+         expect(elm.style.getPropertyValue('display')).toEqual('inline-block');
+         expect(elm.style.getPropertyValue('position')).toEqual('fixed');
+       });
   });
 })();