diff --git a/dialog/dialog_test.ts b/dialog/dialog_test.ts index 52c8150755..17702de4d0 100644 --- a/dialog/dialog_test.ts +++ b/dialog/dialog_test.ts @@ -34,6 +34,7 @@ describe('', () => { throw new Error('Failed to query rendered '); } + disableDialogAnimations(dialog); const harness = new DialogHarness(dialog); const dialogElement = dialog.shadowRoot?.querySelector('dialog'); if (!dialogElement) { @@ -127,6 +128,7 @@ describe('', () => { contentElement.click(); expect(isClosing).not.toHaveBeenCalled(); dialogElement.click(); + await env.waitForStability(); expect(isClosing).toHaveBeenCalled(); }); @@ -175,4 +177,90 @@ describe('', () => { .withContext('dialog.returnValue after close event canceled') .toBe(prevReturnValue); }); + + it('should open on connected if opened before connected to DOM', async () => { + const openListener = jasmine.createSpy('openListener'); + const dialog = document.createElement('md-dialog'); + disableDialogAnimations(dialog); + dialog.addEventListener('open', openListener); + dialog.open = true; + expect(openListener) + .withContext('should not trigger open before connected') + .not.toHaveBeenCalled(); + + const root = env.render(html``); + root.appendChild(dialog); + await env.waitForStability(); + expect(openListener) + .withContext('opens after connecting') + .toHaveBeenCalled(); + }); + + it('should not open on connected if opened, but closed before connected to DOM', + async () => { + const openListener = jasmine.createSpy('openListener'); + const dialog = document.createElement('md-dialog'); + disableDialogAnimations(dialog); + dialog.addEventListener('open', openListener); + dialog.open = true; + await env.waitForStability(); + dialog.open = false; + const root = env.render(html``); + root.appendChild(dialog); + await env.waitForStability(); + expect(openListener) + .withContext('should not open on connected since close was called') + .not.toHaveBeenCalled(); + }); + + it('should not open on connected if opened before connection but closed after', + async () => { + const openListener = jasmine.createSpy('openListener'); + const dialog = document.createElement('md-dialog'); + disableDialogAnimations(dialog); + dialog.addEventListener('open', openListener); + dialog.open = true; + const root = env.render(html``); + root.appendChild(dialog); + dialog.open = false; + await env.waitForStability(); + expect(openListener) + .withContext( + 'should not open on connected since close was called before open could complete') + .not.toHaveBeenCalled(); + }); + + it('should not dispatch close if closed while disconnected', async () => { + const {harness, root} = await setupTest(); + await harness.element.show(); + + const closeListener = jasmine.createSpy('closeListener'); + harness.element.addEventListener('close', closeListener); + harness.element.remove(); + await env.waitForStability(); + + expect(closeListener) + .withContext('should not trigger close when disconnected') + .not.toHaveBeenCalled(); + + await harness.element.close(); + expect(closeListener) + .withContext('should not trigger close when disconnected') + .not.toHaveBeenCalled(); + + root.appendChild(harness.element); + await env.waitForStability(); + expect(closeListener) + .withContext('should not trigger close when disconnected') + .not.toHaveBeenCalled(); + }); }); + +function disableDialogAnimations(dialog: MdDialog) { + dialog.getOpenAnimation = () => { + return {}; + }; + dialog.getCloseAnimation = () => { + return {}; + }; +} diff --git a/dialog/internal/dialog.ts b/dialog/internal/dialog.ts index caf3c6abd9..4902ce0e38 100644 --- a/dialog/internal/dialog.ts +++ b/dialog/internal/dialog.ts @@ -86,6 +86,10 @@ export class Dialog extends LitElement { getCloseAnimation = () => DIALOG_DEFAULT_CLOSE_ANIMATION; private isOpen = false; + private isOpening = false; + // getIsConnectedPromise() immediately sets the resolve property. + private isConnectedPromiseResolve!: () => void; + private isConnectedPromise = this.getIsConnectedPromise(); @query('dialog') private readonly dialog!: HTMLDialogElement|null; @query('.scrim') private readonly scrim!: HTMLDialogElement|null; @query('.container') private readonly container!: HTMLDialogElement|null; @@ -122,8 +126,15 @@ export class Dialog extends LitElement { * `opened` event was fired. */ async show() { - const {dialog, container} = this; - if (!dialog || !container || dialog.open) { + this.isOpening = true; + // Dialogs can be opened before being attached to the DOM, so we need to + // wait until we're connected before calling `showModal()`. + await this.isConnectedPromise; + await this.updateComplete; + const dialog = this.dialog!; + // Check if already opened or if `dialog.close()` was called while awaiting. + if (dialog.open || !this.isOpening) { + this.isOpening = false; return; } @@ -148,6 +159,7 @@ export class Dialog extends LitElement { await this.animateDialog(this.getOpenAnimation()); this.dispatchEvent(new Event('opened')); + this.isOpening = false; } /** @@ -161,8 +173,18 @@ export class Dialog extends LitElement { * `closed` event was fired. */ async close(returnValue = this.returnValue) { - const {dialog, container} = this; - if (!dialog || !container || !dialog.open) { + this.isOpening = false; + if (!this.isConnected) { + // Disconnected dialogs do not fire close events or animate. + this.open = false; + return; + } + + await this.updateComplete; + const dialog = this.dialog!; + // Check if already closed or if `dialog.show()` was called while awaiting. + if (!dialog.open || this.isOpening) { + this.open = false; return; } @@ -181,6 +203,16 @@ export class Dialog extends LitElement { this.dispatchEvent(new Event('closed')); } + override connectedCallback() { + super.connectedCallback(); + this.isConnectedPromiseResolve(); + } + + override disconnectedCallback() { + super.disconnectedCallback(); + this.isConnectedPromise = this.getIsConnectedPromise(); + } + protected override render() { const scrollable = this.open && !(this.isAtScrollTop && this.isAtScrollBottom); @@ -354,4 +386,10 @@ export class Dialog extends LitElement { this.isAtScrollBottom = isIntersecting; } } + + private getIsConnectedPromise() { + return new Promise(resolve => { + this.isConnectedPromiseResolve = resolve; + }); + } }