Skip to content

Commit f73edb3

Browse files
committed
refactor: overflow + interaction outside
1 parent 5fa08c5 commit f73edb3

File tree

6 files changed

+150
-20
lines changed

6 files changed

+150
-20
lines changed

.changeset/lazy-ways-hunt.md

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@zag-js/interact-outside": patch
3+
"@zag-js/menu": patch
4+
---
5+
6+
Fix issue where interaction outside detection doesn't work consistently when trigger is within a scrollable container.
+101
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
import * as menu from "@zag-js/menu"
2+
import { normalizeProps, useMachine } from "@zag-js/react"
3+
import { useId } from "react"
4+
5+
const Menu = () => {
6+
const [state, send] = useMachine(
7+
menu.machine({
8+
id: useId(),
9+
positioning: { hideWhenDetached: true },
10+
}),
11+
)
12+
13+
const api = menu.connect(state, send, normalizeProps)
14+
15+
return (
16+
<div>
17+
<button {...api.getTriggerProps()}>
18+
Actions <span {...api.getIndicatorProps()}></span>
19+
</button>
20+
{api.open && (
21+
<div {...api.getPositionerProps()}>
22+
<ul {...api.getContentProps()}>
23+
<li {...api.getItemProps({ value: "edit" })}>Edit</li>
24+
<li {...api.getItemProps({ value: "duplicate" })}>Duplicate</li>
25+
<li {...api.getItemProps({ value: "delete" })}>Delete</li>
26+
<li {...api.getItemProps({ value: "export" })}>Export...</li>
27+
</ul>
28+
</div>
29+
)}
30+
</div>
31+
)
32+
}
33+
34+
const HorizontalScrollBox = () => {
35+
return (
36+
<div style={{ display: "flex", justifyContent: "center", alignItems: "center" }}>
37+
<div
38+
style={{
39+
display: "flex",
40+
width: "300px",
41+
height: "100%",
42+
overflowX: "scroll",
43+
columnGap: "24px",
44+
flexShrink: 0,
45+
padding: "16px",
46+
border: "1px solid #ccc",
47+
backgroundColor: "#f0f0f0",
48+
}}
49+
>
50+
{[...Array(6).keys()].map((x) => (
51+
<div
52+
key={x}
53+
style={{
54+
backgroundColor: "lightgray",
55+
borderRadius: "4px",
56+
padding: "16px",
57+
whiteSpace: "nowrap",
58+
}}
59+
>
60+
Item {x}
61+
</div>
62+
))}
63+
<div>
64+
<Menu />
65+
</div>
66+
</div>
67+
</div>
68+
)
69+
}
70+
71+
const VerticalScrollBox = () => {
72+
return (
73+
<div style={{ backgroundColor: "#ffffff", border: "1px solid black" }}>
74+
<div
75+
style={{
76+
display: "flex",
77+
flexDirection: "column",
78+
gap: "16px",
79+
alignItems: "center",
80+
maxHeight: "200px",
81+
width: "200px",
82+
overflowY: "auto",
83+
border: "1px solid black",
84+
}}
85+
>
86+
{[...Array(12).keys()].map((x) => (
87+
<Menu key={x} />
88+
))}
89+
</div>
90+
</div>
91+
)
92+
}
93+
94+
export default function Page() {
95+
return (
96+
<main style={{ height: "100vh", display: "flex", justifyContent: "center", alignItems: "center", gap: "40px" }}>
97+
<HorizontalScrollBox />
98+
<VerticalScrollBox />
99+
</main>
100+
)
101+
}

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

+5-5
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ export function machine(userContext: UserDefinedContext) {
4242
lastHighlightedValue: null,
4343
children: cast(ref({})),
4444
suspendPointer: false,
45-
restoreFocus: true,
4645
typeaheadState: getByTypeahead.defaultOptions,
4746
},
4847

