fix(dialog): not showing if opened before connected

Fixes #4728

PiperOrigin-RevId: 559157443

dialog/dialog_test.ts

@@ -34,6 +34,7 @@ describe('<md-dialog>', () => {
       throw new Error('Failed to query rendered <md-dialog>');
     }
 
+    disableDialogAnimations(dialog);
     const harness = new DialogHarness(dialog);
     const dialogElement = dialog.shadowRoot?.querySelector('dialog');
     if (!dialogElement) {
@@ -127,6 +128,7 @@ describe('<md-dialog>', () => {
       contentElement.click();
       expect(isClosing).not.toHaveBeenCalled();
       dialogElement.click();
+      await env.waitForStability();
       expect(isClosing).toHaveBeenCalled();
     });
 
@@ -175,4 +177,90 @@ describe('<md-dialog>', () => {
         .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 {};
+  };
+}

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<void>(resolve => {
+      this.isConnectedPromiseResolve = resolve;
+    });
+  }
 }