diff --git a/core/http/react-ui/e2e/manage-action-menu-position.spec.js b/core/http/react-ui/e2e/manage-action-menu-position.spec.js new file mode 100644 index 000000000..3f4301abe --- /dev/null +++ b/core/http/react-ui/e2e/manage-action-menu-position.spec.js @@ -0,0 +1,50 @@ +import { test, expect } from './coverage-fixtures.js' + +// Regression: opening a row's kebab (ActionMenu) on /app/manage used to snap +// the page scroll to the top and render the menu detached from its trigger, +// making it impossible to operate. Two causes: the menu auto-focus scrolled +// the page (no preventScroll), and the position:fixed popover was rendered +// inside a row whose hover `transform` re-anchored it. Fix portals the popover +// to document.body, positions it before paint, and focuses without scrolling. +test.describe('Manage Page - Action menu positioning', () => { + test('opening a row menu keeps scroll stable and places the menu by its trigger', async ({ page }) => { + // Small viewport so the page is scrollable and a scroll jump is observable. + await page.setViewportSize({ width: 1024, height: 500 }) + await page.goto('/app/manage') + await expect(page.locator('.table')).toBeVisible({ timeout: 10_000 }) + + const trigger = page.locator('button.action-menu__trigger').first() + await expect(trigger).toBeVisible() + + // Bring the trigger into view ourselves first, so the only scroll we then + // measure is the one the menu would (wrongly) cause - not Playwright's own + // scroll-into-view before the click. + await trigger.scrollIntoViewIfNeeded() + const scrollBefore = await page.evaluate(() => window.scrollY) + await trigger.click() + + const menu = page.locator('[role="menu"]') + await expect(menu).toBeVisible() + + // Behavioural symptom 1: focusing the menu must not yank the page scroll. + const scrollAfter = await page.evaluate(() => window.scrollY) + expect(scrollAfter).toBe(scrollBefore) + + // Behavioural symptom 2: the menu must sit next to its trigger, not float + // at the top of the window where it can't be operated. + const triggerBox = await trigger.boundingBox() + const menuBox = await menu.boundingBox() + expect(triggerBox).not.toBeNull() + expect(menuBox).not.toBeNull() + // Menu top is within ~24px of the trigger's bottom (below) or above it + // (flipped) — in all cases it tracks the trigger, never floating at y≈0. + const tracksTrigger = + Math.abs(menuBox.y - (triggerBox.y + triggerBox.height)) < 24 || + Math.abs((menuBox.y + menuBox.height) - triggerBox.y) < 24 + expect(tracksTrigger).toBe(true) + + // Mechanism: the popover must be portaled to document.body so position:fixed + // resolves against the viewport, not a transformed ancestor row. + await expect(page.locator('body > .popover')).toHaveCount(1) + }) +}) diff --git a/core/http/react-ui/src/components/ActionMenu.jsx b/core/http/react-ui/src/components/ActionMenu.jsx index 5c58ecd78..55010102c 100644 --- a/core/http/react-ui/src/components/ActionMenu.jsx +++ b/core/http/react-ui/src/components/ActionMenu.jsx @@ -95,9 +95,11 @@ export default function ActionMenu({ items, ariaLabel = 'Actions', triggerLabel, className="action-menu" onKeyDown={handleMenuKeyDown} // Capture focus when the menu opens so arrow keys work without the - // user clicking inside first. + // user clicking inside first. preventScroll: the popover is portaled + // and positioned by the trigger rect, so focusing it must not scroll + // the page (that yanked the view to the top before it was placed). tabIndex={-1} - ref={el => { if (el && open) el.focus() }} + ref={el => { if (el && open) el.focus({ preventScroll: true }) }} > {visible.map((item, i) => { if (item.divider) { diff --git a/core/http/react-ui/src/components/Popover.jsx b/core/http/react-ui/src/components/Popover.jsx index 96a9e217e..7d002d348 100644 --- a/core/http/react-ui/src/components/Popover.jsx +++ b/core/http/react-ui/src/components/Popover.jsx @@ -1,10 +1,17 @@ -import { useEffect, useRef, useState, useCallback } from 'react' +import { useEffect, useLayoutEffect, useRef, useState, useCallback } from 'react' +import { createPortal } from 'react-dom' // Minimal popover: positions itself below-right of the trigger's bounding box, // flips above when there isn't room below, closes on outside click or Escape, // returns focus to the trigger. Uses the existing .card surface so it picks // up theme/border/shadow automatically — no new theming work. // +// Rendered through a portal on document.body: the popover is position:fixed and +// positioned from the trigger's viewport rect, so it must escape any ancestor +// that establishes a containing block (a row/card with a hover `transform` +// would otherwise re-anchor `position:fixed` to itself, throwing the menu to +// the wrong spot and making it unusable). +// // Props: // anchor: ref to the trigger DOMElement (required) // open: boolean @@ -30,7 +37,9 @@ export default function Popover({ anchor, open, onClose, children, ariaLabel }) setPos({ top, left: Math.max(8, left), flipped }) }, [anchor]) - useEffect(() => { + // useLayoutEffect so we measure + place the popover before the browser + // paints — otherwise it flashes at its initial {0,0} for a frame. + useLayoutEffect(() => { if (!open) return reposition() window.addEventListener('resize', reposition) @@ -65,14 +74,15 @@ export default function Popover({ anchor, open, onClose, children, ariaLabel }) if (!open && anchor?.current) { // requestAnimationFrame so the close is painted before focus jumps; // otherwise screen readers announce the trigger mid-transition. - const raf = requestAnimationFrame(() => anchor.current?.focus?.()) + // preventScroll: focusing the trigger must not yank the page scroll. + const raf = requestAnimationFrame(() => anchor.current?.focus?.({ preventScroll: true })) return () => cancelAnimationFrame(raf) } }, [open, anchor]) if (!open) return null - return ( + return createPortal(
{children} -
+ , + document.body ) }