Skip to content

Commit dd3e23d

Browse files
committed
fix(app): guard sibling submenu opens
1 parent c20e019 commit dd3e23d

2 files changed

Lines changed: 194 additions & 15 deletions

File tree

‎packages/app/e2e/regression/tabs.spec.ts‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1246,9 +1246,9 @@ test("the Windows titlebar menu pastes through the desktop edit action", async (
12461246
await expect(page.getByRole("status", { name: "Desktop menu actions" })).toHaveText("edit.paste")
12471247
})
12481248

1249-
// A submenu must survive the pointer taking the natural path to its last item: down the parent menu
1250-
// first, then across. Moving down crosses the parent's other items, which closes the submenu before
1251-
// the pointer reaches the last row.
1249+
// A submenu must survive a normal-speed diagonal move from its trigger to the last item.
1250+
// Kobalte lets a sibling open while the pointer crosses it; the settle guard keeps the active
1251+
// submenu until the pointer actually stops on the sibling.
12521252
test("the Windows titlebar menu keeps a submenu open while the pointer moves to its last item", async ({ page }) => {
12531253
await mockWorkspace(page, { name: "WindowsMenuHover", sessions: [] })
12541254
await page.goto(`/e2e/utils/windows-menu.html?${new URLSearchParams({ server: SERVER })}`)
@@ -1265,24 +1265,41 @@ test("the Windows titlebar menu keeps a submenu open while the pointer moves to
12651265

12661266
if (!trigger || !last) throw new Error("Expected visible Edit trigger and Select All item")
12671267

1268-
const x = trigger.x + trigger.width / 2
1269-
const y = last.y + last.height / 2
1268+
const start = { x: trigger.x + trigger.width / 2, y: trigger.y + trigger.height / 2 }
1269+
const end = { x: last.x + last.width / 2, y: last.y + last.height / 2 }
1270+
const steps = 25
12701271

1271-
await page.mouse.move(x, trigger.y + trigger.height / 2)
1272-
await page.mouse.move(x, y, { steps: 10 })
1273-
await expect(edit).toHaveAttribute("aria-expanded", "true")
1272+
await page.mouse.move(start.x, start.y)
1273+
1274+
for (let step = 1; step <= steps; step++) {
1275+
const progress = step / steps
12741276

1275-
await page.mouse.move(last.x + last.width / 2, y, { steps: 10 })
1277+
await page.mouse.move(start.x + (end.x - start.x) * progress, start.y + (end.y - start.y) * progress)
1278+
// The issue reproduces at normal hand speed; this paces input, it is not a readiness wait.
1279+
await page.waitForTimeout(16)
1280+
}
1281+
1282+
await expect(edit).toHaveAttribute("aria-expanded", "true")
12761283
await expect(selectAll).toBeVisible()
12771284
await expect(selectAll).toHaveAttribute("data-highlighted", "")
12781285

1286+
// A deliberate hover on a sibling still switches the submenu.
1287+
const view = page.getByRole("menuitem", { name: "View", exact: true })
1288+
const viewBox = await view.boundingBox()
1289+
1290+
if (!viewBox) throw new Error("Expected visible View trigger")
1291+
1292+
await page.mouse.move(viewBox.x + viewBox.width / 2, viewBox.y + viewBox.height / 2)
1293+
await expect(view).toHaveAttribute("aria-expanded", "true")
1294+
await expect(edit).toHaveAttribute("aria-expanded", "false")
1295+
12791296
// Leaving the menu still closes the submenu; the grace belongs to the traversal, not a pinned menu.
12801297
const outside = await page.getByRole("main").boundingBox()
12811298

12821299
if (!outside) throw new Error("Expected visible main content")
12831300

12841301
await page.mouse.move(outside.x + outside.width / 2, outside.y + outside.height / 2)
1285-
await expect(edit).toHaveAttribute("aria-expanded", "false")
1302+
await expect(view).toHaveAttribute("aria-expanded", "false")
12861303
await expect(selectAll).toBeHidden()
12871304
})
12881305

‎packages/app/src/shell/titlebar/windows-menu.tsx‎

Lines changed: 167 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ import { useMenubarItems } from "@/runtime/plugin/menubar-items"
2323
// Keep the submenu through that crossing and close it only after the pointer leaves both menus.
2424
const SUBMENU_CLOSE_DELAY = 300
2525

26+
// A sibling's open request is held while the pointer is crossing the parent menu toward the active
27+
// submenu. It is accepted after the pointer rests on the sibling.
28+
const SUBMENU_OPEN_SETTLE = 150
29+
2630
const accelerators = DESKTOP_MENU.flatMap((menu) => menu.items ?? []).flatMap((entry) => {
2731
if (entry.type === "separator" || !entry.action || !entry.accelerator?.windows) return []
2832

@@ -36,6 +40,60 @@ export function windowsMenuAccelerator(event: KeyboardEvent) {
3640
return accelerators.find((entry) => matchKeybind(entry.keybind, event))?.action
3741
}
3842

43+
function pointInPolygon(x: number, y: number, polygon: readonly (readonly [number, number])[]) {
44+
let inside = false
45+
46+
for (let index = 0, previous = polygon.length - 1; index < polygon.length; previous = index++) {
47+
const [xi, yi] = polygon[index]!
48+
const [xj, yj] = polygon[previous]!
49+
50+
if (yi > y !== yj > y && x < ((xj - xi) * (y - yi)) / (yj - yi) + xi) inside = !inside
51+
}
52+
53+
return inside
54+
}
55+
56+
// The corridor between a trigger and its submenu content. It starts at the trigger's far edge so a
57+
// normal diagonal from the parent item into the last row stays inside it.
58+
function intentPolygon(trigger: DOMRect, content: DOMRect): [number, number][] {
59+
const triggerX = trigger.left + trigger.width / 2
60+
const triggerY = trigger.top + trigger.height / 2
61+
const contentX = content.left + content.width / 2
62+
const contentY = content.top + content.height / 2
63+
64+
if (Math.abs(contentX - triggerX) >= Math.abs(contentY - triggerY)) {
65+
if (contentX >= triggerX)
66+
return [
67+
[trigger.left, trigger.top],
68+
[trigger.left, trigger.bottom],
69+
[content.left, content.bottom],
70+
[content.left, content.top],
71+
]
72+
73+
return [
74+
[trigger.right, trigger.top],
75+
[trigger.right, trigger.bottom],
76+
[content.right, content.bottom],
77+
[content.right, content.top],
78+
]
79+
}
80+
81+
if (contentY >= triggerY)
82+
return [
83+
[trigger.left, trigger.top],
84+
[trigger.right, trigger.top],
85+
[content.right, content.top],
86+
[content.left, content.top],
87+
]
88+
89+
return [
90+
[trigger.left, trigger.bottom],
91+
[trigger.right, trigger.bottom],
92+
[content.right, content.bottom],
93+
[content.left, content.bottom],
94+
]
95+
}
96+
3997
export function WindowsAppMenu(props: {
4098
command: ReturnType<typeof useCommand>
4199
platform: ReturnType<typeof usePlatform>
@@ -45,10 +103,15 @@ export function WindowsAppMenu(props: {
45103
const menubar = useMenubarItems()
46104
const [activeSubmenu, setActiveSubmenu] = createSignal<string>()
47105
const submenuContents = new Map<string, HTMLElement>()
106+
const submenuTriggers = new Map<string, HTMLElement>()
48107
let menuContent: HTMLElement | undefined
49108
let pointer = { x: -1, y: -1 }
50109
let lastInput: "pointer" | "keyboard" = "pointer"
110+
let lastPointerDownAt = Number.NEGATIVE_INFINITY
111+
let pressedSubmenu: string | undefined
51112
let closeTimer: number | undefined
113+
let pendingSubmenu: string | undefined
114+
let openTimer: number | undefined
52115

53116
const pointerInside = (element: HTMLElement | undefined) => {
54117
if (!element?.isConnected) return false
@@ -66,13 +129,46 @@ export function WindowsAppMenu(props: {
66129
return active !== undefined && pointerInside(submenuContents.get(active))
67130
}
68131

132+
const pointerInsideIntent = (id: string) => {
133+
const trigger = submenuTriggers.get(id)
134+
const content = submenuContents.get(id)
135+
136+
if (!trigger?.isConnected || !content?.isConnected) return false
137+
138+
const triggerRect = trigger.getBoundingClientRect()
139+
const contentRect = content.getBoundingClientRect()
140+
141+
if (!triggerRect.width || !contentRect.width) return false
142+
143+
return pointInPolygon(pointer.x, pointer.y, intentPolygon(triggerRect, contentRect))
144+
}
145+
146+
const cancelPendingOpen = () => {
147+
if (openTimer !== undefined) window.clearTimeout(openTimer)
148+
openTimer = undefined
149+
pendingSubmenu = undefined
150+
}
151+
152+
const schedulePendingOpen = () => {
153+
if (openTimer !== undefined) window.clearTimeout(openTimer)
154+
openTimer = window.setTimeout(() => {
155+
openTimer = undefined
156+
157+
const pending = pendingSubmenu
158+
pendingSubmenu = undefined
159+
160+
if (pending && pointerInside(submenuTriggers.get(pending))) openSubmenu(pending)
161+
}, SUBMENU_OPEN_SETTLE)
162+
}
163+
69164
const cancelSubmenuClose = () => {
70165
if (closeTimer !== undefined) window.clearTimeout(closeTimer)
71166
closeTimer = undefined
72167
}
73168

74169
const closeSubmenu = () => {
75170
cancelSubmenuClose()
171+
cancelPendingOpen()
76172
setActiveSubmenu(undefined)
77173
}
78174

@@ -87,10 +183,13 @@ export function WindowsAppMenu(props: {
87183

88184
const openSubmenu = (id: string) => {
89185
cancelSubmenuClose()
186+
cancelPendingOpen()
90187
setActiveSubmenu(id)
91188
}
92189

93190
const requestSubmenuClose = (id: string) => {
191+
if (pendingSubmenu === id) cancelPendingOpen()
192+
94193
if (activeSubmenu() !== id) return
95194

96195
if (lastInput === "keyboard") return closeSubmenu()
@@ -100,6 +199,24 @@ export function WindowsAppMenu(props: {
100199
scheduleSubmenuClose()
101200
}
102201

202+
const requestSubmenuOpen = (id: string) => {
203+
if (activeSubmenu() === id) return
204+
205+
const active = activeSubmenu()
206+
207+
if (
208+
active === undefined ||
209+
lastInput === "keyboard" ||
210+
(pressedSubmenu === id && performance.now() - lastPointerDownAt < 500)
211+
)
212+
return openSubmenu(id)
213+
214+
if (!pointerInsideIntent(active)) return openSubmenu(id)
215+
216+
pendingSubmenu = id
217+
schedulePendingOpen()
218+
}
219+
103220
const entries = (menu: DesktopMenu) =>
104221
desktopMenuWithPlugins(
105222
(menu.items ?? [])
@@ -172,6 +289,15 @@ export function WindowsAppMenu(props: {
172289
pointer = { x: event.clientX, y: event.clientY }
173290
lastInput = "pointer"
174291

292+
if (pendingSubmenu !== undefined) {
293+
const pending = pendingSubmenu
294+
const active = activeSubmenu()
295+
296+
if (!pointerInside(submenuTriggers.get(pending))) cancelPendingOpen()
297+
else if (active !== undefined && !pointerInsideIntent(active)) openSubmenu(pending)
298+
else schedulePendingOpen()
299+
}
300+
175301
if (activeSubmenu() === undefined) return
176302

177303
if (pointerInsideMenu()) return cancelSubmenuClose()
@@ -181,15 +307,46 @@ export function WindowsAppMenu(props: {
181307
{ capture: true },
182308
)
183309

184-
makeEventListener(window, "pointerdown", () => (lastInput = "pointer"), { capture: true })
185-
makeEventListener(window, "keydown", () => (lastInput = "keyboard"), { capture: true })
310+
makeEventListener(
311+
window,
312+
"pointerdown",
313+
(event) => {
314+
const target = event.target
315+
316+
pointer = { x: event.clientX, y: event.clientY }
317+
lastInput = "pointer"
318+
lastPointerDownAt = performance.now()
319+
pressedSubmenu =
320+
target instanceof Node
321+
? Array.from(submenuTriggers).find(([, trigger]) => trigger.contains(target))?.[0]
322+
: undefined
323+
324+
if (pendingSubmenu !== undefined && pointerInside(submenuTriggers.get(pendingSubmenu)))
325+
openSubmenu(pendingSubmenu)
326+
},
327+
{ capture: true },
328+
)
329+
330+
makeEventListener(
331+
window,
332+
"keydown",
333+
() => {
334+
lastInput = "keyboard"
335+
cancelPendingOpen()
336+
},
337+
{ capture: true },
338+
)
186339

187340
// The pointer left the window; no further move can keep the submenu open.
188341
makeEventListener(
189342
window,
190343
"pointerout",
191344
(event) => {
192-
if (event.relatedTarget === null && activeSubmenu() !== undefined) scheduleSubmenuClose()
345+
if (event.relatedTarget !== null) return
346+
347+
cancelPendingOpen()
348+
349+
if (activeSubmenu() !== undefined) scheduleSubmenuClose()
193350
},
194351
{ capture: true },
195352
)
@@ -228,11 +385,15 @@ export function WindowsAppMenu(props: {
228385
<DesktopMenuSubmenu
229386
label={language.t(menu.labelKey)}
230387
open={activeSubmenu() === menu.id}
231-
onOpenChange={(open) => (open ? openSubmenu(menu.id) : requestSubmenuClose(menu.id))}
388+
onOpenChange={(open) => (open ? requestSubmenuOpen(menu.id) : requestSubmenuClose(menu.id))}
232389
onContent={(element) => {
233390
if (element) submenuContents.set(menu.id, element)
234391
else submenuContents.delete(menu.id)
235392
}}
393+
onTrigger={(element) => {
394+
if (element) submenuTriggers.set(menu.id, element)
395+
else submenuTriggers.delete(menu.id)
396+
}}
236397
>
237398
<For each={entries(menu)}>
238399
{(item) => {
@@ -284,11 +445,12 @@ function DesktopMenuSubmenu(props: {
284445
open: boolean
285446
onOpenChange: (open: boolean) => void
286447
onContent: (element: HTMLElement | undefined) => void
448+
onTrigger: (element: HTMLElement | undefined) => void
287449
children: JSX.Element
288450
}) {
289451
return (
290452
<Menu.Sub open={props.open} onOpenChange={props.onOpenChange}>
291-
<Menu.SubTrigger>{props.label}</Menu.SubTrigger>
453+
<Menu.SubTrigger ref={(element: HTMLElement) => props.onTrigger(element)}>{props.label}</Menu.SubTrigger>
292454
<Menu.Portal>
293455
<Menu.SubContent
294456
ref={(element: HTMLElement) => props.onContent(element)}

0 commit comments

Comments
 (0)