@@ -489,6 +488,7 @@ export function machine(userContext: UserDefinedContext) {
489488
},
490489
trackInteractOutside(ctx, _evt, { send }) {
491490
const getContentEl = () => dom.getContentEl(ctx)
491+
let restoreFocus = true
492492
return trackDismissableElement(getContentEl, {
493493
defer: true,
494494
exclude: [dom.getTriggerEl(ctx)],
@@ -500,11 +500,11 @@ export function machine(userContext: UserDefinedContext) {
500500
closeRootMenu(ctx)
501501
},
502502
onPointerDownOutside(event) {
503-
ctx.restoreFocus = !event.detail.focusable
503+
restoreFocus = !event.detail.focusable
504504
ctx.onPointerDownOutside?.(event)
505505
},
506506
onDismiss() {
507-
send({ type: "CLOSE", src: "interact-outside" })
507+
send({ type: "CLOSE", src: "interact-outside", restoreFocus })
508508
},
509509
})
510510
},
@@ -659,8 +659,8 @@ export function machine(userContext: UserDefinedContext) {
659659
if (!ctx.highlightedValue) return
660660
ctx.onSelect?.({ value: ctx.highlightedValue })
661661
},
662-
focusTrigger(ctx) {
663-
if (ctx.isSubmenu || ctx.anchorPoint || !ctx.restoreFocus) return
662+
focusTrigger(ctx, evt) {
663+
if (ctx.isSubmenu || ctx.anchorPoint || evt.restoreFocus === false) return
664664
queueMicrotask(() => dom.getTriggerEl(ctx)?.focus({ preventScroll: true }))
665665
},
666666
highlightMatchedItem(ctx, evt) {

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

-5
Original file line numberDiff line numberDiff line change
@@ -164,11 +164,6 @@ interface PrivateContext {
164164
* The typeahead state for faster keyboard navigation
165165
*/
166166
typeaheadState: TypeaheadState
167-
/**
168-
* @internal
169-
* Whether to return focus to the trigger when the menu is closed
170-
*/
171-
restoreFocus?: boolean | undefined
172167
}
173168

174169
export interface MachineContext extends PublicContext, PrivateContext, ComputedContext {}

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

+37-9
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,32 @@ function isEventPointWithin(node: MaybeElement, event: Event) {
7373
)
7474
}
7575

76-
function isEventWithinScrollbar(event: Event, target: HTMLElement): boolean {
77-
if (!target || !isPointerEvent(event)) return false
76+
function isPointInRect(rect: { x: number; y: number; width: number; height: number }, point: { x: number; y: number }) {
77+
return rect.y <= point.y && point.y <= rect.y + rect.height && rect.x <= point.x && point.x <= rect.x + rect.width
78+
}
79+
80+
function isEventWithinScrollbar(event: Event, ancestor: HTMLElement): boolean {
81+
if (!ancestor || !isPointerEvent(event)) return false
82+
83+
const isScrollableY = ancestor.scrollHeight > ancestor.clientHeight
84+
const onScrollbarY = isScrollableY && event.clientX > ancestor.offsetLeft + ancestor.clientWidth
85+
86+
const isScrollableX = ancestor.scrollWidth > ancestor.clientWidth
87+
const onScrollbarX = isScrollableX && event.clientY > ancestor.offsetTop + ancestor.clientHeight
7888

79-
const isScrollableY = target.scrollHeight > target.clientHeight
80-
const onScrollbarY = isScrollableY && event.clientX > target.clientWidth
89+
const rect = {
90+
x: ancestor.offsetLeft,
91+
y: ancestor.offsetTop,
92+
width: ancestor.clientWidth + (isScrollableY ? 16 : 0),
93+
height: ancestor.clientHeight + (isScrollableX ? 16 : 0),
94+
}
95+
96+
const point = {
97+
x: event.clientX,
98+
y: event.clientY,
99+
}
81100

82-
const isScrollableX = target.scrollWidth > target.clientWidth
83-
const onScrollbarX = isScrollableX && event.clientY > target.clientHeight
101+
if (!isPointInRect(rect, point)) return false
84102

85103
return onScrollbarY || onScrollbarX
86104
}
@@ -98,17 +116,27 @@ function trackInteractOutsideImpl(node: MaybeElement, options: InteractOutsideOp
98116
function isEventOutside(event: Event): boolean {
99117
const target = getEventTarget(event)
100118
if (!isHTMLElement(target)) return false
119+
101120
// ignore disconnected nodes (removed from DOM)
102121
if (!target.isConnected) return false
122+
103123
// ignore nodes that are inside the component
104124
if (contains(node, target)) return false
125+
105126
// Ex: password manager selection
106127
if (isEventPointWithin(node, event)) return false
128+
107129
// Ex: page content that is scrollable
108-
if (isEventWithinScrollbar(event, target)) return false
130+
const triggerEl = doc.querySelector(`[aria-controls="${node!.id}"]`)
131+
if (triggerEl) {
132+
const triggerAncestor = getNearestOverflowAncestor(triggerEl)
133+
if (isEventWithinScrollbar(event, triggerAncestor)) return false
134+
}
135+
109136
// Ex: dialog positioner that is scrollable
110-
const scrollParent = getNearestOverflowAncestor(node!)
111-
if (isEventWithinScrollbar(event, scrollParent)) return false
137+
const nodeAncestor = getNearestOverflowAncestor(node!)
138+
if (isEventWithinScrollbar(event, nodeAncestor)) return false
139+
112140
// Custom exclude function
113141
return !exclude?.(target)
114142
}

playwright.config.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export default defineConfig({
5050
expect: { timeout: 10_000 },
5151
forbidOnly: !!CI,
5252
reportSlowTests: null,
53-
retries: process.env.CI ? 2 : 0,
53+
retries: process.env.CI ? 2 : 1,
5454
workers: process.env.CI ? 1 : "50%",
5555
reporter: [
5656
process.env.CI ? ["github", ["junit", { outputFile: "e2e/junit.xml" }]] : ["list"],

0 commit comments

Comments
 (0)