Make dialog and drawer more resilient to upcoming CloseWatcher API

change/@ni-nimble-components-697d280f-9413-48ef-b2a4-0704abc5584c.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Clean up dialog/drawer properly if cancel event skipped",
+  "packageName": "@ni/nimble-components",
+  "email": "7282195+m-akinc@users.noreply.github.com",
+  "dependentChangeType": "patch"
+}

packages/nimble-components/src/dialog/index.ts

@@ -107,8 +107,7 @@ export class Dialog<CloseReason = void> extends FoundationElement {
             throw new Error('Dialog is not open');
         }
         this.dialogElement.close();
-        this.resolveShow!(reason);
-        this.resolveShow = undefined;
+        this.doResolveShow(reason);
     }
 
     public slottedFooterElementsChanged(
@@ -125,11 +124,33 @@ export class Dialog<CloseReason = void> extends FoundationElement {
         if (this.preventDismiss) {
             event.preventDefault();
         } else {
-            this.resolveShow!(UserDismissed);
-            this.resolveShow = undefined;
+            this.doResolveShow(UserDismissed);
         }
         return true;
     }
+
+    /**
+     * @internal
+     */
+    public closeHandler(): void {
+        if (this.resolveShow) {
+            // If
+            // - the browser implements dialogs with the CloseWatcher API, and
+            // - the user presses ESC without first interacting with the dialog (e.g. clicking, scrolling),
+            // the cancel event is not fired, but the close event still is, and the dialog just closes.
+            this.doResolveShow(UserDismissed);
+        }
+    }
+
+    private doResolveShow(reason: CloseReason | UserDismissed): void {
+        if (!this.resolveShow) {
+            throw new Error(
+                'Do not call doResolveShow unless there is a promise to resolve'
+            );
+        }
+        this.resolveShow(reason);
+        this.resolveShow = undefined;
+    }
 }
 
 // eslint-disable-next-line @typescript-eslint/no-empty-interface

packages/nimble-components/src/dialog/styles.ts

@@ -15,7 +15,8 @@ import {
     elevation3BoxShadow,
     dialogSmallWidth,
     dialogSmallHeight,
-    dialogSmallMaxHeight
+    dialogSmallMaxHeight,
+    borderHoverColor
 } from '../theme-provider/design-tokens';
 import {
     modalBackdropColorThemeColorStatic,
@@ -25,6 +26,7 @@ import {
 import { Theme } from '../theme-provider/types';
 import { themeBehavior } from '../utilities/style/theme';
 import { accessiblyHidden } from '../utilities/style/accessibly-hidden';
+import { focusVisible } from '../utilities/style/focus';
 
 export const styles = css`
     ${display('grid')}
@@ -44,6 +46,10 @@ export const styles = css`
         display: flex;
     }
 
+    dialog${focusVisible} {
+        outline: 2px solid ${borderHoverColor};
+    }
+
     header {
         min-height: 48px;
         padding: 24px 24px 0px 24px;

packages/nimble-components/src/dialog/template.ts

@@ -8,6 +8,7 @@ export const template = html<Dialog>`
             role="dialog"
             part="control"
             @cancel="${(x, c) => x.cancelHandler(c.event)}"
+            @close="${x => x.closeHandler()}"
             aria-labelledby="header"
         >
             <header id="header">

packages/nimble-components/src/dialog/tests/dialog.spec.ts

@@ -173,6 +173,22 @@ describe('Dialog', () => {
         await disconnect();
     });
 
+    // This can potentially happen if the dialog is implemented with the CloseWatcher API
+    it('should resolve promise with UserDismissed when only close event fired', async () => {
+        const { element, connect, disconnect } = await setup();
+        await connect();
+        const dialogPromise = element.show();
+        await waitForUpdatesAsync();
+        // Simulate user dismiss events in browser
+        nativeDialogElement(element).dispatchEvent(new Event('close'));
+        await waitForUpdatesAsync();
+
+        await expectAsync(dialogPromise).toBeResolvedTo(UserDismissed);
+        expect(element.open).toBeFalse();
+
+        await disconnect();
+    });
+
     it('should dismiss an attempted cancel event when prevent-dismiss is enabled', async () => {
         const { element, connect, disconnect } = await setup(true);
         await connect();

packages/nimble-components/src/drawer/index.ts

@@ -92,6 +92,30 @@ export class Drawer<CloseReason = void> extends FoundationElement {
         return true;
     }
 
+    /**
+     * @internal
+     */
+    public closeHandler(): void {
+        if (this.resolveShow) {
+            // If
+            // - the browser implements dialogs with the CloseWatcher API, and
+            // - the user presses ESC without first interacting with the drawer (e.g. clicking, scrolling),
+            // the cancel event is not fired, but the close event still is, and the drawer just closes.
+            // The animation is never started, so there is no animation end listener to clean up.
+            this.doResolveShow(UserDismissed);
+        }
+    }
+
+    private doResolveShow(reason: CloseReason | UserDismissed): void {
+        if (!this.resolveShow) {
+            throw new Error(
+                'Do not call doResolveShow unless there is a promise to resolve'
+            );
+        }
+        this.resolveShow(reason);
+        this.resolveShow = undefined;
+    }
+
     private readonly animationEndHandlerFunction = (): void => this.animationEndHandler();
 
     private openDialog(): void {
@@ -126,8 +150,7 @@ export class Drawer<CloseReason = void> extends FoundationElement {
             this.dialog.classList.remove('closing');
             this.dialog.close();
             this.closing = false;
-            this.resolveShow!(this.closeReason);
-            this.resolveShow = undefined;
+            this.doResolveShow(this.closeReason);
         }
     }
 }

packages/nimble-components/src/drawer/template.ts

@@ -6,6 +6,7 @@ export const template = html<Drawer>`
         ${ref('dialog')}
         aria-label="${x => x.ariaLabel}"
         @cancel="${(x, c) => x.cancelHandler(c.event)}"
+        @close="${x => x.closeHandler()}"
     >
         <div class="dialog-contents">
             <slot></slot>

packages/nimble-components/src/drawer/tests/drawer.spec.ts

@@ -161,6 +161,15 @@ describe('Drawer', () => {
             expect(element.open).toBeFalse();
         });
 
+        // This can potentially happen if the dialog is implemented with the CloseWatcher API
+        it('should resolve promise with UserDismissed when only close event fired', async () => {
+            const promise = element.show();
+            // Simulate user dismiss events in browser
+            nativeDialogElement(element).dispatchEvent(new Event('close'));
+            await expectAsync(promise).toBeResolvedTo(UserDismissed);
+            expect(element.open).toBeFalse();
+        });
+
         it('throws calling show() a second time', async () => {
             void element.show();
             await expectAsync(element.show()).toBeRejectedWithError();