From 96ca9f76db491374fa28b466d562e700aa010fc3 Mon Sep 17 00:00:00 2001 From: ffriedl89 Date: Wed, 3 Jun 2020 09:14:33 +0000 Subject: [PATCH] fix(confirmation-dialog): Fixes an issue that the confirmation dialog was blocking interactions although it was not visible. --- .../src/confirmation-dialog.html | 6 +++- .../src/confirmation-dialog.spec.ts | 29 +++++++--------- .../src/confirmation-dialog.ts | 34 +++++++++++++++---- 3 files changed, 45 insertions(+), 24 deletions(-) diff --git a/libs/barista-components/confirmation-dialog/src/confirmation-dialog.html b/libs/barista-components/confirmation-dialog/src/confirmation-dialog.html index 99403ac05e..528b33c884 100644 --- a/libs/barista-components/confirmation-dialog/src/confirmation-dialog.html +++ b/libs/barista-components/confirmation-dialog/src/confirmation-dialog.html @@ -1,5 +1,9 @@ -
+
{ 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 index d856b4491b..06e545b5f2 100644 --- 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; /** 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