fix(animations): retain triggers values for moved tracked list items (#44578)

dario-piotrowicz · atscott · commit abc217b28e43 · 2022-01-07T22:15:52.000Z
when reordering a list with animations the list items lose their current triggers values, that is because the reordering of an item is implemented as follows (_note:_ the following implementation has been added in PR #23534) - the item is removed and marked _setForRemoval_ - the item is re-inserted, and the _setForRemoval_ is changed to _setForMove_ - the player set for animating the removal is destroyed when _setForMove_ is detected the above steps allow the element not to be animated and to keep its styling but the triggers values get lost since the removal transition/player has already been initialized, so this change adds a _previousTriggersValues_ map in the _REMOVAL_FLAG_ field in order to restore the triggers values if/when the removal turns out to be a move, changing the above steps to: - the item is removed and marked setForRemoval __and its current triggers values are saved as well__ - the item is re-inserted, and the setForRemoval is changed to setForMove - the player set for animating the removal is destroyed when setForMove is detected __and the trigger values are re-applied in the engine's statesByElement map__ resolves #29526 PR Close #44578
diff --git a/packages/animations/browser/src/render/transition_animation_engine.ts b/packages/animations/browser/src/render/transition_animation_engine.ts
@@ -65,6 +65,7 @@ export interface ElementAnimationState {
   hasAnimation: boolean;
   namespaceId: string;
   removedBeforeQueried: boolean;
+  previousTriggersValues?: Map<string, string>;
 }
 
 export class StateValue {
@@ -344,9 +345,11 @@ export class AnimationTransitionNamespace {
       element: any, context: any, destroyAfterComplete?: boolean,
       defaultToFallback?: boolean): boolean {
     const triggerStates = this._engine.statesByElement.get(element);
+    const previousTriggersValues = new Map<string, string>();
     if (triggerStates) {
       const players: TransitionAnimationPlayer[] = [];
       Object.keys(triggerStates).forEach(triggerName => {
+        previousTriggersValues.set(triggerName, triggerStates[triggerName].value);
         // this check is here in the event that an element is removed
         // twice (both on the host level and the component level)
         if (this._triggers[triggerName]) {
@@ -358,7 +361,7 @@ export class AnimationTransitionNamespace {
       });
 
       if (players.length) {
-        this._engine.markElementAsRemoved(this.id, element, true, context);
+        this._engine.markElementAsRemoved(this.id, element, true, context, previousTriggersValues);
         if (destroyAfterComplete) {
           optimizeGroupPlayer(players).onDone(() => this._engine.processLeaveNode(element));
         }
@@ -755,10 +758,17 @@ export class TransitionAnimationEngine {
     }
   }
 
-  markElementAsRemoved(namespaceId: string, element: any, hasAnimation?: boolean, context?: any) {
+  markElementAsRemoved(
+      namespaceId: string, element: any, hasAnimation?: boolean, context?: any,
+      previousTriggersValues?: Map<string, string>) {
     this.collectedLeaveElements.push(element);
-    element[REMOVAL_FLAG] =
-        {namespaceId, setForRemoval: context, hasAnimation, removedBeforeQueried: false};
+    element[REMOVAL_FLAG] = {
+      namespaceId,
+      setForRemoval: context,
+      hasAnimation,
+      removedBeforeQueried: false,
+      previousTriggersValues
+    };
   }
 
   listen(
@@ -991,8 +1001,21 @@ export class TransitionAnimationEngine {
 
         if (this.collectedEnterElements.length) {
           const details = element[REMOVAL_FLAG] as ElementAnimationState;
-          // move animations are currently not supported...
+          // animations for move operations (elements being removed and reinserted,
+          // e.g. when the order of an *ngFor list changes) are currently not supported
           if (details && details.setForMove) {
+            if (details.previousTriggersValues &&
+                details.previousTriggersValues.has(entry.triggerName)) {
+              const previousValue = details.previousTriggersValues.get(entry.triggerName) as string;
+
+              // we need to restore the previous trigger value since the element has
+              // only been moved and hasn't actually left the DOM
+              const triggersWithStates = this.statesByElement.get(entry.element);
+              if (triggersWithStates && triggersWithStates[entry.triggerName]) {
+                triggersWithStates[entry.triggerName].value = previousValue;
+              }
+            }
+
             player.destroy();
             return;
           }
diff --git a/packages/core/test/animation/animation_integration_spec.ts b/packages/core/test/animation/animation_integration_spec.ts
@@ -1644,6 +1644,70 @@ describe('animation tests', function() {
          }
        });
 
+    it('should keep/restore the trigger value when there are move operations (with *ngFor + trackBy)',
+       fakeAsync(() => {
+         @Component({
+           selector: 'ani-cmp',
+           template: `
+          <div *ngFor="let item of items, trackBy: trackItem"
+               @myAnimation (@myAnimation.start)="cb($event)">
+            item{{ item }}
+          </div>
+        `,
+           animations: [trigger('myAnimation', [])]
+         })
+         class Cmp {
+           public items: number[] = [];
+
+           log: string[] = [];
+           cb(event: AnimationEvent) {
+             this.log.push(
+                 `[${event.element.innerText.trim()}] ${event.fromState} => ${event.toState}`);
+           }
+
+           trackItem(_index: number, item: number) {
+             return item.toString();
+           }
+           addItem() {
+             this.items.push(this.items.length);
+           }
+           removeItem() {
+             this.items.pop();
+           }
+           reverseItems() {
+             this.items = this.items.reverse();
+           }
+         }
+
+         TestBed.configureTestingModule({declarations: [Cmp]});
+
+         const engine = TestBed.inject(ɵAnimationEngine);
+         const fixture = TestBed.createComponent(Cmp);
+         const cmp = fixture.componentInstance;
+         fixture.detectChanges();
+
+         const completeAnimations = () => {
+           fixture.detectChanges();
+           flushMicrotasks();
+           engine.players.forEach(player => player.finish());
+         };
+
+         cmp.log = [];
+         [0, 1, 2].forEach(() => cmp.addItem());
+         completeAnimations();
+         expect(cmp.log).toEqual(
+             ['[item0] void => null', '[item1] void => null', '[item2] void => null']);
+
+         cmp.reverseItems();
+         completeAnimations();
+
+         cmp.log = [];
+         [0, 1, 2].forEach(() => cmp.removeItem());
+         completeAnimations();
+         expect(cmp.log).toEqual(
+             ['[item2] null => void', '[item1] null => void', '[item0] null => void']);
+       }));
+
     it('should animate removals of nodes to the `void` state for each animation trigger, but treat all auto styles as pre styles',
        () => {
          @Component({
