fix(confirmation-dialog): Fixes an issue that the confirmation dialog…

… was blocking interactions although it was not visible.
dynatrace-oss · Jun 4, 2020 · 96ca9f7 · 96ca9f7
1 parent fe960b7
commit 96ca9f7
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 24 deletions.
diff --git a/libs/barista-components/confirmation-dialog/src/confirmation-dialog.html b/libs/barista-components/confirmation-dialog/src/confirmation-dialog.html
@@ -1,5 +1,9 @@
 <ng-template>
-  <div class="dt-confirmation-dialog-wrapper" [@pop]="_positionState">
+  <div
+    class="dt-confirmation-dialog-wrapper"
+    [@pop]="_positionState"
+    (@pop.done)="_popDone()"
+  >
     <div
       class="dt-confirmation-dialog-container"
       dtTheme=":dark"

diff --git a/libs/barista-components/confirmation-dialog/src/confirmation-dialog.spec.ts b/libs/barista-components/confirmation-dialog/src/confirmation-dialog.spec.ts
@@ -37,7 +37,6 @@ import {
 
 describe('ConfirmationDialogComponent', () => {
   const UP = 'translateY(0)';
-  const DOWN = 'translateY(100%)';
 
   let overlayContainerElement: HTMLElement;
   let overlayContainer: OverlayContainer;
@@ -96,15 +95,16 @@ describe('ConfirmationDialogComponent', () => {
       expect(dialog.style.transform).toEqual(UP);
     }));
 
-    it('should not pop up if state does not match any child', fakeAsync(() => {
+    it('should not create an overlay if state does not match any child', fakeAsync(() => {
       fixture.componentInstance.testState = 'missingState';
       fixture.detectChanges();
       tick();
       fixture.detectChanges();
       tick();
       fixture.detectChanges();
+      flush();
       const dialog = getDialog(overlayContainerElement);
-      expect(dialog.style.transform).toEqual(DOWN);
+      expect(dialog).toBeNull();
     }));
 
     it('should display matched dt-confirmation-dialog-state', fakeAsync(() => {
@@ -222,21 +222,15 @@ describe('ConfirmationDialogComponent', () => {
       fixture = TestBed.createComponent(DynamicStates);
       fixture.componentInstance.changeToDynamic();
       fixture.detectChanges();
-      tick();
-      fixture.detectChanges();
+      tick(DT_CONFIRMATION_POP_DURATION);
 
       fixture.componentInstance.hasDynamic = false;
       fixture.detectChanges();
-      tick();
-      fixture.detectChanges();
-      tick();
+      tick(DT_CONFIRMATION_POP_DURATION);
       fixture.detectChanges();
-
-      expect(
-        (overlayContainerElement.querySelector(
-          '.dt-confirmation-dialog-wrapper',
-        ) as HTMLElement).style.transform,
-      ).toBe('translateY(100%)');
+      flush();
+      const dialog = getDialog(overlayContainerElement);
+      expect(dialog).toBeNull();
     }));
   });
 
@@ -255,17 +249,19 @@ describe('ConfirmationDialogComponent', () => {
           '.dt-confirmation-dialog-wrapper',
         ),
       );
+      // There should only be one dialog wrapper created
+      expect(dialogs.length).toBe(1);
       expect(dialogs[0].style.transform).toEqual(UP);
-      expect(dialogs[1].style.transform).toEqual(DOWN);
       fixture.componentInstance.secondDialogState = 'state2';
       fixture.detectChanges();
       tick();
       fixture.detectChanges();
       tick();
       fixture.detectChanges();
+      flush();
       const dialog1State = getState(overlayContainerElement, 'state1');
       const dialog2State = getState(overlayContainerElement, 'state2');
-      expect(dialog1State.textContent).toEqual('');
+      expect(dialog1State).toBeNull();
       expect(dialog2State.textContent!.trim()).toEqual('state2');
     }));
   });
