fix(animations): make sure 'ngbRunTransition' runs inside the zone (#…

…3957) Fixes #3950 (cherry picked from commit a29f570)
ng-bootstrap · Jan 21, 2021 · a006a62 · a006a62
1 parent dce3886
commit a006a62
Show file tree

Hide file tree

Showing 14 changed files with 191 additions and 88 deletions.
diff --git a/src/accordion/accordion.ts b/src/accordion/accordion.ts
@@ -407,7 +407,7 @@ export class NgbAccordion implements AfterContentChecked {
       // The direction (show or hide) is choosen by each panel.isOpen state
       if (panel.transitionRunning) {
         const panelElement = this._getPanelElement(panel.id);
-        ngbRunTransition(panelElement !, ngbCollapsingTransition, {
+        ngbRunTransition(this._ngZone, panelElement !, ngbCollapsingTransition, {
           animation,
           runningTransition: 'stop',
           context: {direction: panel.isOpen ? 'show' : 'hide'}

diff --git a/src/alert/alert.ts b/src/alert/alert.ts
@@ -9,7 +9,8 @@ import {
   OnChanges,
   OnInit,
   SimpleChanges,
-  ViewEncapsulation
+  ViewEncapsulation,
+  NgZone
 } from '@angular/core';
 
 import {Observable} from 'rxjs';
@@ -75,7 +76,8 @@ export class NgbAlert implements OnInit,
   @Output() closed = new EventEmitter<void>();
 
 
-  constructor(config: NgbAlertConfig, private _renderer: Renderer2, private _element: ElementRef) {
+  constructor(
+      config: NgbAlertConfig, private _renderer: Renderer2, private _element: ElementRef, private _zone: NgZone) {
     this.dismissible = config.dismissible;
     this.type = config.type;
     this.animation = config.animation;
@@ -93,7 +95,7 @@ export class NgbAlert implements OnInit,
    */
   close(): Observable<void> {
     const transition = ngbRunTransition(
-        this._element.nativeElement, ngbAlertFadingTransition,
+        this._zone, this._element.nativeElement, ngbAlertFadingTransition,
         {animation: this.animation, runningTransition: 'continue'});
     transition.subscribe(() => this.closed.emit());
     return transition;

diff --git a/src/carousel/carousel.ts b/src/carousel/carousel.ts
@@ -377,15 +377,16 @@ export class NgbCarousel implements AfterContentChecked,
       const activeSlide = this._getSlideById(this.activeId);
       if (activeSlide) {
         const activeSlideTransition =
-            ngbRunTransition(this._getSlideElement(activeSlide.id), ngbCarouselTransitionOut, options);
+            ngbRunTransition(this._ngZone, this._getSlideElement(activeSlide.id), ngbCarouselTransitionOut, options);
         activeSlideTransition.subscribe(() => { activeSlide.slid.emit({isShown: false, direction, source}); });
         transitions.push(activeSlideTransition);
       }
 
       const previousId = this.activeId;
       this.activeId = selectedSlide.id;
       const nextSlide = this._getSlideById(this.activeId);
-      const transition = ngbRunTransition(this._getSlideElement(selectedSlide.id), ngbCarouselTransitionIn, options);
+      const transition =
+          ngbRunTransition(this._ngZone, this._getSlideElement(selectedSlide.id), ngbCarouselTransitionIn, options);
       transition.subscribe(() => { nextSlide ?.slid.emit({isShown: true, direction, source}); });
       transitions.push(transition);
 

diff --git a/src/collapse/collapse.ts b/src/collapse/collapse.ts
@@ -3,6 +3,7 @@ import {
   ElementRef,
   EventEmitter,
   Input,
+  NgZone,
   OnChanges,
   OnInit,
   Output,
@@ -49,7 +50,9 @@ export class NgbCollapse implements OnInit, OnChanges {
   @Output() hidden = new EventEmitter<void>();
 
 
-  constructor(private _element: ElementRef, config: NgbCollapseConfig) { this.animation = config.animation; }
+  constructor(private _element: ElementRef, config: NgbCollapseConfig, private _zone: NgZone) {
+    this.animation = config.animation;
+  }
 
   ngOnInit() {
     this._element.nativeElement.classList.add('collapse');
@@ -77,7 +80,7 @@ export class NgbCollapse implements OnInit, OnChanges {
   }
 
   private _runTransition(collapsed: boolean, animation: boolean, emitEvent = true) {
-    ngbRunTransition(this._element.nativeElement, ngbCollapsingTransition, {
+    ngbRunTransition(this._zone, this._element.nativeElement, ngbCollapsingTransition, {
       animation,
       runningTransition: 'stop',
       context: {direction: collapsed ? 'hide' : 'show'}

diff --git a/src/modal/modal-backdrop.ts b/src/modal/modal-backdrop.ts
@@ -25,14 +25,14 @@ export class NgbModalBackdrop implements OnInit {
   ngOnInit() {
     this._zone.onStable.asObservable().pipe(take(1)).subscribe(() => {
       ngbRunTransition(
-          this._el.nativeElement, ({classList}) => classList.add('show'),
+          this._zone, this._el.nativeElement, ({classList}) => classList.add('show'),
           {animation: this.animation, runningTransition: 'continue'});
     });
   }
 
   hide(): Observable<void> {
     return ngbRunTransition(
-        this._el.nativeElement, ({classList}) => classList.remove('show'),
+        this._zone, this._el.nativeElement, ({classList}) => classList.remove('show'),
         {animation: this.animation, runningTransition: 'stop'});
   }
 }
diff --git a/src/modal/modal-window.ts b/src/modal/modal-window.ts
@@ -79,8 +79,9 @@ export class NgbModalWindow implements OnInit,
     const {nativeElement} = this._elRef;
     const context: NgbTransitionOptions<any> = {animation: this.animation, runningTransition: 'stop'};
 
-    const windowTransition$ = ngbRunTransition(nativeElement, () => nativeElement.classList.remove('show'), context);
-    const dialogTransition$ = ngbRunTransition(this._dialogEl.nativeElement, () => {}, context);
+    const windowTransition$ =
+        ngbRunTransition(this._zone, nativeElement, () => nativeElement.classList.remove('show'), context);
+    const dialogTransition$ = ngbRunTransition(this._zone, this._dialogEl.nativeElement, () => {}, context);
 
     const transitions$ = zip(windowTransition$, dialogTransition$);
     transitions$.subscribe(() => {
@@ -98,8 +99,9 @@ export class NgbModalWindow implements OnInit,
     const {nativeElement} = this._elRef;
     const context: NgbTransitionOptions<any> = {animation: this.animation, runningTransition: 'continue'};
 
-    const windowTransition$ = ngbRunTransition(nativeElement, () => nativeElement.classList.add('show'), context);
-    const dialogTransition$ = ngbRunTransition(this._dialogEl.nativeElement, () => {}, context);
+    const windowTransition$ =
+        ngbRunTransition(this._zone, nativeElement, () => nativeElement.classList.add('show'), context);
+    const dialogTransition$ = ngbRunTransition(this._zone, this._dialogEl.nativeElement, () => {}, context);
 
     zip(windowTransition$, dialogTransition$).subscribe(() => {
       this.shown.next();
@@ -189,7 +191,7 @@ export class NgbModalWindow implements OnInit,
 
   private _bumpBackdrop() {
     if (this.backdrop === 'static') {
-      ngbRunTransition(this._elRef.nativeElement, ({classList}) => {
+      ngbRunTransition(this._zone, this._elRef.nativeElement, ({classList}) => {
         classList.add('modal-static');
         return () => classList.remove('modal-static');
       }, {animation: this.animation, runningTransition: 'continue'});

diff --git a/src/nav/nav-outlet.ts b/src/nav/nav-outlet.ts
@@ -6,6 +6,7 @@ import {
   Directive,
   ElementRef,
   Input,
+  NgZone,
   QueryList,
   ViewChildren,
   ViewEncapsulation
@@ -68,7 +69,7 @@ export class NgbNavOutlet implements AfterViewInit {
    */
   @Input('ngbNavOutlet') nav: NgbNav;
 
-  constructor(private _cd: ChangeDetectorRef) {}
+  constructor(private _cd: ChangeDetectorRef, private _ngZone: NgZone) {}
 
   isPanelTransitioning(item: NgbNavItem) { return this._activePane ?.item === item; }
 
@@ -88,32 +89,34 @@ export class NgbNavOutlet implements AfterViewInit {
 
       // fading out
       if (this._activePane) {
-        ngbRunTransition(this._activePane.elRef.nativeElement, ngbNavFadeOutTransition, options).subscribe(() => {
-          const activeItem = this._activePane ?.item;
-          this._activePane = this._getPaneForItem(nextItem);
-
-          // mark for check when transition finishes as outlet or parent containers might be OnPush
-          // without this the panes that have "faded out" will stay in DOM
-          this._cd.markForCheck();
-
-          // fading in
-          if (this._activePane) {
-            // we have to add the '.active' class before running the transition,
-            // because it should be in place before `ngbRunTransition` does `reflow()`
-            this._activePane.elRef.nativeElement.classList.add('active');
-            ngbRunTransition(this._activePane.elRef.nativeElement, ngbNavFadeInTransition, options).subscribe(() => {
-              if (nextItem) {
-                nextItem.shown.emit();
-                this.nav.shown.emit(nextItem.id);
+        ngbRunTransition(this._ngZone, this._activePane.elRef.nativeElement, ngbNavFadeOutTransition, options)
+            .subscribe(() => {
+              const activeItem = this._activePane ?.item;
+              this._activePane = this._getPaneForItem(nextItem);
+
+              // mark for check when transition finishes as outlet or parent containers might be OnPush
+              // without this the panes that have "faded out" will stay in DOM
+              this._cd.markForCheck();
+
+              // fading in
+              if (this._activePane) {
+                // we have to add the '.active' class before running the transition,
+                // because it should be in place before `ngbRunTransition` does `reflow()`
+                this._activePane.elRef.nativeElement.classList.add('active');
+                ngbRunTransition(this._ngZone, this._activePane.elRef.nativeElement, ngbNavFadeInTransition, options)
+                    .subscribe(() => {
+                      if (nextItem) {
+                        nextItem.shown.emit();
+                        this.nav.shown.emit(nextItem.id);
+                      }
+                    });
               }
-            });
-          }
 
-          if (activeItem) {
-            activeItem.hidden.emit();
-            this.nav.hidden.emit(activeItem.id);
-          }
-        });
+              if (activeItem) {
+                activeItem.hidden.emit();
+                this.nav.hidden.emit(activeItem.id);
+              }
+            });
       } else {
         this._updateActivePane();
       }

diff --git a/src/popover/popover.spec.ts b/src/popover/popover.spec.ts
@@ -10,7 +10,8 @@ import {
   OnDestroy,
   TemplateRef,
   ViewContainerRef,
-  AfterViewInit
+  AfterViewInit,
+  NgZone
 } from '@angular/core';
 
 import {NgbPopoverModule} from './popover.module';
@@ -511,8 +512,8 @@ describe('ngb-popover', () => {
           `<div ngbPopover="Great tip!" triggers="click" (shown)="shown()" (hidden)="hidden()"></div>`);
       const directive = fixture.debugElement.query(By.directive(NgbPopover));
 
-      let shownSpy = spyOn(fixture.componentInstance, 'shown');
-      let hiddenSpy = spyOn(fixture.componentInstance, 'hidden');
+      let shownSpy = spyOn(fixture.componentInstance, 'shown').and.callThrough();
+      let hiddenSpy = spyOn(fixture.componentInstance, 'hidden').and.callThrough();
 
       triggerEvent(directive, 'click');
       fixture.detectChanges();
@@ -907,8 +908,8 @@ export class TestComponent {
     this._vcRef.remove(0);
   }
 
-  shown() {}
-  hidden() {}
+  shown() { expect(NgZone.isInAngularZone()).toBe(true, `'shown' should run inside the Angular zone`); }
+  hidden() { expect(NgZone.isInAngularZone()).toBe(true, `'hidden' should run inside the Angular zone`); }
 }
 
 @Component({selector: 'test-onpush-cmpt', changeDetection: ChangeDetectionStrategy.OnPush, template: ``})

diff --git a/src/toast/toast.ts b/src/toast/toast.ts
@@ -164,7 +164,8 @@ export class NgbToast implements AfterContentInit,
   hide(): Observable<void> {
     this._clearTimeout();
     const transition = ngbRunTransition(
-        this._element.nativeElement, ngbToastFadeOutTransition, {animation: this.animation, runningTransition: 'stop'});
+        this._zone, this._element.nativeElement, ngbToastFadeOutTransition,
+        {animation: this.animation, runningTransition: 'stop'});
     transition.subscribe(() => { this.hidden.emit(); });
     return transition;
   }
@@ -180,7 +181,7 @@ export class NgbToast implements AfterContentInit,
    * @since 8.0.0
    */
   show(): Observable<void> {
-    const transition = ngbRunTransition(this._element.nativeElement, ngbToastFadeInTransition, {
+    const transition = ngbRunTransition(this._zone, this._element.nativeElement, ngbToastFadeInTransition, {
       animation: this.animation,
       runningTransition: 'continue',
     });

diff --git a/src/tooltip/tooltip.spec.ts b/src/tooltip/tooltip.spec.ts
@@ -6,6 +6,7 @@ import {
   AfterViewInit,
   ChangeDetectionStrategy,
   Component,
+  NgZone,
   TemplateRef,
   ViewChild,
   ViewContainerRef
@@ -531,8 +532,8 @@ describe('ngb-tooltip', () => {
           `<div ngbTooltip="Great tip!" triggers="click" (shown)="shown()" (hidden)="hidden()"></div>`);
       const directive = fixture.debugElement.query(By.directive(NgbTooltip));
 
-      let shownSpy = spyOn(fixture.componentInstance, 'shown');
-      let hiddenSpy = spyOn(fixture.componentInstance, 'hidden');
+      const shownSpy = spyOn(fixture.componentInstance, 'shown').and.callThrough();
+      const hiddenSpy = spyOn(fixture.componentInstance, 'hidden').and.callThrough();
 
       triggerEvent(directive, 'click');
       fixture.detectChanges();
@@ -789,8 +790,8 @@ export class TestComponent {
 
   @ViewChild(NgbTooltip, {static: true}) tooltip: NgbTooltip;
 
-  shown() {}
-  hidden() {}
+  shown() { expect(NgZone.isInAngularZone()).toBe(true, `'shown' should run inside the Angular zone`); }
+  hidden() { expect(NgZone.isInAngularZone()).toBe(true, `'hidden' should run inside the Angular zone`); }
 
   constructor(private _vcRef: ViewContainerRef) {}
 

diff --git a/src/util/popup.ts b/src/util/popup.ts
@@ -38,11 +38,11 @@ export class PopupService<T> {
     }
 
     const {nativeElement} = this._windowRef.location;
-    const onStable$ = this._ngZone.onStable.asObservable().pipe(take(1));
-    const transition$ = onStable$.pipe(mergeMap(() => this._ngZone.run(() => {
-      return ngbRunTransition(
-          nativeElement, ({classList}) => classList.add('show'), {animation, runningTransition: 'continue'});
-    })));
+    const transition$ = this._ngZone.onStable.pipe(
+        take(1), mergeMap(
+                     () => ngbRunTransition(
+                         this._ngZone, nativeElement, ({classList}) => classList.add('show'),
+                         {animation, runningTransition: 'continue'})));
 
     return {windowRef: this._windowRef, transition$};
   }
@@ -53,7 +53,7 @@ export class PopupService<T> {
     }
 
     return ngbRunTransition(
-               this._windowRef.location.nativeElement, ({classList}) => classList.remove('show'),
+               this._ngZone, this._windowRef.location.nativeElement, ({classList}) => classList.remove('show'),
                {animation, runningTransition: 'stop'})
         .pipe(tap(() => {
           if (this._windowRef) {