Context
Context menus should close on outside-click and Escape. They should also close on scroll (because they remain anchored to the click coordinates while the grid/sidebar scrolls beneath them).
Problem
useContextMenu opens a context menu via showContextMenu(e, items). The auto-close hooks (in src/components/common/ContextMenu.tsx:36-52) are:
useEffect(() => {
const handleKey = (e: KeyboardEvent) => {
if (e.key === "Escape") onClose();
};
const handleClick = (e: MouseEvent) => {
if (menuRef.current && !menuRef.current.contains(e.target as Node)) {
onClose();
}
};
document.addEventListener("keydown", handleKey);
document.addEventListener("mousedown", handleClick);
return () => { ... };
}, [onClose]);
There is no listener for:
- Scroll — the menu stays anchored at
x, y while the user scrolls the grid/sidebar; it ends up offscreen with no obvious way to close it.
- Resize — same problem if the window resizes.
- Visibility change — if the OS hides the window, the menu stays open invisibly.
The single-result test that mocks useContextMenu (src/components/layout/__tests__/Sidebar.browser.test.tsx:806 — "No crash means pass — context menu is handled by useContextMenu mock") does not exercise real scroll behaviour. There is no integration test for scroll.
The cross-cutting audit (docs/audits/cross-cutting.md F10) flagged this as P1.
Files
- src/components/common/ContextMenu.tsx:36-52 — only Escape + outside-click
- src/hooks/useContextMenu.ts:1-36 — no scroll logic
Repro
- Right-click a row in the ResultsGrid → context menu opens at (200, 400).
- Scroll the grid down by 100 px → menu still anchored at (200, 400) but the row is now at y=300.
- Menu overlaps a different row; user clicks the menu thinking it applies to that row → applies to the original row instead.
Expected
Menu closes on scroll, resize, and visibility change. The next scroll/move leaves no orphaned menu.
Proposed fix
Scope S. In ContextMenu.tsx, add to the existing useEffect:
const handleScroll = () => onClose();
const handleResize = () => onClose();
const handleVisibility = () => { if (document.visibilityState === 'hidden') onClose(); };
window.addEventListener("scroll", handleScroll, { passive: true, capture: true });
window.addEventListener("resize", handleResize);
document.addEventListener("visibilitychange", handleVisibility);
return () => {
window.removeEventListener("scroll", handleScroll, { capture: true });
window.removeEventListener("resize", handleResize);
document.removeEventListener("visibilitychange", handleVisibility);
// existing keydown + mousedown cleanup
};
{ capture: true } ensures the listener fires for scroll on any descendant.
Acceptance
A test mounts <ContextMenu />, dispatches a wheel/scroll event on a child, asserts the menu's onClose spy fires. Same for resize and visibilitychange.
Needs human verify
Yes (browser scroll behaviour needs jsdom + event simulation).
Labels: audit, area/cross-cutting, severity/p1, kind/bug
Context
Context menus should close on outside-click and Escape. They should also close on scroll (because they remain anchored to the click coordinates while the grid/sidebar scrolls beneath them).
Problem
useContextMenuopens a context menu viashowContextMenu(e, items). The auto-close hooks (insrc/components/common/ContextMenu.tsx:36-52) are:There is no listener for:
x, ywhile the user scrolls the grid/sidebar; it ends up offscreen with no obvious way to close it.The single-result test that mocks
useContextMenu(src/components/layout/__tests__/Sidebar.browser.test.tsx:806— "No crash means pass — context menu is handled by useContextMenu mock") does not exercise real scroll behaviour. There is no integration test for scroll.The cross-cutting audit (docs/audits/cross-cutting.md F10) flagged this as P1.
Files
Repro
Expected
Menu closes on scroll, resize, and visibility change. The next scroll/move leaves no orphaned menu.
Proposed fix
Scope S. In
ContextMenu.tsx, add to the existing useEffect:{ capture: true }ensures the listener fires for scroll on any descendant.Acceptance
A test mounts
<ContextMenu />, dispatches awheel/scrollevent on a child, asserts the menu'sonClosespy fires. Same forresizeandvisibilitychange.Needs human verify
Yes (browser scroll behaviour needs jsdom + event simulation).
Labels: audit, area/cross-cutting, severity/p1, kind/bug