refactor: overflow + interaction outside

.changeset/lazy-ways-hunt.md

@@ -0,0 +1,6 @@
+---
+"@zag-js/interact-outside": patch
+"@zag-js/menu": patch
+---
+
+Fix issue where interaction outside detection doesn't work consistently when trigger is within a scrollable container.

examples/next-ts/pages/menu-overflow.tsx

@@ -0,0 +1,101 @@
+import * as menu from "@zag-js/menu"
+import { normalizeProps, useMachine } from "@zag-js/react"
+import { useId } from "react"
+
+const Menu = () => {
+  const [state, send] = useMachine(
+    menu.machine({
+      id: useId(),
+      positioning: { hideWhenDetached: true },
+    }),
+  )
+
+  const api = menu.connect(state, send, normalizeProps)
+
+  return (
+    <div>
+      <button {...api.getTriggerProps()}>
+        Actions <span {...api.getIndicatorProps()}>▾</span>
+      </button>
+      {api.open && (
+        <div {...api.getPositionerProps()}>
+          <ul {...api.getContentProps()}>
+            <li {...api.getItemProps({ value: "edit" })}>Edit</li>
+            <li {...api.getItemProps({ value: "duplicate" })}>Duplicate</li>
+            <li {...api.getItemProps({ value: "delete" })}>Delete</li>
+            <li {...api.getItemProps({ value: "export" })}>Export...</li>
+          </ul>
+        </div>
+      )}
+    </div>
+  )
+}
+
+const HorizontalScrollBox = () => {
+  return (
+    <div style={{ display: "flex", justifyContent: "center", alignItems: "center" }}>
+      <div
+        style={{
+          display: "flex",
+          width: "300px",
+          height: "100%",
+          overflowX: "scroll",
+          columnGap: "24px",
+          flexShrink: 0,
+          padding: "16px",
+          border: "1px solid #ccc",
+          backgroundColor: "#f0f0f0",
+        }}
+      >
+        {[...Array(6).keys()].map((x) => (
+          <div
+            key={x}
+            style={{
+              backgroundColor: "lightgray",
+              borderRadius: "4px",
+              padding: "16px",
+              whiteSpace: "nowrap",
+            }}
+          >
+            Item {x}
+          </div>
+        ))}
+        <div>
+          <Menu />
+        </div>
+      </div>
+    </div>
+  )
+}
+
+const VerticalScrollBox = () => {
+  return (
+    <div style={{ backgroundColor: "#ffffff", border: "1px solid black" }}>
+      <div
+        style={{
+          display: "flex",
+          flexDirection: "column",
+          gap: "16px",
+          alignItems: "center",
+          maxHeight: "200px",
+          width: "200px",
+          overflowY: "auto",
+          border: "1px solid black",
+        }}
+      >
+        {[...Array(12).keys()].map((x) => (
+          <Menu key={x} />
+        ))}
+      </div>
+    </div>
+  )
+}
+
+export default function Page() {
+  return (
+    <main style={{ height: "100vh", display: "flex", justifyContent: "center", alignItems: "center", gap: "40px" }}>
+      <HorizontalScrollBox />
+      <VerticalScrollBox />
+    </main>
+  )
+}

packages/machines/menu/src/menu.machine.ts

@@ -42,7 +42,6 @@ export function machine(userContext: UserDefinedContext) {
         lastHighlightedValue: null,
         children: cast(ref({})),
         suspendPointer: false,
-        restoreFocus: true,
         typeaheadState: getByTypeahead.defaultOptions,
       },
 
@@ -489,6 +488,7 @@ export function machine(userContext: UserDefinedContext) {
         },
         trackInteractOutside(ctx, _evt, { send }) {
           const getContentEl = () => dom.getContentEl(ctx)
+          let restoreFocus = true
           return trackDismissableElement(getContentEl, {
             defer: true,
             exclude: [dom.getTriggerEl(ctx)],
@@ -500,11 +500,11 @@ export function machine(userContext: UserDefinedContext) {
               closeRootMenu(ctx)
             },
             onPointerDownOutside(event) {
-              ctx.restoreFocus = !event.detail.focusable
+              restoreFocus = !event.detail.focusable
               ctx.onPointerDownOutside?.(event)
             },
             onDismiss() {
-              send({ type: "CLOSE", src: "interact-outside" })
+              send({ type: "CLOSE", src: "interact-outside", restoreFocus })
             },
           })
         },
@@ -659,8 +659,8 @@ export function machine(userContext: UserDefinedContext) {
           if (!ctx.highlightedValue) return
           ctx.onSelect?.({ value: ctx.highlightedValue })
         },