@@ -275,6 +271,7 @@ describe('ConfirmationDialogComponent', () => {
       fixture.componentInstance.testState = 'missingState';
       fixture.detectChanges();
       tick();
+      flush();
       expect(overlayContainerElement.innerHTML).toContain(
         'dt-ui-test-id="confirmation-dialog-overlay',
       );

diff --git a/libs/barista-components/confirmation-dialog/src/confirmation-dialog.ts b/libs/barista-components/confirmation-dialog/src/confirmation-dialog.ts
@@ -150,7 +150,7 @@ export class DtConfirmationDialog
   private readonly _stateChildren: QueryList<DtConfirmationDialogState>;
   /** A static reference to the currently active dialog to prevent multiple */
   private static _activeDialog: DtConfirmationDialog | null = null;
-  private _overlayRef: OverlayRef;
+  private _overlayRef: OverlayRef | null = null;
   private _showBackdrop = false;
   private _stateChildrenSubscription: Subscription = Subscription.EMPTY;
   private _viewportChangesSubscription: Subscription = Subscription.EMPTY;
@@ -173,15 +173,19 @@ export class DtConfirmationDialog
   ) {}
 
   ngAfterContentChecked(): void {
-    if (this._selectedState !== this._stateToSelect) {
+    // Initally we get undefined and null for selectedState and stateToSelect which would not pass
+    // the equal check. Therefore we need to check whether one of them is defined to not
+    // create the overlay instantly on the first run
+    if (
+      this._selectedState !== this._stateToSelect &&
+      (this._stateToSelect || this._selectedState)
+    ) {
       this._update();
       this._selectedState = this._stateToSelect;
     }
   }
 
   ngAfterContentInit(): void {
-    this._createAndAttachOverlay();
-    this._updateBackdropVisibility();
     this._stateChildrenSubscription = this._stateChildren.changes.subscribe(
       () => {
         this._update();
@@ -196,7 +200,7 @@ export class DtConfirmationDialog
     }
     // clear state and dispose overlay.
     this.state = null;
-    this._overlayRef.dispose();
+    this._overlayRef?.dispose();
     this._stateChildrenSubscription.unsubscribe();
     this._viewportChangesSubscription.unsubscribe();
   }
@@ -206,8 +210,20 @@ export class DtConfirmationDialog
     this._wiggleState = true;
   }
 
+  /** @internal Callback for the animation done on the pop animation */
+  _popDone(): void {
+    if (this._positionState === 'down') {
+      this._overlayRef?.detach();
+      this._overlayRef = null;
+    }
+  }
+
   /** Updates the children's active properties and the position of the dialog depending on the state to select */
   private _update(): void {
+    if (!this._overlayRef) {
+      this._createAndAttachOverlay();
+      this._updateBackdropVisibility();
+    }
     // handle old dialog already being displayed by closing it and showing a warning in dev mode.
     if (this._stateToSelect) {
       if (
@@ -261,6 +277,8 @@ export class DtConfirmationDialog
 
   /** Creates and attaches the overlay to the dom */
   private _createAndAttachOverlay(): void {
+    this._viewportChangesSubscription.unsubscribe();
+
     const offsetLeft = this._viewportResizer.getOffset().left;
     const positionStrategy = this._overlay
       .position()
@@ -280,6 +298,8 @@ export class DtConfirmationDialog
       positionStrategy,
     });
 
+    this._overlayRef = overlayRef;
+
     this._viewportChangesSubscription = this._viewportResizer
       .change()
       .subscribe(() => {
@@ -288,8 +308,8 @@ export class DtConfirmationDialog
         // because the positionstrategy determines the left offset based on the value
         // of width - if width is 100%, 100vw or max-width is 100% or 100vw
         // the left offset is always set to 0
-        this._overlayRef.updateSize({ width: getOverlayWidth(newOffset) });
-        this._overlayRef.updatePositionStrategy(
+        this._overlayRef?.updateSize({ width: getOverlayWidth(newOffset) });
+        this._overlayRef?.updatePositionStrategy(
           // We need to create a new position strategy instance here otherwise the updatePositionStrategy
           // performs an early access and noop
           this._overlay