fix(dialog, modal, popover, sheet): restore deactivating focus traps on outside click

packages/calcite-components/src/components/dialog/dialog.tsx

@@ -78,20 +78,8 @@ export class Dialog
 
   private dragPosition: DialogDragPosition = { ...initialDragPosition };
 
-  private escapeDeactivates = (event: KeyboardEvent): boolean => {
-    if (event.defaultPrevented || this.escapeDisabled) {
-      return false;
-    }
-    event.preventDefault();
-    return true;
-  };
-
   focusTrap: FocusTrap;
 
-  private focusTrapDeactivates = (): void => {
-    this.open = false;
-  };
-
   private ignoreOpenChange = false;
 
   private interaction: Interactable;
@@ -306,10 +294,16 @@ export class Dialog
     this.mutationObserver?.observe(this.el, { childList: true, subtree: true });
     connectFocusTrap(this, {
       focusTrapOptions: {
-        // Scrim has it's own close handler, allow it to take over.
-        clickOutsideDeactivates: false,
-        escapeDeactivates: this.escapeDeactivates,
-        onDeactivate: this.focusTrapDeactivates,
+        // scrim closes on click, so we let it take over
+        clickOutsideDeactivates: () => !this.modal,
+        escapeDeactivates: (event) => {
+          if (!event.defaultPrevented && !this.escapeDisabled) {
+            this.open = false;
+            event.preventDefault();
+          }
+
+          return false;
+        },
       },
     });
     this.setupInteractions();

packages/calcite-components/src/components/modal/modal.tsx

@@ -80,20 +80,8 @@ export class Modal
     this.updateSizeCssVars();
   });
 
-  private escapeDeactivates = (event: KeyboardEvent) => {
-    if (event.defaultPrevented || this.escapeDisabled) {
-      return false;
-    }
-    event.preventDefault();
-    return true;
-  };
-
   focusTrap: FocusTrap;
 
-  private focusTrapDeactivates = () => {
-    this.open = false;
-  };
-
   private ignoreOpenChange = false;
 
   private modalContent = createRef<HTMLDivElement>();
@@ -276,10 +264,16 @@ export class Modal
     this.updateSizeCssVars();
     connectFocusTrap(this, {
       focusTrapOptions: {
-        // Scrim has it's own close handler, allow it to take over.
+        // scrim closes on click, so we let it take over
         clickOutsideDeactivates: false,
-        escapeDeactivates: this.escapeDeactivates,
-        onDeactivate: this.focusTrapDeactivates,
+        escapeDeactivates: (event) => {
+          if (!event.defaultPrevented && !this.escapeDisabled) {
+            this.open = false;
+            event.preventDefault();
+          }
+
+          return false;
+        },
       },
     });
   }

packages/calcite-components/src/components/popover/popover.tsx

@@ -78,20 +78,6 @@ export class Popover
 
   private arrowEl: SVGSVGElement;
 
-  private clickOutsideDeactivates = (event: MouseEvent): boolean => {
-    const path = event.composedPath();
-    const isReferenceElementInPath =
-      this.referenceEl instanceof EventTarget && path.includes(this.referenceEl);
-
-    const outsideClick = !path.includes(this.el);
-    const shouldCloseOnOutsideClick = this.autoClose && outsideClick;
-
-    return (
-      shouldCloseOnOutsideClick &&
-      (this.triggerDisabled || (isReferenceElementInPath && !this.autoClose))
-    );
-  };
-
   private closeButtonEl = createRef<Action["el"]>();
 
   private filteredFlipPlacements: FlipPlacement[];
@@ -100,10 +86,6 @@ export class Popover
 
   focusTrap: FocusTrap;
 
-  private focusTrapDeactivates = (): void => {
-    this.open = false;
-  };
-
   private guid = `calcite-popover-${guid()}`;
 
   private hasLoaded = false;
@@ -298,9 +280,15 @@ export class Popover
       focusTrapEl: this.el,
       focusTrapOptions: {
         allowOutsideClick: true,
-        clickOutsideDeactivates: this.clickOutsideDeactivates,
+        escapeDeactivates: (event) => {
+          if (!event.defaultPrevented) {
+            this.open = false;
+            event.preventDefault();
+          }
+
+          return false;
+        },
         initialFocus: this.initialFocusTrapFocus,
-        onDeactivate: this.focusTrapDeactivates,
       },
     });
 

packages/calcite-components/src/components/sheet/sheet.tsx

@@ -60,20 +60,8 @@ export class Sheet
 
   private contentId: string;
 
-  private escapeDeactivates = (event: KeyboardEvent) => {
-    if (event.defaultPrevented || this.escapeDisabled) {
-      return false;
-    }
-    event.preventDefault();
-    return true;
-  };
-
   focusTrap: FocusTrap;
 
-  private focusTrapDeactivates = (): void => {
-    this.open = false;
-  };
-
   private ignoreOpenChange = false;
 
   private initialOverflowCSS: string;
@@ -254,10 +242,16 @@ export class Sheet
     this.mutationObserver?.observe(this.el, { childList: true, subtree: true });
     connectFocusTrap(this, {
       focusTrapOptions: {
-        // Scrim has it's own close handler, allow it to take over.
+        // scrim closes on click, so we let it take over
         clickOutsideDeactivates: false,
-        escapeDeactivates: this.escapeDeactivates,
-        onDeactivate: this.focusTrapDeactivates,
+        escapeDeactivates: (event) => {
+          if (!event.defaultPrevented && !this.escapeDisabled) {
+            this.open = false;
+            event.preventDefault();
+          }
+
+          return false;
+        },
       },
     });
     this.setupInteractions();

packages/calcite-components/src/utils/focusTrapComponent.ts

@@ -46,6 +46,7 @@ export function connectFocusTrap(component: FocusTrapComponent, options?: Connec
   }
 
   const focusTrapOptions: FocusTrapOptions = {
+    clickOutsideDeactivates: true,
     fallbackFocus: focusTrapNode,
     setReturnFocus: (el) => {
       focusElement(el as FocusableElement);