-        focusTrigger(ctx) {
-          if (ctx.isSubmenu || ctx.anchorPoint || !ctx.restoreFocus) return
+        focusTrigger(ctx, evt) {
+          if (ctx.isSubmenu || ctx.anchorPoint || evt.restoreFocus === false) return
           queueMicrotask(() => dom.getTriggerEl(ctx)?.focus({ preventScroll: true }))
         },
         highlightMatchedItem(ctx, evt) {

packages/machines/menu/src/menu.types.ts

@@ -164,11 +164,6 @@ interface PrivateContext {
    * The typeahead state for faster keyboard navigation
    */
   typeaheadState: TypeaheadState
-  /**
-   * @internal
-   * Whether to return focus to the trigger when the menu is closed
-   */
-  restoreFocus?: boolean | undefined
 }
 
 export interface MachineContext extends PublicContext, PrivateContext, ComputedContext {}

packages/utilities/interact-outside/src/index.ts

@@ -73,14 +73,32 @@ function isEventPointWithin(node: MaybeElement, event: Event) {
   )
 }
 
-function isEventWithinScrollbar(event: Event, target: HTMLElement): boolean {
-  if (!target || !isPointerEvent(event)) return false
+function isPointInRect(rect: { x: number; y: number; width: number; height: number }, point: { x: number; y: number }) {
+  return rect.y <= point.y && point.y <= rect.y + rect.height && rect.x <= point.x && point.x <= rect.x + rect.width
+}
+
+function isEventWithinScrollbar(event: Event, ancestor: HTMLElement): boolean {
+  if (!ancestor || !isPointerEvent(event)) return false
+
+  const isScrollableY = ancestor.scrollHeight > ancestor.clientHeight
+  const onScrollbarY = isScrollableY && event.clientX > ancestor.offsetLeft + ancestor.clientWidth
+
+  const isScrollableX = ancestor.scrollWidth > ancestor.clientWidth
+  const onScrollbarX = isScrollableX && event.clientY > ancestor.offsetTop + ancestor.clientHeight
 
-  const isScrollableY = target.scrollHeight > target.clientHeight
-  const onScrollbarY = isScrollableY && event.clientX > target.clientWidth
+  const rect = {
+    x: ancestor.offsetLeft,
+    y: ancestor.offsetTop,
+    width: ancestor.clientWidth + (isScrollableY ? 16 : 0),
+    height: ancestor.clientHeight + (isScrollableX ? 16 : 0),
+  }
+
+  const point = {
+    x: event.clientX,
+    y: event.clientY,
+  }
 
-  const isScrollableX = target.scrollWidth > target.clientWidth
-  const onScrollbarX = isScrollableX && event.clientY > target.clientHeight
+  if (!isPointInRect(rect, point)) return false
 
   return onScrollbarY || onScrollbarX
 }
@@ -98,17 +116,27 @@ function trackInteractOutsideImpl(node: MaybeElement, options: InteractOutsideOp
   function isEventOutside(event: Event): boolean {
     const target = getEventTarget(event)
     if (!isHTMLElement(target)) return false
+
     // ignore disconnected nodes (removed from DOM)
     if (!target.isConnected) return false
+
     // ignore nodes that are inside the component
     if (contains(node, target)) return false
+
     // Ex: password manager selection
     if (isEventPointWithin(node, event)) return false
+
     // Ex: page content that is scrollable
-    if (isEventWithinScrollbar(event, target)) return false
+    const triggerEl = doc.querySelector(`[aria-controls="${node!.id}"]`)
+    if (triggerEl) {
+      const triggerAncestor = getNearestOverflowAncestor(triggerEl)
+      if (isEventWithinScrollbar(event, triggerAncestor)) return false
+    }
+
     // Ex: dialog positioner that is scrollable
-    const scrollParent = getNearestOverflowAncestor(node!)
-    if (isEventWithinScrollbar(event, scrollParent)) return false
+    const nodeAncestor = getNearestOverflowAncestor(node!)
+    if (isEventWithinScrollbar(event, nodeAncestor)) return false
+
     // Custom exclude function
     return !exclude?.(target)
   }

playwright.config.ts

@@ -50,7 +50,7 @@ export default defineConfig({
   expect: { timeout: 10_000 },
   forbidOnly: !!CI,
   reportSlowTests: null,
-  retries: process.env.CI ? 2 : 0,
+  retries: process.env.CI ? 2 : 1,
   workers: process.env.CI ? 1 : "50%",
   reporter: [
     process.env.CI ? ["github", ["junit", { outputFile: "e2e/junit.xml" }]] : ["list"],