diff --git a/apps/visual-regression/playwright.config.ts b/apps/visual-regression/playwright.config.ts index b3641cfd3..6c15211c3 100644 --- a/apps/visual-regression/playwright.config.ts +++ b/apps/visual-regression/playwright.config.ts @@ -9,8 +9,8 @@ import { defineConfig, devices } from '@playwright/test'; * baselines can coexist — regenerate the Linux set in the official Playwright Docker image when * wiring CI. */ -const PORT = 4400; -const BASE_URL = `http://localhost:${PORT}`; +const PORT = Number(process.env['PLAYWRIGHT_PORT'] ?? 4400); +const BASE_URL = process.env['PLAYWRIGHT_BASE_URL'] ?? `http://localhost:${PORT}`; export default defineConfig({ testDir: './tests', @@ -38,10 +38,12 @@ export default defineConfig({ use: { ...devices['Desktop Chrome'] } } ], - webServer: { - command: `pnpm exec http-server ../../dist/radix-storybook -p ${PORT} -s -c-1`, - url: `${BASE_URL}/index.json`, - reuseExistingServer: !process.env.CI, - timeout: 120_000 - } + webServer: process.env['PLAYWRIGHT_NO_WEBSERVER'] + ? undefined + : { + command: `pnpm exec http-server ../../dist/radix-storybook -p ${PORT} -s -c-1`, + url: `${BASE_URL}/index.json`, + reuseExistingServer: !process.env.CI, + timeout: 120_000 + } }); diff --git a/apps/visual-regression/tests/autocomplete.behavior.spec.ts b/apps/visual-regression/tests/autocomplete.behavior.spec.ts index c7c613010..a354638a0 100644 --- a/apps/visual-regression/tests/autocomplete.behavior.spec.ts +++ b/apps/visual-regression/tests/autocomplete.behavior.spec.ts @@ -14,6 +14,42 @@ const visibleItems = '[rdxAutocompleteItem]:not([hidden])'; const highlighted = '[rdxAutocompleteItem][data-highlighted]'; const popup = '[rdxAutocompletePopup]'; +/** + * ADR 0015/0017 Phase-4 migration of Autocomplete onto the new floating dismissal engine. + */ +test.describe('Autocomplete — new floating engine migration', () => { + test('Escape closes the autocomplete', async ({ page }) => { + await gotoStory(page, 'primitives-autocomplete--default'); + await page.locator(input).click(); + await page.locator(input).pressSequentially('f'); + await expect(page.locator(popup)).toBeVisible(); + + await page.keyboard.press('Escape'); + await expect(page.locator(popup)).toHaveCount(0); + }); + + test('an outside press closes the autocomplete', async ({ page }) => { + await gotoStory(page, 'primitives-autocomplete--default'); + await page.locator(input).click(); + await page.locator(input).pressSequentially('f'); + await expect(page.locator(popup)).toBeVisible(); + + await page.mouse.click(5, 5); + await expect(page.locator(popup)).toHaveCount(0); + }); + + test('a press on the input keeps the popup open (the input is registered inside)', async ({ page }) => { + await gotoStory(page, 'primitives-autocomplete--default'); + await page.locator(input).click(); + await page.locator(input).pressSequentially('f'); + await expect(page.locator(popup)).toBeVisible(); + + // Clicking the input again must not self-dismiss — it is "inside" the floating layer. + await page.locator(input).click(); + await expect(page.locator(popup)).toBeVisible(); + }); +}); + test.describe('Autocomplete auto highlight', () => { test('typing a full match highlights the item so Enter selects it', async ({ page }) => { await gotoStory(page, 'primitives-autocomplete--auto-highlight'); diff --git a/apps/visual-regression/tests/combobox.behavior.spec.ts b/apps/visual-regression/tests/combobox.behavior.spec.ts index 9c118b230..1e5d3fdfa 100644 --- a/apps/visual-regression/tests/combobox.behavior.spec.ts +++ b/apps/visual-regression/tests/combobox.behavior.spec.ts @@ -30,6 +30,32 @@ test('autocomplete teleports the positioner directly into with no wrapper expect(parentTag).toBe('BODY'); }); +/** + * ADR 0015/0017 Phase-4 migration of Combobox onto the new floating dismissal engine. + */ +test.describe('Combobox — new floating engine migration', () => { + const input = '[rdxComboboxInput]'; + const popup = '[rdxComboboxPopup]'; + + test('Escape closes the combobox', async ({ page }) => { + await gotoStory(page, 'primitives-combobox--default'); + await page.locator(input).click(); + await expect(page.locator(popup)).toBeVisible(); + + await page.keyboard.press('Escape'); + await expect(page.locator(popup)).toHaveCount(0); + }); + + test('an outside press closes the combobox', async ({ page }) => { + await gotoStory(page, 'primitives-combobox--default'); + await page.locator(input).click(); + await expect(page.locator(popup)).toBeVisible(); + + await page.mouse.click(5, 5); + await expect(page.locator(popup)).toHaveCount(0); + }); +}); + /** * Regression: in multiple mode, once focus has stepped into the chips (ArrowLeft from the input), * ArrowDown / ArrowUp must hand focus back to the input and engage the list — otherwise the popup diff --git a/apps/visual-regression/tests/context-menu.behavior.spec.ts b/apps/visual-regression/tests/context-menu.behavior.spec.ts new file mode 100644 index 000000000..12fbdd123 --- /dev/null +++ b/apps/visual-regression/tests/context-menu.behavior.spec.ts @@ -0,0 +1,67 @@ +import { expect, Page, test } from '@playwright/test'; + +/** + * ADR 0015/0017 Phase-4 migration of Context Menu (composes `RdxMenuRoot`, so it inherits the new + * floating dismissal engine) onto a real browser. Context menus open at the cursor via a virtual + * anchor; these guard that opening + every dismissal path still works and throws no runtime errors. + */ +async function gotoStory(page: Page, storyId: string): Promise { + await page.goto(`/iframe.html?id=${storyId}&viewMode=story`); + await page.waitForSelector('#storybook-root', { state: 'attached' }); +} + +const trigger = '[rdxContextMenuTrigger]'; +const popup = '[rdxMenuPopup]'; + +async function openAtTrigger(page: Page): Promise { + await page.locator(trigger).first().click({ button: 'right' }); + await expect(page.locator(popup)).toBeVisible(); +} + +test('right-click opens the context menu without runtime errors', async ({ page }) => { + const errors: string[] = []; + page.on('pageerror', (e) => errors.push(String(e))); + await gotoStory(page, 'primitives-context-menu--default'); + + await openAtTrigger(page); + expect(errors).toEqual([]); +}); + +test('Escape closes the context menu', async ({ page }) => { + await gotoStory(page, 'primitives-context-menu--default'); + await openAtTrigger(page); + + await page.keyboard.press('Escape'); + await expect(page.locator(popup)).toHaveCount(0); +}); + +test('a modal context menu renders an internal backdrop (finding #1)', async ({ page }) => { + await gotoStory(page, 'primitives-context-menu--default'); + await openAtTrigger(page); + + await expect(page.locator('[data-rdx-menu-internal-backdrop]')).toHaveCount(1); +}); + +test('a modal context menu traps focus — a focus-out does not close it (finding #3)', async ({ page }) => { + await gotoStory(page, 'primitives-context-menu--default'); + await openAtTrigger(page); + + // Programmatically move focus to an element outside the menu. A context menu is the one menu kind + // that TRAPS focus (Base UI `FloatingFocusManager modal`), so focus is pulled back and it stays open. + await page.evaluate(() => { + const b = document.createElement('button'); + b.id = 'cm-outside'; + document.body.appendChild(b); + b.focus(); + }); + await page.waitForTimeout(120); // let the async focus-out check settle + await expect(page.locator(popup)).toBeVisible(); +}); + +test('an outside press closes the context menu', async ({ page }) => { + await gotoStory(page, 'primitives-context-menu--default'); + await openAtTrigger(page); + + await page.mouse.click(5, 5); + await expect(page.locator(popup)).toHaveCount(0); +}); diff --git a/apps/visual-regression/tests/dialog.behavior.spec.ts b/apps/visual-regression/tests/dialog.behavior.spec.ts index e9338e106..3bf04e831 100644 --- a/apps/visual-regression/tests/dialog.behavior.spec.ts +++ b/apps/visual-regression/tests/dialog.behavior.spec.ts @@ -48,33 +48,37 @@ test.describe('Dialog structural portal', () => { await expect(page.locator(popup)).toHaveCount(0); }); - test('holds the scroll lock through the exit animation, releasing it only on unmount', async ({ page }) => { + test('releases the scroll lock at close-start, before the exit animation finishes (Base UI parity)', async ({ + page + }) => { await gotoStory(page, 'primitives-dialog--default'); // Slow the 150ms exit keyframes to a comfortable window so the transitional state below is - // observable without a race (the bug — releasing the lock the instant `isOpen` flips — would - // restore `overflow` synchronously, well before the animation finishes). + // observable without a race: the lock must already be released while the popup is still + // mounted-and-animating-out (`open && modal` gating, not mounted-lifetime gating). await page.addStyleTag({ content: `[rdxDialogBackdrop][data-closed], [rdxDialogPopup][data-closed] { animation-duration: 2000ms !important; }` }); - const htmlOverflow = () => page.locator('html').evaluate((el) => el.style.overflow); + // `useScrollLock` marks `` with `data-rdx-scroll-locked` (strategy-independent). + const scrollLocked = () => page.locator('html').evaluate((el) => el.hasAttribute('data-rdx-scroll-locked')); await page.locator(trigger).first().click(); await expect(page.locator(popup)).toBeVisible(); // Modal dialog locks page scroll while open. - expect(await htmlOverflow()).toBe('hidden'); + expect(await scrollLocked()).toBe(true); await page.locator('[rdxDialogClose][aria-label="Close"]').click(); - // Mid-exit: the popup is closing but still mounted — the scroll lock must stay held so the - // page scrollbar doesn't reappear and reflow the page (the judder) while the dialog animates. + // Mid-exit: the popup is closing but still mounted, yet the scroll lock is already released + // (Base UI gates it on `open && modal === true`). The inset-scrollbar strategy reserves the + // scrollbar gutter, so no content reflow accompanies the release. await expect(page.locator(popup)).toHaveAttribute('data-closed', ''); - expect(await htmlOverflow()).toBe('hidden'); + expect(await scrollLocked()).toBe(false); - // Only once the view unmounts does the lock release and the original overflow return. + // Still released after unmount. await expect(page.locator(popup)).toHaveCount(0); - expect(await htmlOverflow()).toBe(''); + expect(await scrollLocked()).toBe(false); }); }); @@ -132,6 +136,216 @@ test.describe('Dialog outside-scroll (custom scroll area)', () => { }); }); +/** + * ⚠️ **NOT YET VERIFIED — ADR 0015/0017 Phase-4 Dialog migration onto the new floating engine.** These + * are the browser checks the migrated `RdxDialogPopup` must pass before merge (jsdom cannot exercise + * focus trap / live focus / focus-out / aria-hidden). Run with `pnpm test-visual` (or the local loop + * against `:4400`). Some tests below intentionally encode **known gaps** in the first-cut wiring — see + * the `KNOWN GAP` notes; they are expected to FAIL until the wiring is fixed in the verification session. + */ +test.describe('Dialog — new floating engine migration', () => { + const closeButton = '[rdxDialogClose][aria-label="Close"]'; + + const focusInsidePopup = (page: Page) => + page.locator(popup).evaluate((el) => el.contains(el.ownerDocument.activeElement)); + + test('modal dialog moves focus into the popup on open', async ({ page }) => { + await gotoStory(page, 'primitives-dialog--default'); + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + + expect(await focusInsidePopup(page)).toBe(true); + }); + + test('modal dialog traps focus — Tab keeps focus inside the popup', async ({ page }) => { + await gotoStory(page, 'primitives-dialog--default'); + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + + // Tab repeatedly: focus must cycle within the popup, never escaping to the trigger / page. + for (let i = 0; i < 8; i++) { + await page.keyboard.press('Tab'); + expect(await focusInsidePopup(page)).toBe(true); + } + }); + + test('returns focus to the trigger after closing', async ({ page }) => { + await gotoStory(page, 'primitives-dialog--default'); + const triggerEl = page.locator(trigger).first(); + await triggerEl.click(); + await expect(page.locator(popup)).toBeVisible(); + + await page.keyboard.press('Escape'); + await expect(page.locator(popup)).toHaveCount(0); + + await expect(triggerEl).toBeFocused(); + }); + + test('Escape and outside-press still close (dismissal regression)', async ({ page }) => { + await gotoStory(page, 'primitives-dialog--default'); + + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + await page.keyboard.press('Escape'); + await expect(page.locator(popup)).toHaveCount(0); + + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + await page.locator(closeButton).click(); + await expect(page.locator(popup)).toHaveCount(0); + }); + + test('non-modal dialog closes when focus leaves to an unrelated element', async ({ page }) => { + await gotoStory(page, 'primitives-dialog--non-modal'); + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + + // Move focus to a real element OUTSIDE the dialog (relatedTarget set, unrelated node) → the + // focus manager's focus-out close fires (§3). A null relatedTarget (bare blur) does NOT close. + await page.evaluate(() => { + const button = document.createElement('button'); + button.id = 'rdx-focus-out-target'; + document.body.appendChild(button); + button.focus(); + }); + await expect(page.locator(popup)).toHaveCount(0); + }); + + test('an outside press onto an interactive element keeps focus there, not back on the trigger (finding #3)', async ({ + page + }) => { + await gotoStory(page, 'primitives-dialog--non-modal'); + + // A real focusable control outside the non-modal dialog, present before the marker pass. + // Pressing it dismisses the dialog AND moves focus onto it — the return-focus must not yank + // focus back to the trigger. + await page.evaluate(() => { + const button = document.createElement('button'); + button.id = 'rdx-outside-target'; + button.textContent = 'Outside'; + document.body.appendChild(button); + }); + + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + + await page.locator('#rdx-outside-target').click(); + + await expect(page.locator(popup)).toHaveCount(0); + await expect(page.locator('#rdx-outside-target')).toBeFocused(); + }); + + test('nested dialog: Escape closes only the inner dialog (deepest-first ownership)', async ({ page }) => { + await gotoStory(page, 'primitives-dialog--nested'); + await page.locator(trigger).first().click(); + await expect(page.locator(popup).first()).toBeVisible(); + + // Open the nested dialog from inside the first. + await page.locator(popup).first().locator(trigger).first().click(); + await expect(page.locator(popup)).toHaveCount(2); + + // Escape closes the deepest (inner) layer only — the outer stays open. + await page.keyboard.press('Escape'); + await expect(page.locator(popup)).toHaveCount(1); + }); + + test('nested dialog: an outside press closes only the topmost dialog (finding #1)', async ({ page }) => { + await gotoStory(page, 'primitives-dialog--nested'); + await page.locator(trigger).first().click(); + await expect(page.locator(popup).first()).toBeVisible(); + + await page.locator(popup).first().locator(trigger).first().click(); + await expect(page.locator(popup)).toHaveCount(2); + + // A press in the far corner lands on the (parent) backdrop. Only the topmost (inner) dialog + // dismisses — the parent, which has an open nested dialog, must NOT self-close. + await page.mouse.click(5, 5); + await expect(page.locator(popup)).toHaveCount(1); + await expect(page.locator(popup)).toBeVisible(); + }); + + test('marks the dialog backdrop as an outside sibling root', async ({ page }) => { + await gotoStory(page, 'primitives-dialog--default'); + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + + // The backdrop is a second portal root (sibling of the popup), but ADR 0017 keeps the marker + // keep-set narrow: [popup, ...descendantPortalRoots], not own sibling roots. + await expect(page.locator(backdrop)).toHaveAttribute('data-rdx-floating-inert', ''); + }); + + test('a modal dialog inerts outside content, not the global body pointer-events (finding #4)', async ({ page }) => { + await gotoStory(page, 'primitives-dialog--default'); + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + + // No global `body { pointer-events: none }` lock anymore — independent overlays keep working. + expect(await page.evaluate(() => document.body.style.pointerEvents)).toBe(''); + + // Outside content (the app root that holds the trigger) is a body sibling of the portal → it gets + // the real `inert` attribute: non-interactive AND removed from the a11y tree, scoped to it. + expect(await page.locator('#storybook-root').evaluate((el) => el.hasAttribute('inert'))).toBe(true); + + // On close the isolation lifts. + await page.keyboard.press('Escape'); + await expect(page.locator(popup)).toHaveCount(0); + expect(await page.locator('#storybook-root').evaluate((el) => el.hasAttribute('inert'))).toBe(false); + }); + + test('releases marker and inert at close-start, before the exit animation finishes', async ({ page }) => { + await gotoStory(page, 'primitives-dialog--default'); + + // Slow the exit keyframes so the mid-exit state is observable without a race. + await page.addStyleTag({ + content: `[rdxDialogBackdrop][data-closed], [rdxDialogPopup][data-closed] { animation-duration: 2000ms !important; }` + }); + + const rootState = () => + page.locator('#storybook-root').evaluate((el) => ({ + inert: el.hasAttribute('inert'), + marked: el.hasAttribute('data-rdx-floating-inert') + })); + + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + expect(await rootState()).toEqual({ inert: true, marked: true }); + + await page.locator('[rdxDialogClose][aria-label="Close"]').click(); + + // Mid-exit: the popup is closing (`data-closed`) but still mounted. Base UI keeps the focus trap + // mounted, but marker / isolation follow `open` and release at close-start. + await expect(page.locator(popup)).toHaveAttribute('data-closed', ''); + expect(await rootState()).toEqual({ inert: false, marked: false }); + + // Still released after unmount. + await expect(page.locator(popup)).toHaveCount(0); + expect(await rootState()).toEqual({ inert: false, marked: false }); + }); + + test('the backdrop is decorative — role="presentation" (Base UI parity)', async ({ page }) => { + await gotoStory(page, 'primitives-dialog--default'); + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + + await expect(page.locator(backdrop)).toHaveAttribute('role', 'presentation'); + }); + + test('a nested dialog renders no second backdrop (only the parent dims the page)', async ({ page }) => { + await gotoStory(page, 'primitives-dialog--nested'); + await page.locator(trigger).first().click(); + await expect(page.locator(popup).first()).toBeVisible(); + await expect(page.locator(`${backdrop}:visible`)).toHaveCount(1); + + // Open the nested dialog from inside the parent. + await page.locator(popup).first().locator(trigger).first().click(); + await expect(page.locator(popup)).toHaveCount(2); + + // Both backdrops are in the DOM, but the nested one is `[hidden]` — exactly one stays visible. + await expect(page.locator(backdrop)).toHaveCount(2); + await expect(page.locator(`${backdrop}:visible`)).toHaveCount(1); + }); +}); + /** * The "uncontained" story renders the close button outside the visible content card while keeping it * inside the popup (and thus inside the focus trap). Guards the layout (close above the card) and that diff --git a/apps/visual-regression/tests/menu-submenu.behavior.spec.ts b/apps/visual-regression/tests/menu-submenu.behavior.spec.ts index b56db14f0..40f08f143 100644 --- a/apps/visual-regression/tests/menu-submenu.behavior.spec.ts +++ b/apps/visual-regression/tests/menu-submenu.behavior.spec.ts @@ -69,16 +69,118 @@ test('RTL: diagonal traversal toward a left-placed submenu keeps it open', async const submenu = await page.locator(findRtlSubmenu).boundingBox(); if (!find || !submenu) throw new Error('missing layout boxes'); - // Exit from the LEFT edge of "بحث" and descend into the left-placed submenu's near (right) edge, - // past the trigger's bottom so the path crosses the sibling row. This exercises the safe-polygon - // 'left' geometry (the popup resolves `data-side="left"`). Few, large steps keep pointer velocity - // above the slow-cursor close threshold. + // Exit from the LEFT edge of "بحث" into the left-placed submenu's near (right) lower edge. Mirror + // the LTR diagonal in the opposite direction so we still cross the sibling row while staying on a + // realistic path toward the popup, regardless of the popup's measured height. await page.mouse.move(find.x + 4, find.y + find.height / 2); - await page.mouse.move(submenu.x + submenu.width - 8, submenu.y + 70, { steps: 5 }); + await page.mouse.move(submenu.x + submenu.width - 8, submenu.y + submenu.height - 8, { steps: 5 }); await expect(page.locator(findRtlSubmenu)).toBeVisible(); await expect(page.locator(spellingRtlSubmenu)).toHaveCount(0); }); +test('RTL: ArrowLeft opens a submenu trigger and ArrowRight closes it', async ({ page }) => { + const findRtl = '[rdxMenuSubTrigger]:has-text("بحث")'; + const findRtlSubmenu = '[rdxMenuPopup][data-side="left"]:has-text("بحث في الويب")'; + + await gotoStory(page, 'primitives-menu--nested-rtl'); + + const trigger = page.locator('[rdxMenuTrigger]').first(); + const rootItems = page.locator('[rdxMenuPopup]').first().locator('[rdxMenuItem], [rdxMenuSubTrigger]'); + + await trigger.focus(); + await page.keyboard.press('Enter'); + await expect(rootItems.first()).toBeFocused(); + + await page.keyboard.press('ArrowDown'); + await page.keyboard.press('ArrowDown'); + await expect(page.locator(findRtl)).toBeFocused(); + + await page.keyboard.press('ArrowLeft'); + await expect(page.locator(findRtlSubmenu)).toBeVisible(); + await expect(page.locator(findRtlSubmenu).locator('[rdxMenuItem]').first()).toBeFocused(); + + await page.keyboard.press('ArrowRight'); + await expect(page.locator(findRtlSubmenu)).toHaveCount(0); + await expect(page.locator(findRtl)).toBeFocused(); +}); + +test('Escape closes only the deepest submenu, keeping the parent menu open (tree deepest-first)', async ({ page }) => { + await openEditMenu(page); + await openFindSubmenu(page); + + // The Find submenu is the deepest open layer. Escape (a document-level dismissal) closes only it — + // the parent Edit menu stays open because the open submenu node blocks the parent's Escape. + await page.keyboard.press('Escape'); + await expect(page.locator(findSubmenu)).toHaveCount(0); + await expect(page.locator('[rdxMenuPopup]').first()).toBeVisible(); + + // A second Escape now closes the parent. + await page.keyboard.press('Escape'); + await expect(page.locator('[rdxMenuPopup]')).toHaveCount(0); +}); + +test('Enter on a focused submenu trigger moves focus into the nested popup', async ({ page }) => { + await gotoStory(page, 'primitives-menu--nested'); + + const trigger = page.locator('[rdxMenuTrigger]').first(); + const rootItems = page.locator('[rdxMenuPopup]').first().locator('[rdxMenuItem], [rdxMenuSubTrigger]'); + const nestedItems = page.locator(findSubmenu).locator('[rdxMenuItem]'); + + await trigger.focus(); + await page.keyboard.press('Enter'); + await expect(rootItems.first()).toBeFocused(); + + await page.keyboard.press('ArrowDown'); + await page.keyboard.press('ArrowDown'); + await expect(page.locator(findTrigger)).toBeFocused(); + + await page.keyboard.press('Enter'); + await expect(page.locator(findSubmenu)).toBeVisible(); + await expect(nestedItems.first()).toBeFocused(); +}); + +test('a submenu renders no internal backdrop (only the root modal menu does, finding #1)', async ({ page }) => { + await openEditMenu(page); // root menu opened by click → modal → one internal backdrop + await expect(page.locator('[data-rdx-menu-internal-backdrop]')).toHaveCount(1); + + await openFindSubmenu(page); // submenu (parent.type === 'menu') → adds no backdrop of its own + await expect(page.locator('[data-rdx-menu-internal-backdrop]')).toHaveCount(1); +}); + +test('an outside press closes the whole open menu chain (tree containment)', async ({ page }) => { + await openEditMenu(page); + await openFindSubmenu(page); + await expect(page.locator('[rdxMenuPopup]')).toHaveCount(2); + + // A press far outside both popups closes the entire stack — the submenu is logically "inside" the + // parent via the shared floating tree, so neither survives. + await page.mouse.click(5, 5); + await expect(page.locator('[rdxMenuPopup]')).toHaveCount(0); +}); + +test('a mouse click on a hover-opened submenu trigger does not close it (no flicker)', async ({ page }) => { + await openEditMenu(page); + await openFindSubmenu(page); // hover opens the Find submenu + await expect(page.locator(findSubmenu)).toBeVisible(); + + // A real mouse click on the (hover-driven) sub-trigger is ignored, so the submenu stays open instead + // of toggling shut (Base UI `ignoreMouse: openOnHover`). + await page.locator(findTrigger).click(); + await page.waitForTimeout(60); + await expect(page.locator(findSubmenu)).toBeVisible(); +}); + +test('selecting an item inside a submenu closes the whole menu chain, not just the submenu', async ({ page }) => { + await openEditMenu(page); + await openFindSubmenu(page); + await expect(page.locator('[rdxMenuPopup]')).toHaveCount(2); + + // Click an item in the Find submenu — selecting an item dismisses the entire menu (root + submenu), + // not just the innermost popup. + await page.locator(findSubmenu).locator('[rdxMenuItem]').first().click(); + await expect(page.locator('[rdxMenuPopup]')).toHaveCount(0); +}); + test('moving straight down to the sibling switches submenus', async ({ page }) => { await openEditMenu(page); await openFindSubmenu(page); diff --git a/apps/visual-regression/tests/menu.behavior.spec.ts b/apps/visual-regression/tests/menu.behavior.spec.ts index ac7fadb07..257124e49 100644 --- a/apps/visual-regression/tests/menu.behavior.spec.ts +++ b/apps/visual-regression/tests/menu.behavior.spec.ts @@ -23,15 +23,61 @@ test('menu teleports the positioner directly into with no wrapper element test('menu locks page scrolling by default and releases it when closed', async ({ page }) => { await gotoStory(page, 'primitives-menu--default'); - const htmlOverflow = () => page.locator('html').evaluate((el) => el.style.overflow); + // `useScrollLock` marks `` with `data-rdx-scroll-locked` (strategy-independent: the inset and + // overlay-scrollbar strategies set different overflow properties, but both set the marker). + const scrollLocked = () => page.locator('html').evaluate((el) => el.hasAttribute('data-rdx-scroll-locked')); await page.locator('[rdxMenuTrigger]').first().click(); await expect(page.locator('[rdxMenuPopup]')).toBeVisible(); - expect(await htmlOverflow()).toBe('hidden'); + expect(await scrollLocked()).toBe(true); await page.keyboard.press('Escape'); await expect(page.locator('[rdxMenuPopup]')).toHaveCount(0); - expect(await htmlOverflow()).toBe(''); + expect(await scrollLocked()).toBe(false); +}); + +test('Enter opens the default menu, focuses the first item, and ArrowDown moves to the next item', async ({ page }) => { + await gotoStory(page, 'primitives-menu--default'); + + const trigger = page.locator('[rdxMenuTrigger]').first(); + const items = page.locator('[rdxMenuItem]'); + + await trigger.focus(); + await page.keyboard.press('Enter'); + + await expect(page.locator('[rdxMenuPopup]')).toBeVisible(); + await expect(items.first()).toBeFocused(); + + await page.keyboard.press('ArrowDown'); + await expect(items.nth(1)).toBeFocused(); +}); + +test('a modal menu renders an internal backdrop that blocks the background and is the outside-press target (finding #1)', async ({ + page +}) => { + await gotoStory(page, 'primitives-menu--default'); + + // A fixed background button in the far corner that must NOT receive clicks while the modal menu is + // open — the internal backdrop has to intercept them. + await page.evaluate(() => { + const b = document.createElement('button'); + b.id = 'rdx-bg-btn'; + b.style.cssText = 'position:fixed; right:2px; top:2px; width:40px; height:40px'; + b.addEventListener('click', () => { + (window as { __bgHits?: number }).__bgHits = ((window as { __bgHits?: number }).__bgHits || 0) + 1; + }); + document.body.appendChild(b); + }); + + await page.locator('[rdxMenuTrigger]').first().click(); + await expect(page.locator('[rdxMenuPopup]')).toBeVisible(); + await expect(page.locator('[data-rdx-menu-internal-backdrop]')).toHaveCount(1); + + // A press in the far corner lands on the backdrop (over the background button): the button must not + // fire (background blocked) and the menu closes (the backdrop is the outside-press target). + await page.mouse.click(10, 10); + await expect(page.locator('[rdxMenuPopup]')).toHaveCount(0); + expect(await page.evaluate(() => (window as { __bgHits?: number }).__bgHits || 0)).toBe(0); }); test('modal menu trigger stays interactive and closes the open menu on click', async ({ page }) => { @@ -45,6 +91,22 @@ test('modal menu trigger stays interactive and closes the open menu on click', a await expect(page.locator('[rdxMenuPopup]')).toHaveCount(0); }); +test('a standalone menu closes when focus leaves to an unrelated element (finding #3)', async ({ page }) => { + await gotoStory(page, 'primitives-menu--default'); + await page.locator('[rdxMenuTrigger]').first().click(); + await expect(page.locator('[rdxMenuPopup]')).toBeVisible(); + + // A standalone menu does NOT trap focus (only a context menu does) — moving focus to an unrelated + // element closes it. + await page.evaluate(() => { + const b = document.createElement('button'); + b.id = 'm-outside'; + document.body.appendChild(b); + b.focus(); + }); + await expect(page.locator('[rdxMenuPopup]')).toHaveCount(0); +}); + test('animated menu keeps the popup mounted through the exit animation, then unmounts it', async ({ page }) => { await gotoStory(page, 'primitives-menu--animated'); diff --git a/apps/visual-regression/tests/menubar.behavior.spec.ts b/apps/visual-regression/tests/menubar.behavior.spec.ts new file mode 100644 index 000000000..7756404a2 --- /dev/null +++ b/apps/visual-regression/tests/menubar.behavior.spec.ts @@ -0,0 +1,60 @@ +import { expect, Page, test } from '@playwright/test'; + +/** + * ADR 0015/0017 Phase-4 migration of Menubar (a coordinator over `RdxMenuRoot` + `rdxMenuTrigger`, so + * each menu uses the new floating dismissal engine) onto a real browser. Guards that opening, switching + * between sibling menus, and dismissal still work — the cases the per-menu (vs legacy global-stack) + * containment had to preserve. The Default story keeps every menu popup mounted, so assertions scope to + * the *open* popup (`[data-state="open"]`) rather than counting all mounted popups. + */ +async function gotoStory(page: Page, storyId: string): Promise { + await page.goto(`/iframe.html?id=${storyId}&viewMode=story`); + await page.waitForSelector('#storybook-root', { state: 'attached' }); +} + +const trigger = '[rdxMenuTrigger]'; +const openPopup = '[rdxMenuPopup][data-state="open"]'; + +test('opens a menu and keeps it open without runtime errors', async ({ page }) => { + const errors: string[] = []; + page.on('pageerror', (e) => errors.push(String(e))); + await gotoStory(page, 'primitives-menubar--default'); + + await page.locator(trigger).first().click(); + // The freshly opened menu must not be dismissed by the async focus-outside check. + await expect(page.locator(openPopup)).toHaveCount(1); + expect(errors).toEqual([]); +}); + +test('hovering a sibling trigger switches menus — only one open at a time', async ({ page }) => { + await gotoStory(page, 'primitives-menubar--default'); + const triggers = page.locator(trigger); + + await triggers.first().click(); + await expect(page.locator(openPopup)).toHaveCount(1); + await expect(triggers.first()).toHaveAttribute('data-state', 'open'); + + await triggers.nth(1).hover(); + // Switched: still exactly one open popup, now the second menu's. + await expect(page.locator(openPopup)).toHaveCount(1); + await expect(triggers.nth(1)).toHaveAttribute('data-state', 'open'); + await expect(triggers.first()).toHaveAttribute('data-state', 'closed'); +}); + +test('Escape closes the open menu', async ({ page }) => { + await gotoStory(page, 'primitives-menubar--default'); + await page.locator(trigger).first().click(); + await expect(page.locator(openPopup)).toHaveCount(1); + + await page.keyboard.press('Escape'); + await expect(page.locator(openPopup)).toHaveCount(0); +}); + +test('an outside press closes the open menu', async ({ page }) => { + await gotoStory(page, 'primitives-menubar--default'); + await page.locator(trigger).first().click(); + await expect(page.locator(openPopup)).toHaveCount(1); + + await page.mouse.click(5, 5); + await expect(page.locator(openPopup)).toHaveCount(0); +}); diff --git a/apps/visual-regression/tests/navigation-menu.behavior.spec.ts b/apps/visual-regression/tests/navigation-menu.behavior.spec.ts index 3520e7b96..b518d58b4 100644 --- a/apps/visual-regression/tests/navigation-menu.behavior.spec.ts +++ b/apps/visual-regression/tests/navigation-menu.behavior.spec.ts @@ -18,3 +18,91 @@ test('navigation menu teleports the positioner directly into with no wrap const parentTag = await page.locator('[rdxNavigationMenuPositioner]').evaluate((el) => el.parentElement?.tagName); expect(parentTag).toBe('BODY'); }); + +/** + * ADR 0015/0017 Phase-4 migration of Navigation Menu onto the new floating dismissal engine + * (node-optional capability: one shared popup, `node === null`). + */ +test.describe('Navigation Menu — new floating engine migration', () => { + const trigger = '[rdxNavigationMenuTrigger]'; + const popup = '[rdxNavigationMenuPopup]'; + + test('Escape closes the menu and returns focus to the trigger', async ({ page }) => { + await gotoStory(page, 'primitives-navigation-menu--default'); + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + + await page.keyboard.press('Escape'); + await expect(page.locator(popup)).toHaveCount(0); + await expect(page.locator(trigger).first()).toBeFocused(); + }); + + test('an outside press closes the menu', async ({ page }) => { + await gotoStory(page, 'primitives-navigation-menu--default'); + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + + await page.mouse.click(5, 5); + await expect(page.locator(popup)).toHaveCount(0); + }); + + test('pressing a sibling trigger switches items instead of dismissing', async ({ page }) => { + await gotoStory(page, 'primitives-navigation-menu--default'); + const triggers = page.locator(trigger); + await triggers.first().click(); + await expect(page.locator(popup)).toBeVisible(); + + // A sibling trigger is a registered "inside" element, so opening it switches the shared popup + // rather than counting as an outside press / focus-out that would dismiss. + await triggers.nth(1).click(); + await expect(page.locator(popup)).toBeVisible(); + }); +}); + +test('nested navigation menu keeps parent and nested popups open when hovering the last nested trigger', async ({ + page +}) => { + await gotoStory(page, 'primitives-navigation-menu--nested'); + + await page.locator('[rdxNavigationMenuTrigger]:has-text("Company")').hover(); + await expect(page.locator('[rdxNavigationMenuPopup]:has-text("About")')).toBeVisible(); + + await page.locator('[rdxNavigationMenuTrigger]:has-text("Press")').hover(); + await expect(page.locator('[rdxNavigationMenuPopup]:has-text("News")')).toBeVisible(); + + await page.waitForTimeout(120); + await expect(page.locator('[rdxNavigationMenuPopup]:has-text("About")')).toBeVisible(); + await expect(page.locator('[rdxNavigationMenuPopup]:has-text("News")')).toBeVisible(); +}); + +test('nested navigation menu keeps parent popup open when hovering a nested popup link', async ({ page }) => { + await gotoStory(page, 'primitives-navigation-menu--nested'); + + await page.locator('[rdxNavigationMenuTrigger]:has-text("Company")').hover(); + await expect(page.locator('[rdxNavigationMenuPopup]:has-text("About")')).toBeVisible(); + + await page.locator('[rdxNavigationMenuTrigger]:has-text("About")').hover(); + await expect(page.locator('[rdxNavigationMenuPopup]:has-text("Mission")')).toBeVisible(); + + await page.locator('[rdxNavigationMenuLink]:has-text("Mission")').hover(); + await page.waitForTimeout(120); + + await expect(page.locator('[rdxNavigationMenuPopup]:has-text("About")')).toBeVisible(); + await expect(page.locator('[rdxNavigationMenuPopup]:has-text("Mission")')).toBeVisible(); +}); + +test('nested inline navigation menu keeps the parent popup open when hovering the last nested trigger', async ({ + page +}) => { + await gotoStory(page, 'primitives-navigation-menu--nested-inline'); + + await page.locator('[rdxNavigationMenuTrigger]:has-text("Browse")').hover(); + await expect(page.locator('[rdxNavigationMenuPopup]:has-text("Learn")')).toBeVisible(); + + await page.locator('[rdxNavigationMenuTrigger]:has-text("Resources")').hover(); + await expect(page.locator('[rdxNavigationMenuPopup]:has-text("Blog")')).toBeVisible(); + + await page.waitForTimeout(120); + await expect(page.locator('[rdxNavigationMenuPopup]:has-text("Resources")')).toBeVisible(); + await expect(page.locator('[rdxNavigationMenuPopup]:has-text("Blog")')).toBeVisible(); +}); diff --git a/apps/visual-regression/tests/popover.behavior.spec.ts b/apps/visual-regression/tests/popover.behavior.spec.ts index 0eb0e7d5f..3708294e1 100644 --- a/apps/visual-regression/tests/popover.behavior.spec.ts +++ b/apps/visual-regression/tests/popover.behavior.spec.ts @@ -45,3 +45,119 @@ test.describe('Popover structural portal', () => { await expect(page.locator(popup)).toHaveCount(0); }); }); + +/** + * ADR 0015/0017 Phase-4 migration of Popover onto the new floating engine (same pattern as Dialog). + * Browser-only: trap / live focus / dismissal need a real browser. + */ +test.describe('Popover — new floating engine migration', () => { + const focusInsidePopup = (page: Page) => + page.locator(popup).evaluate((el) => el.contains(el.ownerDocument.activeElement)); + + test('Escape closes the popover', async ({ page }) => { + await gotoStory(page, 'primitives-popover--default'); + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + + await page.keyboard.press('Escape'); + await expect(page.locator(popup)).toHaveCount(0); + }); + + test('an outside press closes the popover', async ({ page }) => { + await gotoStory(page, 'primitives-popover--default'); + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + + // Far top-left corner — outside the trigger and the popup. + await page.mouse.click(5, 5); + await expect(page.locator(popup)).toHaveCount(0); + }); + + test('a hover-opened popover does NOT pull focus into the popup (Base UI parity)', async ({ page }) => { + await gotoStory(page, 'primitives-popover--hover'); + await page.locator(trigger).first().hover(); + await expect(page.locator(popup)).toBeVisible(); + + // Hover-open disables the focus manager → no auto-focus into the popup. + expect(await focusInsidePopup(page)).toBe(false); + }); + + test('a modal popover traps focus once it is inside — Tab keeps it in', async ({ page }) => { + await gotoStory(page, 'primitives-popover--modal'); + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + + // Like the legacy, a positioned popover does not auto-focus into the popup on open; once focus + // is inside, the trap holds it there on Tab. (Auto-focus-on-open + Tab-from-trigger redirection + // would need the deferred portal-focus bridge / guards.) + await page.locator('[rdxPopoverClose]').focus(); + for (let i = 0; i < 4; i++) { + await page.keyboard.press('Tab'); + expect(await focusInsidePopup(page)).toBe(true); + } + }); + + test('releases marker and inert at close-start, before the exit animation finishes', async ({ page }) => { + await gotoStory(page, 'primitives-popover--modal'); + + // The modal demo popup has no exit animation by default — inject a slow one so the mid-exit + // mounted-but-closing window (the presence machine holds it while the keyframe runs) is observable. + await page.addStyleTag({ + content: `@keyframes rdx-test-out { to { opacity: 0 } } + [rdxPopoverPopup][data-closed] { animation: rdx-test-out 2000ms forwards }` + }); + + const rootState = () => + page.locator('#storybook-root').evaluate((el) => ({ + inert: el.hasAttribute('inert'), + marked: el.hasAttribute('data-rdx-floating-inert') + })); + + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + // A modal popover marks and inerts outside content while open (background isolation), like Dialog. + expect(await rootState()).toEqual({ inert: true, marked: true }); + + await page.locator('[rdxPopoverClose]').click(); + + // Mid-exit: the popup is closing (`data-closed`) but still mounted. Base UI keeps the focus manager + // mounted for trap / return-focus, but releases marker + isolation when `open` flips false. + await expect(page.locator(popup)).toHaveAttribute('data-closed', ''); + expect(await rootState()).toEqual({ inert: false, marked: false }); + + // Still released after unmount. + await expect(page.locator(popup)).toHaveCount(0); + expect(await rootState()).toEqual({ inert: false, marked: false }); + }); + + test('releases the scroll lock at close-start, before the exit animation finishes (ADR 0016 §2)', async ({ + page + }) => { + await gotoStory(page, 'primitives-popover--modal'); + + // Slow the exit so the mid-exit (mounted-but-closing) window is observable without a race. + await page.addStyleTag({ + content: `@keyframes rdx-test-out { to { opacity: 0 } } + [rdxPopoverPopup][data-closed] { animation: rdx-test-out 2000ms forwards }` + }); + + // `useScrollLock` marks `` with `data-rdx-scroll-locked` (strategy-independent). + const scrollLocked = () => page.locator('html').evaluate((el) => el.hasAttribute('data-rdx-scroll-locked')); + + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + // A modal popover locks page scroll while open. + expect(await scrollLocked()).toBe(true); + + await page.locator('[rdxPopoverClose]').click(); + + // Mid-exit: the popup is closing (`data-closed`) but still mounted. The scroll lock is already + // released because the predicate gates on `open` (not mounted). Base UI parity. + await expect(page.locator(popup)).toHaveAttribute('data-closed', ''); + expect(await scrollLocked()).toBe(false); + + // Still released after unmount. + await expect(page.locator(popup)).toHaveCount(0); + expect(await scrollLocked()).toBe(false); + }); +}); diff --git a/apps/visual-regression/tests/preview-card.behavior.spec.ts b/apps/visual-regression/tests/preview-card.behavior.spec.ts index 2932b27a8..608ec6e94 100644 --- a/apps/visual-regression/tests/preview-card.behavior.spec.ts +++ b/apps/visual-regression/tests/preview-card.behavior.spec.ts @@ -18,3 +18,36 @@ test('preview-card teleports the positioner directly into with no wrapper const parentTag = await page.locator('[rdxPreviewCardPositioner]').evaluate((el) => el.parentElement?.tagName); expect(parentTag).toBe('BODY'); }); + +/** + * ADR 0015 migration of Preview Card onto the new floating dismissal engine (dismissal-only — no focus + * manager). Browser-only: real keyboard / pointer dismissal needs a real browser. + */ +test.describe('Preview Card — new floating engine migration', () => { + const trigger = '[rdxPreviewCardTrigger]'; + const popup = '[rdxPreviewCardPopup]'; + + test('Escape closes the preview-card', async ({ page }) => { + const errors: string[] = []; + page.on('pageerror', (e) => errors.push(String(e))); + await gotoStory(page, 'primitives-preview-card--default'); + + await page.locator(trigger).first().hover(); + await expect(page.locator(popup)).toBeVisible(); + + await page.keyboard.press('Escape'); + await expect(page.locator(popup)).toHaveCount(0); + expect(errors).toEqual([]); + }); + + test('an outside press closes the preview-card', async ({ page }) => { + await gotoStory(page, 'primitives-preview-card--default'); + + await page.locator(trigger).first().hover(); + await expect(page.locator(popup)).toBeVisible(); + + // Far top-left corner — outside the trigger and the popup. + await page.mouse.click(5, 5); + await expect(page.locator(popup)).toHaveCount(0); + }); +}); diff --git a/apps/visual-regression/tests/select.behavior.spec.ts b/apps/visual-regression/tests/select.behavior.spec.ts index 82c7536b0..a12d6d03c 100644 --- a/apps/visual-regression/tests/select.behavior.spec.ts +++ b/apps/visual-regression/tests/select.behavior.spec.ts @@ -34,6 +34,55 @@ test('select teleports the positioner directly into with no wrapper eleme expect(await page.locator('[rdxSelectItem]').count()).toBeGreaterThan(0); }); +/** + * ADR 0015/0017 Phase-4 migration of Select onto the new floating dismissal engine. + */ +test.describe('Select — new floating engine migration', () => { + const trigger = '[rdxSelectTrigger]'; + const popup = '[rdxSelectPopup]'; + + test('Escape closes the select', async ({ page }) => { + await gotoStory(page, 'primitives-select--default'); + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + + await page.keyboard.press('Escape'); + await expect(page.locator(popup)).toHaveCount(0); + }); + + test('an outside press closes the select', async ({ page }) => { + await gotoStory(page, 'primitives-select--default'); + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + + await page.mouse.click(5, 5); + await expect(page.locator(popup)).toHaveCount(0); + }); + + test('a modal select renders an internal backdrop that blocks the background (finding #1)', async ({ page }) => { + await gotoStory(page, 'primitives-select--default'); + await page.evaluate(() => { + const b = document.createElement('button'); + b.id = 'sel-bg'; + b.style.cssText = 'position:fixed; left:2px; top:2px; width:40px; height:40px'; + b.addEventListener('click', () => { + (window as { __selBg?: number }).__selBg = ((window as { __selBg?: number }).__selBg || 0) + 1; + }); + document.body.appendChild(b); + }); + + await page.locator(trigger).first().click(); + await expect(page.locator(popup)).toBeVisible(); + await expect(page.locator('[data-rdx-internal-backdrop]')).toHaveCount(1); + + // A press over the background button lands on the backdrop: the button must not fire and the + // select closes (the backdrop is the outside-press target). + await page.mouse.click(10, 10); + await expect(page.locator(popup)).toHaveCount(0); + expect(await page.evaluate(() => (window as { __selBg?: number }).__selBg || 0)).toBe(0); + }); +}); + /** * Regression for ADR 0010 §6 (select restructure: positioner outer, popup inner). After the swap the * popup is a `display:flex` column and `rdxSelectList` (which carries an inline `flex: 1`) became its @@ -85,6 +134,23 @@ test('aligned-position opens without provider/runtime errors', async ({ page }) expect(errors).toEqual([]); }); +test('aligned-position locks page scroll even when modal=false (ADR 0016 AC #3)', async ({ page }) => { + await gotoStory(page, 'primitives-select--aligned-position-non-modal'); + const scrollLocked = () => page.locator('html').evaluate((el) => el.hasAttribute('data-rdx-scroll-locked')); + + expect(await scrollLocked()).toBe(false); + + await page.locator('[rdxSelectTrigger]').first().click(); + await expect(page.locator('[rdxSelectPopup]')).toBeVisible(); + // A non-modal item-aligned select still locks (Base UI `(alignItemWithTriggerActive || modal) && open`) + // — the popup overlays the trigger, so the page must not scroll behind it. + expect(await scrollLocked()).toBe(true); + + await page.keyboard.press('Escape'); + await expect(page.locator('[rdxSelectPopup]')).toHaveCount(0); + expect(await scrollLocked()).toBe(false); +}); + test('aligned-position-with-scroll: viewport overflows and scroll buttons toggle on scroll', async ({ page }) => { await gotoStory(page, 'primitives-select--aligned-position-with-scroll'); await page.locator('[rdxSelectTrigger]').first().click(); diff --git a/apps/visual-regression/tests/tooltip.behavior.spec.ts b/apps/visual-regression/tests/tooltip.behavior.spec.ts index 26d2b16d1..9ba21d1b6 100644 --- a/apps/visual-regression/tests/tooltip.behavior.spec.ts +++ b/apps/visual-regression/tests/tooltip.behavior.spec.ts @@ -19,3 +19,36 @@ test('tooltip teleports the positioner directly into with no wrapper elem const parentTag = await page.locator('[rdxTooltipPositioner]').evaluate((el) => el.parentElement?.tagName); expect(parentTag).toBe('BODY'); }); + +/** + * ADR 0015 migration of Tooltip onto the new floating dismissal engine (dismissal-only — no focus + * manager). Browser-only: real keyboard / pointer dismissal needs a real browser. + */ +test.describe('Tooltip — new floating engine migration', () => { + const trigger = '[rdxTooltipTrigger]'; + const popup = '[rdxTooltipPopup]'; + + test('Escape closes the tooltip', async ({ page }) => { + const errors: string[] = []; + page.on('pageerror', (e) => errors.push(String(e))); + await gotoStory(page, 'primitives-tooltip--default'); + + await page.locator(trigger).first().hover(); + await expect(page.locator(popup)).toBeVisible(); + + await page.keyboard.press('Escape'); + await expect(page.locator(popup)).toHaveCount(0); + expect(errors).toEqual([]); + }); + + test('an outside press closes the tooltip', async ({ page }) => { + await gotoStory(page, 'primitives-tooltip--default'); + + await page.locator(trigger).first().hover(); + await expect(page.locator(popup)).toBeVisible(); + + // Far top-left corner — outside the trigger and the popup. + await page.mouse.click(5, 5); + await expect(page.locator(popup)).toHaveCount(0); + }); +}); diff --git a/docs/adr/0015-base-ui-aligned-dismissal-engine.md b/docs/adr/0015-base-ui-aligned-dismissal-engine.md index bd68d7e3d..6781b6de4 100644 --- a/docs/adr/0015-base-ui-aligned-dismissal-engine.md +++ b/docs/adr/0015-base-ui-aligned-dismissal-engine.md @@ -1,7 +1,7 @@ # ADR 0015: Base UI-aligned dismissal engine -- Status: Proposed -- Date: 2026-06-14 +- Status: Accepted +- Date: 2026-06-14 (accepted 2026-06-16) - Decision owners: Radix NG maintainers - Related: ADR 0005 (owned floating stack), ADR 0010 (structural portal presence), ADR 0011 (WAAPI presence exit detection), ADR 0017 (floating focus manager: produces the `aria-hidden` isolation and @@ -9,6 +9,17 @@ this ADR scopes out (§9). `packages/primitives/dismissable-layer`, all floating primitive consumers listed below +> **Implementation status (2026-06-16): fully landed.** Phases -1…5 are complete — the shared floating +> tree + node/root-context/trigger-registry/event-channel pillars (§1), scoped branches (§2), Base UI +> ownership/propagation (§3), the full outside-press contract + IME/touch hardening (§4–§5), per-`Document` +> scoping (§6), and the directive API cleanup (§7). The Phase-4 atomic cutover migrated **every** floating +> primitive (Dialog, Popover, Tooltip, Preview Card, Menu/Menubar/Context Menu, Select, Combobox, +> Autocomplete, Navigation Menu) and Editable onto the engine; the legacy `RdxDismissableLayer` / +> `…Branch` / layer-stack / `disableOutsidePointerEvents` body-toggle were **deleted**. The capability is +> shipped as **`RdxDismiss`** / **`RdxDismissProps`** (Base UI `useDismiss` naming). Acceptance criteria +> 1–9 are met and verified by the Chromium behavior suites. Out-of-scope follow-ups remain owned by +> **ADR 0016** (scroll-lock behavioral parity) and **ADR 0017** (focus manager). + ## Context `RdxDismissableLayer` is the shared outside-dismiss mechanism for dialogs, menus, popovers, and other @@ -102,34 +113,53 @@ Each mounted floating element registers a **neutral** node in a **shared floatin ```ts // Shared floating infrastructure (a core/floating package) — see "Shared infrastructure" below. +// Base UI splits the lightweight tree NODE from the per-popup ROOT STORE; we mirror that exactly. interface RdxFloatingTree { /* node store; register/unregister, query children/ancestors (traversal below) */ // Typed event channel — Base UI's `FloatingTreeStore.events`, used for hover-close, virtual // focus, menu open/close coordination, and list navigation. Neutral, not dismissal-specific. events: RdxFloatingEvents; - triggers: RdxTriggerRegistry; // shared trigger registry (§2) — read by dismissal AND focus + // NOTE: neither `triggers` nor `open` live on the tree — both are per-popup on RdxFloatingRootContext. } +// Lightweight node = tree membership only (Base UI `FloatingNode`: id, parentId, context?). interface RdxFloatingNode { id: string; tree: RdxFloatingTree; // which store this node belongs to (Base UI `externalTree`) parent: RdxFloatingNode | null; // resolved logical parent (resolution rules below) - element: HTMLElement | null; + context: RdxFloatingRootContext | null; // per-popup store; `null` for a contextless intermediate. + // Associated AFTER registration and re-settable (Base UI attaches the context once the element + // resolves) via tree.setContext(node, ctx) — lifecycle `null → context → null`, owner-`Document` + // validated across ancestry AND subtree (so a contextless node can't bridge two documents). +} + +// Per-popup root store (Base UI `FloatingRootStore` / `FloatingRootContext`). CAN EXIST WITHOUT A NODE +// (`getEmptyRootContext()` analog) — this is what makes the node-optional NavMenu case work. +interface RdxFloatingRootContext { ownerDocument: Document; - // NOTE: no `open` / `active` field — open-ness is a per-capability property (below). + open: () => boolean; // ONE neutral popup open-state; traversal's `onlyOpen` reads `node.context?.open()` + triggers: RdxTriggerRegistry; // per-popup (§2) — read by both dismissal and focus of THIS popup + floatingElement: HTMLElement | null; // read-only; assigned via validated setter (owner-Document checked) + referenceElement: Element | null; // read-only; assigned via validated setter } ``` -**The node is mounted-state; "open/active" is a per-capability property — they are not the same.** Base -UI keeps `open` on a node's `context` (capability), not the node, so a popup can be **mounted but closed** -(keep-mounted / animated exit) or a node can be a contextless intermediate. Each capability exposes its -own `open`/`active`: +**Node vs root context — they are distinct, exactly as in Base UI.** A node is **mounted** iff it is +registered (so a popup can be **mounted but closed** — keep-mounted / animated exit — or be a contextless +intermediate). Open-ness, the trigger registry, and the elements all live on the **root context**, not the +node; tree traversal's `onlyOpen` filter reads **`node.context?.open()`**, **never** an OR over attached +capabilities (that conflates independent capabilities — an early foundation bug, fixed). A capability's +**own** active-ness (does _this_ capability handle events) is a separate `active()` it owns, distinct from +the popup's `open()`. The split is what keeps the **node-optional** case sound: a capability references a +**root context mandatorily** and a **node optionally**, so Navigation Menu can read `open()`/`triggers` +from a standalone context while its tree node is temporarily absent. ```ts -// 0015-owned capability attached to an RdxFloatingNode. +// 0015-owned capability. References a ROOT CONTEXT (mandatory) + a NODE (optional, #8/NavMenu). interface RdxDismissableCapability { - node: RdxFloatingNode | null; // node-OPTIONAL: may be absent in a contextless/transient state (#8, NavMenu) - open: () => boolean; // active-ness lives on the capability, not the node + context: RdxFloatingRootContext; // mandatory — open/triggers/elements live here, node-or-not + node: RdxFloatingNode | null; // node-OPTIONAL: absent in a contextless/transient state (#8, NavMenu) + active: () => boolean; // this capability's active-ness (≈ context.open() && enabled) layer: RdxDismissableLayer; branches: Set; policy: RdxDismissableLayerPolicy; @@ -203,15 +233,35 @@ _result_ yet the walk **still descends into its children** (so a keep-mounted/cl open grandchild). Mirroring Base UI's `getNodeChildren(nodes, id, onlyOpenChildren)`: ```ts -// filters the *result* by open-capability; recursion continues regardless. +// filters the *result* by node.context?.open(); recursion continues regardless. children(node, { onlyOpen?: boolean }): RdxFloatingNode[]; ancestors(node): RdxFloatingNode[]; -deepestOpen(node): RdxFloatingNode | null; // topmost-within-tree = deepest open descendant (§1) ``` +**Dismissal ownership is per-node, not a global selector (verified `useDismiss.ts:170,214`).** Base UI +does **not** pick one deepest/"topmost" node. Every open node's `useDismiss` handler runs and closes +**unless** `!escapeKeyBubbles && hasBlockingChild('__escapeKeyBubbles')` — i.e. it defers only when it +has its **own** open descendant that does not bubble. Two open **siblings** in one tree **both** respond +to Escape / outside-press. `hasBlockingChild` is a **local function inside `useDismiss`** (`useDismiss.ts:170`), +not a tree API — in Angular it lives in `RdxDismissableCapability` as: +`tree.children(node, { onlyOpen: true }).some(isBlocking)`. A global `deepestOpen` selector would +reintroduce the old single-active-layer stack under a new name and must not be added. (`getDeepestNode` +exists in Base UI's `nodes.ts:18` but is deliberately **not** called by `useDismiss` — confirmed.) + +**Unregister lifecycle — no ghost ancestry (verified against `getNodeAncestors`).** Base UI resolves +ancestry by `parentId` lookup in the **live** nodes array (`nodes.find(n => n.id === currentParentId)`, +`nodes.ts:45`), so unregistering a node **breaks the chain**: a removed middle node truncates its +descendants' ancestry (they keep the raw `parent` identity but it is no longer a traversable ancestor — +the walk **stops**, it does not skip to the grandparent). We match this: `ancestors()` / `nearestContext()` +**stop at an unregistered node**, so a parent that Angular destroys before its child cannot linger as a +ghost ancestor influencing DI-ownership, document context, or dismissal/focus traversal. Correspondingly, +`register()` / `setParent()` **reject an unregistered (or foreign) parent** — you cannot attach under a node +that has already left the tree. + **Focus-return uses `onlyOpen: false` — it must include closed-but-mounted descendants (#4, verified).** -The `onlyOpen` filter is **not** one global default. Dismissal "topmost"/children queries use -`onlyOpen: true`, but the focus manager's **focus-inside-tree** check on unmount/close walks +`tree.children()` defaults `onlyOpen` to `true` (the dismissal default), but the focus manager's +**focus-inside-tree** check +on unmount/close walks `getNodeChildren(tree, nodeId, false)` (`FloatingFocusManager.tsx:842`) — `onlyOpen=false`, so focus living inside a **mounted-but-closed** descendant still counts as "inside the floating tree" and can govern whether return-focus runs. The shared traversal must therefore expose **both** filters and the ADR 0017 focus-return @@ -251,19 +301,39 @@ type RdxFloatingParentOverride = The point is that "no override" (`inherit`) and "explicit independent root" (`root`) are **distinct** — they must not both reduce to `parent == null`. +**`{ kind: 'root' }` is NOT tree isolation — it is `parent = null` _within the current tree_.** Tree +selection is a **separate** contract: `resolveFloatingTree(externalTree?)` = `externalTree ?? nearest +injected RDX_FLOATING_TREE` (Base UI `externalTree ?? contextTree`). The parent override **never** selects +the tree. Two consequences pinned for the implementation: (a) a node made `{ kind: 'root' }` is still a node +of the **same** tree (it just has no parent there) — to put a node in a genuinely separate store, supply an +explicit `externalTree`; (b) for a detached `{ kind: 'node', parent }` from a **sibling injector**, the +nearest injected tree may be absent or a different tree than `parent.tree`, so the registrant **must** pass +`externalTree = parent.tree` (so the node joins its parent's tree and the cross-tree invariant holds) — +relying on the nearest injected tree would throw `cross-tree-parent` or mis-place the node. + This override applies to **floating nodes only**. A detached _trigger_ is a different mechanism: it has no node and no parent — it registers as an inside-element with the layer it controls through the scoped registrar of §2, so pressing or focusing it does not dismiss its popup. Do not register a trigger as a node parent; the two paths must stay distinct in the implementation. -**Tree/document invariants (dev-mode diagnostics).** Detached-composition mistakes must surface as -**early diagnostics**, not as wrong dismissal/focus ownership later. The registration API must reject (or -`rdxDevError`): - -- a `parent` that belongs to a **different `tree`**; -- a `parent` in a **different `ownerDocument`**; -- an **ancestry cycle** (the new parent chain reaching back to the node); -- registering one node in **more than one tree** at a time. +**Tree/document invariants — split by whether a violation corrupts structure.** Detached-composition +mistakes must surface as **early diagnostics**, not as wrong dismissal/focus ownership later. But the +checks are **not** uniformly dev-only: a check whose violation would **corrupt the tree's internal +structure** runs in **every** build (production included), because skipping it leaves a broken tree, not +just an undiagnosed misuse. Only the **expensive** correctness checks are gated behind `isDevMode()`. + +- **Always-on (structural integrity)** — a node passed to a mutator (`setParent`/`setContext`) must + belong to **this** tree and still be **registered**; a `parent` (`register`/`setParent`) must belong to + this tree and be registered; and `setParent` must **reject an ancestry cycle**. Violating any of these + corrupts the tree: a foreign/ghost parent, a mutation of another tree's node, or — for a cycle — a + `children`/`ancestors`/`nearestContext` traversal that **recurses/loops forever**. The cycle walk is + O(depth), cheap enough to keep on in production. +- **Dev-only (`isDevMode`)** — the **owner-`Document`** consistency check across the ancestry **and** + subtree (O(subtree)), and registering one node in more than one tree. These catch misuse without being + load-bearing for structural integrity. + +(Read-only traversal — `children`/`ancestors` — keeps its ownership check dev-only: a foreign node +there yields a wrong result, not corruption.) **Owner-`Document` relocation rule (single normative decision).** Moving a portal's nodes **within the same `Document`** is allowed (that is the normal portal case, and DI ancestry is unaffected). @@ -289,20 +359,25 @@ exit. Our `RdxPopperContentWrapper.autoUpdate` already lives for the directive's **parity, kept as-is**: no `active` input and no freeze-on-`open=false`. If a future primitive needs a frozen exit, that becomes a separate Popper capability, not a dismissal concern. -**The portal registry is DOM-inside-checks only — it does not define ancestry.** The scoped portal registry -(ADR 0017 §6a) exists so a parent can read its descendant portals' DOM roots for keep-sets / outside-press -containment. It does **not** establish logical floating ancestry: `RdxFloatingNode.parent` (DI-derived) -remains the **sole** source of dismissal ownership, independent of where nodes are appended in the DOM. +**The portal registry is for `markOthers` keep-sets only — never for dismissal.** The scoped portal registry +(ADR 0017 §6a) exists so a parent can read its descendant portals' DOM roots for **`markOthers` / aria-hidden +keep-sets** (ADR 0017). It is **not** a dismissal-inside source and **not** used for outside-press containment +(§4: "inside" is the floating tree + trigger/branch/marker, **never** portal-registry membership — Base UI +`useDismiss.ts` reads no `PortalContext`), and it does **not** establish logical floating ancestry: +`RdxFloatingNode.parent` (DI-derived) remains the **sole** source of dismissal ownership, independent of +where nodes are appended in the DOM. -`RdxFloatingNode.parent` (logical ancestry) drives all dismissal ownership. Within a logical -tree, "topmost" is the deepest active descendant, resolved from the tree (ancestry + blocking-child) — -never from DOM or construction order. +`RdxFloatingNode.parent` (logical ancestry) drives all dismissal ownership. Each open node handles +its own Escape and outside-press; a node defers **only** when it has a non-bubbling open descendant +(`hasBlockingChild` pattern, implemented in the capability — **not** a tree API and **not** a global +"deepest open" selector). **Independent roots are not coordinated by the engine — strict Base UI parity.** Base UI's `useDismiss` does not coordinate independent floating trees against each other: each open independent root handles its own Escape and outside-press. We deliberately match that and add **no** document-scoped activation -order across unrelated roots. The engine resolves "topmost" only **within** a tree; ordering between -independent roots is the concern of the owning primitive or the application, not the dismissal engine. +order across unrelated roots. Each open node within a tree handles its own event independently — there +is no global "topmost" selector; ordering between independent roots is the concern of the owning +primitive or the application, not the dismissal engine. This is an intentional change from the current `RdxDismissableLayer`, whose shared `layersRoot` makes Escape close only the last-registered layer across **all** roots. Dropping that shared order means: with @@ -313,8 +388,7 @@ primitive layer), exactly as Base UI leaves it to the consumer. The engine must answer these questions without querying all `[data-dismissable-layer]` elements: -- Is this layer the topmost active layer **within its tree**? (logical ancestry, not cross-root order) -- Does this layer have an active blocking descendant? +- Does this node have an open, non-bubbling descendant (`hasBlockingChild` pattern in the capability) that should handle the event instead? - Is an event inside this layer, one of its branches, its trigger, or an active descendant? - Should Escape or outside press propagate to an ancestor? @@ -329,10 +403,15 @@ node store: 1. **Neutral nodes** — `RdxFloatingTree` / `RdxFloatingNode` (no dismissal name, no baked-in `layer: RdxDismissableLayer`). -2. **Typed capabilities** bound to a node — dismissal (`RdxDismissableCapability`, this ADR), focus (ADR - 0017), and future hover/list-navigation each attach their own; each owns its `open`/`active`. -3. **A shared trigger registry** on the tree/root (Base UI `context.triggerElements`, §2) — read by - **both** dismissal and the focus manager, so they never keep divergent inside-element lists. +2. **Typed capabilities** that reference a node — dismissal (`RdxDismissableCapability`, this ADR), focus + (ADR 0017), and future hover/list-navigation each own their `active()`, distinct from the node's single + `open()` lifecycle. +3. **A per-popup trigger registry** (Base UI `triggerElements` on each `FloatingRootStore`, §2) — one per + **root context** (`RdxFloatingRootContext`, which can exist without a node), read by **both** that + popup's dismissal and focus, so they never keep divergent inside-element lists. It is **not** tree-wide + (that would leak one popup's trigger into an unrelated popup's inside-set) and **not** on the node + (the context outlives / precedes the node). Membership matching is **cross-realm-safe** (reference + identity, not `instanceof`), for triggers in another `Window`/iframe. 4. **Typed event channels** — Base UI's `FloatingTreeStore.events` (hover-close, virtual focus, menu open/close coordination, list navigation). Pin a neutral typed emitter on the tree now, rather than bolting one on later (which would change the fundamental tree API). @@ -433,8 +512,9 @@ cancelable while silently ignoring `preventDefault()`. Defaults preserve intended current public behavior, except where this ADR explicitly fixes observable bugs: -- Within a tree, Escape dismisses only the topmost blocking layer. Independent roots are not - coordinated by the engine (§1) — each handles its own Escape. +- Within a tree, each open node handles Escape unless it has a non-bubbling open descendant + (`hasBlockingChild` pattern, capability-owned). Independent roots are not coordinated by the engine + (§1) — each handles its own Escape. - Pointer interaction inside a child layer does not dismiss its ancestors. This holds today only for modal children (via pointer-events layering); the tree makes it hold for non-modal children too, fixing the latent bug captured by the Phase 0 known-bug target. @@ -786,16 +866,17 @@ non-parity transitional behavior. ## Implementation Plan -### Phase -1: Document-scope `core/useScrollLock` (prerequisite) - -- Convert `packages/primitives/core/src/dom/use-scroll-lock.ts` from module-level `original` / - `scrollLockCount` to `WeakMap` with a browser guard. **All** mutable state - (the saved original, the count — and, once ADR 0016 lands the behavioral port, snapshots/timers/frames/ - restore) lives on the per-`Document` state, never at module scope (ADR 0016 §1). -- Verify scroll locking is isolated per document and SSR-safe for **every** `useScrollLock` caller: - Dialog, Menu, Popover, Select, Combobox, and Autocomplete. -- This may land as its own small change before Phase 1; the dismissal engine's per-`Document` isolation - is incomplete without it. +### Phase -1: Document-scope `core/useScrollLock` (prerequisite) — ✅ LANDED + +- ✅ **Done.** `packages/primitives/core/src/dom/use-scroll-lock.ts` converted from module-level `original` / + `scrollLockCount` to `WeakMap` (`{ original, count }` per `Document`) with an + `isPlatformBrowser(PLATFORM_ID)` guard (no-op on the server). **All** mutable state lives on the + per-`Document` state, never at module scope (ADR 0016 §1); the behavioral-parity additions + (snapshots/timers/frames/restore) land on the same state in ADR 0016. +- ✅ Tested (`core/__tests__/use-scroll-lock.spec.ts`): lock/restore, per-`Document` isolation (an iframe + document lock does not touch the main document), shared per-document count composition (nested overlays), + and server no-op. Covers all `useScrollLock` callers (Dialog, Menu, Popover, Select, Combobox, + Autocomplete) since they share the one utility. - Scope: **per-`Document` correctness only.** Base UI scroll-lock behavioral parity (scroll-position, gutter, resize, pinch-zoom, owner element) is ADR 0016 (§6/§9), not this phase. @@ -804,14 +885,16 @@ non-parity transitional behavior. Split the tests by whether they describe behavior that currently passes or a known bug. Do not assert a single blanket "nested child does not dismiss the parent" — that holds today only for some variants. -**Characterization (must pass against current code, must keep passing):** +**Characterization (must pass against current code, must keep passing) — ✅ baseline LANDED:** -- topmost Escape dismissal; -- branch pointer and focus interaction; -- associated trigger interaction does not dismiss its popup; -- nested **modal** child: pointer interaction does not dismiss the parent (protected today by - pointer-events layering); -- stacked `disableOutsidePointerEvents` restoration. +- ✅ topmost Escape dismissal (`dismissable-layer-stack.spec.ts`); +- ✅ branch pointer and focus interaction (`dismissable-layer-characterization.spec.ts`); +- ✅ associated trigger interaction does not dismiss its popup (registered as a branch today, same spec); +- ✅ nested **modal** child: pointer interaction does not dismiss the parent (same spec; protected today by + pointer-events layering + DOM nesting); +- ✅ stacked `disableOutsidePointerEvents` restoration (`dismissable-layer-stack.spec.ts`). + +These now lock the pre-refactor behavior so Phase 1 can be proven behavior-preserving. **Known-bug targets — the suite must stay green, so these are NOT committed as failing tests:** @@ -835,15 +918,128 @@ commit, with the corrected behavior asserted the moment the fix lands. Nested portal, scrollbar, and real pointer-gesture cases belong in Playwright behavior tests because jsdom does not provide trustworthy layout or pointer behavior. -### Phase 1: Explicit tree and document registry - -- Add the layer-node/context and document-scoped registry. -- Replace DOM-order `isLayerExist`. -- Derive ancestry and within-tree "topmost" from the logical tree. Add **no** cross-root activation - order — independent roots stay independent (Base UI parity). -- Add the explicit parent-override registration handle for detached composition. -- Scope branches to their owner. -- Replace the global body pointer-events variable with the document registry. +### Phase 1: Dismissal capability on the shared tree + document registry + +> The neutral shared floating **foundation already exists** in `@radix-ng/primitives/core` +> (`core/src/floating`): `RdxFloatingTree` / `RdxFloatingNode` / `RdxFloatingRootContext`, traversal +> `children`/`ancestors`, `RdxTriggerRegistry`, `RdxFloatingEvents`, the DI seams +> (`RDX_FLOATING_TREE`, `RDX_FLOATING_ROOT_CONTEXT`, `resolveFloatingTree`, `injectFloatingRootContext`, +> `RdxFloatingRegistrationContext`, `RDX_FLOATING_REGISTRATION`, `provideFloatingRegistration`), and the +> structural/dev invariants. Phase 1 **consumes** it — do **not** re-implement the +> node/context/tree/ancestry/parent-override or the handle propagation contract. + +**Root-context ownership (decided — Base UI parity, `useDismiss.ts:117`).** `useDismiss` **receives** an +existing `FloatingRootContext`; it never creates one. So a **primitive root** (Dialog/Popover/Menu/…) +creates **one** `RdxFloatingRootContext` and provides it via `provideFloatingRootContext`; the dismissal +capability **and** the focus manager (ADR 0017) read that **same** context (one `open` / `triggers` / +elements). A **standalone** `rdxDismissableLayer` creates a fallback context **only when none is provided** +(`injectFloatingRootContext(fallback)`). `RDX_FLOATING_TREE` is **optional**: with no enclosing tree the +capability runs **node-optional** (`node === null`), reading its context directly. + +**Standalone fallback-context lifecycle (decided — must not be inert).** `createFloatingRootContext()` +defaults `open: () => false`, so the fallback **must** be configured or a bare `rdxDismissableLayer` would +never dismiss. The fallback is built with: **`open: () => true` for the directive's lifetime** (a standalone +layer has no separate open-state — it is active whenever mounted; if a consumer needs to toggle it, that is +an explicit `enabled`/`open` input, not the default), **`ownerDocument` = the host element's +`ownerDocument`**, and **`floatingElement` = the host element** (via `setFloatingElement`). So a standalone +layer is active-while-mounted with its own element as the inside-surface. + +**Provider migration vs the legacy stack (decided — no broken intermediate).** Replacing `layersRoot` is +**unsafe until** each primitive root provides `provideFloatingTree()` + `provideFloatingRootContext()` — +otherwise every layer resolves `node === null`, becomes an independent root, and **loses nested ownership** +(parent Dialog/Menu would no longer stop responding while a nested child is open). A **per-primitive +incremental** switchover would also create unsound mixed states — legacy-parent + migrated-child, +migrated-parent + legacy-child, or a branch registered only in the new capability while the **legacy** engine +still reads events — so "no broken intermediate" would not hold automatically. + +**Decision: ATOMIC cutover — no dual-run, no compatibility bridge.** Phases 1–3 build the new tree/capability +engine **in parallel**, unit-tested standalone, **without altering the live legacy path**: the legacy +`RdxDismissableLayer` + `layersRoot` + global `context.branches` stay authoritative and untouched, and the +new capability uses its **own** branch store / event handling that is **not yet wired** to behavior. **Phase +4 performs a single atomic cutover** — every primitive root gains its providers (`provideFloatingTree()` +inherit-or-create + `provideFloatingRootContext()`), branch registration moves to the capabilities, event +handling switches to the tree, and `layersRoot` / `isLayerExist` / the global branch array are removed — **in +one change**. Before the flip the tree drives nothing; after it, the tree drives everything. There is never a +mix of legacy and migrated consumers, so the mixed-state failure modes cannot occur. + +- Build the **dismissal capability** (`RdxDismissableCapability`, §1) that references the root context + (mandatory) + node (optional), with `active()` tied to `context.open()`. +- Build the **registration directive** for `rdxDismissableLayer`. It accepts two options: + `externalTree?: RdxFloatingTree` (explicit tree for detached sibling composition; same as Base UI's + `externalTree`) and `parentOverride?: RdxFloatingParentOverride` (defaults to `{ kind: 'inherit' }`). + + **Angular DI propagation — handle pattern (decided; do not use dynamic token replacement).** + Angular injectors are sealed at creation time: a directive cannot change what `RDX_FLOATING_TREE` + resolves to for descendants after it processes runtime inputs. Instead, the directive provides a + `RdxFloatingRegistrationContext` (a stable DI handle with a **single atomic state signal** holding a + **three-state lifecycle** — `pending | detached | registered` — not two independent `WritableSignal` + fields) via `provideFloatingRegistration()` **in its `providers` array** (at injector creation). After + resolving inputs in an `effect()`, the directive calls `selfReg.register(resolvedTree, registeredNode)` + (→ `registered`) or `selfReg.markDetached()` (→ `detached`, node-optional). Descendants inject the + handle with `{ optional: true, skipSelf: true }` and read `parentReg.status()` / `parentReg.tree()` / + `parentReg.node()` reactively. + + **Reader / writer split.** `provideFloatingRegistration()` returns **two** providers over **one** + instance: the concrete `RdxFloatingRegistrationContext` (the writer — `register` / `markDetached` / + `clear`) and a `useExisting` alias under the reader-typed `RDX_FLOATING_REGISTRATION` token. The owning + directive injects the **class** (writer) for its own handle; a descendant injects the **token** + (`RdxFloatingRegistrationReader`: only `status` / `tree` / `node`). So a descendant cannot clear or + re-point its parent's registration — the reader/writer boundary is enforced at the type level. + + **Why three states (not nullable).** A child must distinguish a parent that is **still resolving** + (`pending`) from one that **resolved with no node** (`detached`, node-optional). Both report + `node() === null`, so a two-state `null | {tree,node}` handle conflates them: a child seeing the + initial `null` could fall back to the ambient tree and **transiently register as a root in the wrong + tree** before the parent finishes resolving. With `status()`, a child **waits** on `pending` (its + effect re-runs reactively when the parent flips) and only treats `detached` as "no parent". + + **Only `inherit` waits — which is also why a destroyed parent never strands a child.** The wait is + gated on `override.kind === 'inherit'`: a `root` / `node` override does not depend on the DI parent and + registers immediately (waiting on a `pending` DI parent would wrongly stall it, or — once that parent + is destroyed and its handle is fixed at `pending` by the final `clear()` — strand it forever). And an + `inherit` node is by definition a **DI descendant** of the parent whose handle it reads, so Angular + tears it down together with (before) that parent: it can never survive to observe the parent's + post-destroy `pending`. So the only handle that ever waits is one guaranteed to die with its parent — + `clear()`'s transient `pending` is safe, and no extra terminal/destroyed state is needed. + + Concrete sequence in `providers` + `constructor`: + 1. `providers: [provideFloatingRegistration()]` — seals the stable handle at injector creation. + 2. Inject at construction time (injection context): + `selfReg = inject(RdxFloatingRegistrationContext)` (the concrete writer, own handle), + `parentReg = inject(RDX_FLOATING_REGISTRATION, { optional: true, skipSelf: true })` (reader), + `ambientTree = inject(RDX_FLOATING_TREE, { optional: true })`. + 3. In `effect((onCleanup) => { … })`: resolve `override` first, then wait **only for `inherit`** — + `if (override.kind === 'inherit' && parentReg?.status() === 'pending') return;` (reading `status()` + subscribes us, so the effect re-runs on the parent's next transition). `root` / `node` overrides are + independent of the DI ancestor and proceed immediately. Then resolve `parentNode`: `inherit` → + `parentReg?.node() ?? null` (a `detached` parent reads `null` → this node becomes a root in its + tree); `root` → `null`; `node` → `override.parent`. Resolve the tree: + `tree = (override.kind === 'node' ? override.parent.tree : undefined) ?? externalTree ?? +parentReg?.tree() ?? ambientTree`. + 4. If tree is non-null: `const node = tree.register(…)`, then `selfReg.register(tree, node)`; + cleanup on `onCleanup(() => { tree.unregister(node); selfReg.clear(); })` (→ back to `pending`). + 5. If tree is null: node-optional mode — `selfReg.markDetached()`; the capability runs without a tree + node, reading only the root context. `selfReg.status()` is `detached`, `selfReg.tree()` is `null`. + + `inject()` is **not** available inside `effect()` (no injection context there), so all DI resolution + (`inject(RDX_FLOATING_REGISTRATION, …)`, `inject(RDX_FLOATING_TREE, …)`) happens in the constructor. + The handle's `parentReg.status()` / `parentReg.node()` is the single mechanism for parent resolution — + there is no separate `resolveFloatingParent` / `RDX_FLOATING_NODE` API (those were removed from the + foundation; the handle subsumes them). + +- Build the dismissal-ownership resolution within the capability using + `tree.children(node, { onlyOpen: true }).some(isBlocking)` — the `hasBlockingChild` pattern lives in + `RdxDismissableCapability` (Base UI `useDismiss.ts:170` is a local function, **not** a tree method); + the tree stays neutral. Add **no** cross-root activation order — independent roots stay independent + (Base UI parity). **Do not remove DOM-order `isLayerExist` / the `layersRoot` stack here** — it stays + authoritative until the per-primitive Phase 4 switchover (see "Provider migration" above), so nested + ownership is never broken mid-migration. +- Give the new capability its **own** branch store (`branches: Set`), scoped to it — **leave the + legacy global `context.branches` array in place** (the legacy engine still reads it); the move is part of + the Phase 4 atomic cutover. +- Build the per-`Document` body `pointer-events` registry (`WeakMap`, replacing the + module-global variable) on the new engine, building on the Phase -1 `useScrollLock` document-scope + precedent — wired in at the Phase 4 cutover. - Preserve owner-document and SSR-safe listener behavior. ### Phase 2: Event policy, propagation, and focus ownership @@ -852,8 +1048,9 @@ jsdom does not provide trustworthy layout or pointer behavior. - Block parent dismissal while a non-bubbling child is active. - Migrate Menu's existing `closeParentOnEsc` (submenu re-emit) onto `escapeKeyBubbles`; keep `menu.spec.ts` green and the observable submenu-Escape behavior unchanged. -- Move focus-out detection and closing into each owning primitive; remove focus-out from the shared - dismissal engine while preserving primitive behavior and compatibility outputs. +- **Focus-out is NOT removed in this phase** — only Escape/outside policy + propagation land here. The + shared engine keeps driving focus-out (unchanged) until the **coordinated migration (Phase 4)**, so there + is never a window with no focus-out close. Removal is sequenced after the replacement exists (see Phase 4). ### Phase 3: Press and IME hardening @@ -867,6 +1064,18 @@ jsdom does not provide trustworthy layout or pointer behavior. ### Phase 4: Directive API cleanup and internal consumer migration +> **Cross-ADR ordering (hard dependency):** the focus-out **removal + rewiring** below requires the +> replacement to already exist — i.e. **ADR 0017 Phase 3** (close-on-focus-out) must be landed for the FFM +> primitives, and the package-internal `useFocusOutside` must exist for the three non-FFM primitives. Until +> then the shared engine keeps driving focus-out (ADR 0015 Phase 2). This is the single coordinated phase +> where focus-out moves; it is **not** removed earlier. + +- **Atomic cutover to the tree (decided above — done in ONE change, all consumers at once).** Every + primitive root gains `provideFloatingTree()` (**inherit-or-create**, so a top Menu/Dialog creates the tree + and a nested one inherits it — `MenuRoot.tsx:533` parity) + `provideFloatingRootContext()`; branch + registration moves to the capabilities; event handling switches from the legacy stack to the tree; and the + `layersRoot` stack + DOM-order `isLayerExist` + the global branch array are removed — **simultaneously**. + No mixed legacy/migrated state ever exists (no per-primitive interim). - Introduce the discriminated `dismissRequest`. - Remove raw helper directives and implementation-detail tokens from the public barrel. - Migrate Dialog, Popover, Menu, Select, Combobox, Autocomplete, Preview Card, Tooltip, Navigation @@ -967,18 +1176,27 @@ portaled child's logical parent or configurable propagation through nested float This ADR can move to Accepted when: -1. Logical ancestry and within-tree "topmost" never depend on `[data-dismissable-layer]` DOM order or - directive construction order. The engine adds no cross-root activation order — independent roots each - handle their own Escape/outside-press, matching Base UI. +1. Logical ancestry and per-node blocking-child ownership never depend on `[data-dismissable-layer]` DOM + order or directive construction order. The engine adds no cross-root activation order — independent + roots each handle their own Escape/outside-press, matching Base UI. 2. A layer node can declare an explicit logical parent, so a detached/cross-injector-subtree popup resolves to the correct owner; detached triggers resolve as scoped inside-elements (§2), not layer parents. -3. The shared floating infrastructure (§1) ships all **four** pillars from the start — neutral nodes, - typed per-capability state (with `open`/`active` on the capability, not the node), a **shared trigger - registry** (`hasElement`/`hasMatchingElement`), and **typed event channels** — read by both this ADR - and ADR 0017. Tree traversal **filters** by open-capability but **does not abort recursion** at a - closed node (a closed parent never hides an open descendant). Tree/document invariants are enforced as - dev diagnostics (§1). +3. The shared floating infrastructure (§1) ships all **four** pillars from the start — a lightweight + neutral **node** (`id`/`parent`/`context`) distinct from the per-popup **root context** + (`RdxFloatingRootContext`: `open()`, `triggers`, elements) which can exist **without** a node + (`getEmptyRootContext` analog, for node-optional NavMenu); typed capabilities that **reference** a root + context (mandatory) + node (optional) and own their `active()`; a **per-popup trigger registry** + (`hasElement`/`hasMatchingElement`, on the root context, **not** tree-wide); and **typed event + channels** (neutral, private per tree) — all read by both this ADR and ADR 0017. The tree is + **scoped-by-default** (one per coordinating root via `provideFloatingTree()`, no application-root + singleton — Base UI parity), so its events never leak across unrelated popups. Tree traversal + **filters** by `node.context?.open()` (never an OR over capabilities) but **does not abort recursion** + at a closed node (a closed parent never hides an open descendant). Tree invariants are enforced by + severity (§1): the **structural** checks — node/parent belongs to this tree and is registered, and **no + ancestry cycle** — run in **every build** (a violation corrupts the tree or hangs traversal); the + **expensive** correctness checks — owner-`Document` consistency across the ancestry **and subtree** + (through contextless intermediates) — are **dev-only** (`isDevMode`). 4. Parent layers remain open while interacting with active child layers or scoped branches. 5. Escape closes the correct layer and does not dismiss during IME composition. 6. Outside press implements the **full `RdxOutsidePressStrategy` contract** (`PressType | { mouse, touch } | @@ -1008,9 +1226,13 @@ lazy fn`, resolved per-pointer per-press), with the per-primitive `outsidePressE one. 10. Standalone `RdxDismissableLayer` performs no focus-out dismissal. Focus-out ownership splits: FFM primitives → **ADR 0017** (which owns the parity table — its acceptance gate, not 0015's); the three - non-FFM primitives (Tooltip, Preview Card, Navigation Menu) **re-wire their own focus-out via - `useFocusOutside`** so their current behavior is preserved (§3, Phase 4). Every retained `focusOutside` - output has documented cancellation semantics (§3) instead of a no-op `preventDefault()`. + non-FFM primitives (Tooltip, Preview Card, Navigation Menu) **re-wire their own focus-out to match + Base UI's own close behavior** via `useFocusOutside` **only where Base UI installs a focus-out path** + (§3, Phase 4). This is a **normative breaking change, not preservation**: the Radix-only divergences are + **deleted** (e.g. Preview Card's blanket `focusOutside` close → Base UI hover-interaction close; Navigation + Menu's shared-layer close → its own `useDismiss`/`REASONS.focusOut`; Tooltip's manual `preventDefault` + workaround disappears). Every retained `focusOutside` output has documented cancellation semantics (§3) + instead of a no-op `preventDefault()`. 11. Editable no longer depends on the removed `RdxFocusOutside` / `RdxPointerDownOutside` directives and retains its current click-outside / focus-out commit behavior. 12. `RdxDismissableLayersContextToken`, raw helper directives, and global branch mutation are no longer diff --git a/docs/adr/0016-scroll-lock-parity-and-activation-policy.md b/docs/adr/0016-scroll-lock-parity-and-activation-policy.md index cfdd0c120..97e0c5c4d 100644 --- a/docs/adr/0016-scroll-lock-parity-and-activation-policy.md +++ b/docs/adr/0016-scroll-lock-parity-and-activation-policy.md @@ -1,7 +1,7 @@ # ADR 0016: Scroll-lock parity and activation policy -- Status: Proposed -- Date: 2026-06-14 +- Status: Accepted +- Date: 2026-06-14 (accepted 2026-06-16) - Decision owners: Radix NG maintainers - Related: ADR 0015 (dismissal engine — scopes these concerns out in its §9), ADR 0017 (floating focus manager — owns aria-hidden isolation, marker, focus trap; sibling to this ADR), ADR 0005 (owned @@ -71,6 +71,26 @@ mutable algorithm state — style/scroll-position **snapshots**, scrollbar-gutte `ScrollLocker` instance**, with nothing left at module scope. (This sharpens ADR 0015 Phase -1, which only required the lock counter to be document-scoped.) +> **§1 status (2026-06-16): LANDED.** `core/src/dom/use-scroll-lock.ts` is now a faithful port of Base UI's +> `useScrollLock` — the two strategies (`preventScrollOverlayScrollbars` for iOS / overlay-scrollbar +> documents, `preventScrollInsetScrollbars` for inset scrollbars), html-vs-body scroller selection +> (`isOverflowElement`), scroll-position save/restore, `scrollbar-gutter: stable` feature-detect with the +> `body { position: relative; width/height: calc(...) }` compensation fallback, WebKit pinch-zoom bail-out, +> and resize re-lock. **Divergence from Base UI (intentional, per the paragraph above):** every snapshot +> (`originalHtmlStyles` / `originalBodyStyles` / `originalHtmlScrollBehavior`) is **closure-local** and the +> ref count + restore callback live on a per-`Document` `ScrollLocker` (Base UI keeps the snapshots at +> module scope) — iframe-safe. Two small **deliberate omissions**: the `setTimeout(0)` lock/unlock +> coalescing (a React-render micro-optimization; our signal effect locks/unlocks once per `active()` edge), +> and the body `overflow` short-hand is written as `overflowX`/`overflowY` long-hands so the snapshot +> restores symmetrically. A strategy-independent `data-rdx-scroll-locked` marker on `` exposes the +> lock (the inset and overlay strategies set different overflow properties). Verified: unit +> (`use-scroll-lock.spec.ts` — marker, overflow applied, exact restore round-trip, ref-count, per-document +> isolation, SSR no-op, respect-author-overflow) + browser (the marker-based lock/release tests across +> Dialog / Popover / Menu, 92 behavior tests green). **Not done:** non-DI owner-element / `referenceElement` +> support (the current helper resolves the owner document from Angular DI); the **scroll-position-preservation** +> behavior is the verbatim Base UI port (which has its own tests) but is **not** browser-asserted here — the +> Storybook iframe positions `#storybook-root` out of body flow, so it is not a usable scroll harness. + ### 2. Scroll-lock **activation policy** — not just the utility Base UI gates **whether** to lock per primitive/mode/open-reason/interaction, via @@ -110,6 +130,21 @@ the computed predicate below. Verified against `mui/base-ui` master (References) | Combobox | `useScrollLock(modal)` | Add `&& open` + touch | | Autocomplete | `useScrollLock(root.modal)` | Same as Combobox | +> **§2 status (2026-06-16): the `&& open` gating is LANDED for all six popups.** Dialog (`open && modal === true`) +> and Menu/Menubar/Context Menu (`open && (menubarModal ‖ popupModal)`, hover-excluded) were already gated; +> Popover (`open && modal === true && !hover`), Combobox / Autocomplete (`open && modal`), and Select +> (`(alignItemWithTriggerActive || modal) && open`) were migrated off the plain `useScrollLock(modal)`. Each +> predicate keys on `open()` so the lock releases at **close-start** even when an exit animation keeps the +> popup mounted — verified by the `popover.behavior` "releases the scroll lock at close-start" Playwright +> test (mirrors the Dialog one). **Select `alignItemWithTrigger` — RESOLVED (AC #3, 2026-06-16):** we keep +> the two structural positioner directives (`[rdxSelectPositioner]` popper vs `[rdxSelectItemAlignedPosition]` +> item-aligned) but made the item-aligned one Base-UI-faithful in **behavior** — it exposes +> `alignItemWithTriggerActive = open && !openedByTouch` (the positioner falls back to a plain anchored +> dropdown on touch, Base UI parity), and the popup locks scroll whenever it is active **even if +> `modal === false`** (the item-aligned popup overlays the trigger, so the page must not scroll behind it). +> Verified by `select.behavior` "aligned-position locks page scroll even when modal=false". The **touch** +> near-fullscreen opt-out is §3 (status below). + ### 3. Anchored-popup touch / near-fullscreen policy `useAnchoredPopupScrollLock`'s second argument is `openMethod === 'touch'`: a **touch-opened anchored @@ -120,6 +155,22 @@ the viewport. This is a **single anchored-popup policy** in `useAnchoredPopupScr per-primitive threshold — Select/Combobox/Menu all inherit it. (Confirm the exact 20px constant against the Base UI source during Phase 1.) +> **§3 status (2026-06-16): LANDED and wired into every anchored popup.** `core`'s +> **`useAnchoredScrollLock(enabled, { touchOpen, element })`** is the reusable Base UI +> `useAnchoredPopupScrollLock` (the `VIEWPORT_WIDTH_TOLERANCE_PX = 20` constant confirmed against source): +> a non-touch open locks while `enabled()`; a **touch** open locks only when `popupWidth >= viewportWidth - +20px`. The gate is unit-tested (`use-scroll-lock.spec.ts` — stubbing `offsetWidth` / `clientWidth`, since +> jsdom has no layout). **Wired into Combobox, Autocomplete, Select, Menu / Menubar / Context Menu, and +> Popover** — every popup measures its own element. Each primitive now plumbs an `openedByTouch` signal +> through its open path and resets it on close: Combobox / Autocomplete via the shared engine; **Select** +> (trigger `handlePointerOpen` records the `pointerType`); **Menu** family (`RdxMenuRoot.show()` derives it +> from the open event's `pointerType`, so hover / mouse / keyboard all read non-touch; **Context Menu** +> threads the touch long-press event through `openAt → show`); **Popover** (trigger captures the +> `pointerdown` `pointerType`, with a `detail === 0` guard so a keyboard-activated click reads non-touch). +> A non-touch open is byte-for-byte the old `useScrollLock` behavior (verified: full unit suite + the +> mouse-driven browser suites across all consumers stay green). The **touch** path itself is unit-only +> (our Playwright suite has no touch-open harness). + ## Out of scope (owned elsewhere) - `aria-hidden` isolation, the `data-*` marker, and focus trap — **ADR 0017** (`RdxFloatingFocusManager`). @@ -178,8 +229,8 @@ the Base UI source during Phase 1.) This ADR can move to Accepted when: -1. `core/useScrollLock` ports the **full** Base UI behavioral set (§1 — html/body, scroll-position, - gutter, resize, pinch-zoom, owner element; nothing deferred) across ``- and ``-scroller +1. `core/useScrollLock` ports the confirmed Base UI behavioral set (§1 — html/body, scroll-position, + gutter, resize, pinch-zoom; owner-element/`referenceElement` support is explicitly deferred) across ``- and ``-scroller pages, with scroll position preserved. **All** mutable algorithm state (style/scroll snapshots, gutter values, timers/frames, restore callback) is owned by the per-`Document` `ScrollLocker` — nothing at module scope — verified with a two-document/iframe test that one document's lock does not corrupt the @@ -193,6 +244,16 @@ This ADR can move to Accepted when: (`disableOutsidePointerEvents` removal is **not** a gate here — it depends on ADR 0017 only, see Context.) +> **Acceptance status (2026-06-16): accepted with the documented owner-element gap.** (1) +> `core/use-scroll-lock.ts` ports the confirmed behavior with per-`Document` `ScrollLocker` state (see §1 +> status). (2) All six call sites use +> the `open`-gated activation policy; the touch near-fullscreen opt-out is `useAnchoredScrollLock` (§2/§3 +> statuses). (3) Select `alignItemWithTrigger` resolved — item-aligned mode kept as a structural directive +> but made Base-UI-faithful (`alignItemWithTriggerActive = open && !openedByTouch`, touch falls back to a +> plain dropdown, and it locks even when `modal === false`; see the §2 status). (4) `useScrollLock` is an +> `isPlatformBrowser`-guarded no-op on the server. Remaining future polish (non-gating): `referenceElement` / +> owner-element support, and a touch / scroll-position browser harness (our Playwright suite has neither). + ## Base UI References - Scroll-lock predicates per primitive — **verified against `mui/base-ui` master**: diff --git a/docs/adr/0017-floating-focus-manager.md b/docs/adr/0017-floating-focus-manager.md index efcc86049..c471cc43a 100644 --- a/docs/adr/0017-floating-focus-manager.md +++ b/docs/adr/0017-floating-focus-manager.md @@ -11,6 +11,19 @@ > focus-policy set. Architectural decisions are settled below; Phase 0 fills two parity-characterization > tables before implementation. +> **Implementation status (2026-06-16): core landed; a few parity items open.** `RdxFloatingFocusManager` +> (entry `@radix-ng/primitives/floating-focus-manager`) composes the reworked `RdxFocusScope` and owns the +> independent policies — `enabled`/`modal`→trap, the `markOthers` aria-hidden + `inert` passes (§3), +> close-on-focus-out (§3), `initialFocus` (§2), and now **`returnFocus` (§2) — DONE**: the manager owns the +> return _target_ (resolving `true`/`false`/element/callback against the close interaction) via the focus +> scope's `returnFocus` config seam, while the scope owns the _timing_ (its queued post-unmount frame). +> Used by Dialog / Popover / Menu. **Still open (none blocking day-to-day use):** the **`aria-modal` +> attribute** AT-review (we set it only for `modal === true`; Base UI emits none and relies on `inert` — see +> the Dialog parity notes); the **Select `aria-activedescendant` focus-out divergence** (recorded breaking +> change, needs real-AT confirmation); the `markOthers` **`inert` internal variant** (deferred — no Base UI +> consumer passes it); and the **Phase-5 WebKit / screen-reader matrices** (Tier-B confirmations before +> Acceptance). The portal-focus bridge / focus-guards toolkit exist; full portal tab-order is a later phase. + ## Context Base UI co-locates a cluster of behaviors in **one** component, `FloatingFocusManager`, under **one @@ -335,8 +348,9 @@ the typeable-combobox ancestor, and focus-out ownership. So `RdxFloatingFocusMan infrastructure"), attaching its own focus capability to a node — **not** a second focus-only tree, and **not** the dismissal capability. Specifically it reads, from the shared infra: -- the **traversal** API (`ancestors`, `children({ onlyOpen })`, `deepestOpen`) — same open-filtering / - recurse-through-closed semantics as dismissal (ADR 0015 §1); +- the **traversal** API (`ancestors`, `children({ onlyOpen })` — the focus-return path uses + `onlyOpen: false`) — same open-filtering / recurse-through-closed semantics as dismissal (ADR 0015 §1); + it does **not** use the dismissal-only `hasBlockingChild`; - the **shared trigger registry** (`triggerElements`, ADR 0015 §2) for its inside-element checks — so focus and dismissal never compute divergent inside-sets; - the **typed event channels** (`tree.events`) for cross-mechanism coordination (hover, list-navigation). @@ -407,11 +421,32 @@ enough for strict parity: | **menubar child** | yes iff `parent.context.modal` (menubar's flag) | | **context menu** | yes if `modal` | -| Primitive | what intercepts a background press | internal backdrop when none provided? | trigger/input cutout | lifecycle (mounted vs open) | interactive predicate (`inert` when `!open`) | state during animated exit | behavior with **no** backdrop | ⇒ drop the body toggle? | -| ------------------------------------------------------------------------------- | ---------------------------------- | ------------------------------------- | -------------------- | --------------------------- | -------------------------------------------- | -------------------------- | ----------------------------- | ----------------------- | -| Dialog / Popover (click) / Select / Combobox / Autocomplete | | | | | | | | | -| Menu (submenu / root / hover-root / menubar-child / context) — **one row each** | | | | | | | | | -| _(Phase 0 fills each row from Base UI source + Chromium)_ | | | | | | | | | +**Filled (source-derived against `mui/base-ui` master; paths relative to `packages/react/src/`).** The +single decisive finding: **Base UI uses no `body { pointer-events }` toggle anywhere.** Modal popups block +the background **only** via a full-viewport `InternalBackdrop` (clipPath cutout around the anchor); non-modal +/ hover popups **never block** the background — outside-press only _dismisses_ (floating-ui `useDismiss`), +it does not intercept. So the Radix body toggle can be dropped for **every** row, conditional on porting +`RdxInternalBackdrop`-with-cutout for the modal cases. + +| Primitive | what intercepts a background press | internal backdrop when none provided? | trigger/input cutout | lifecycle (mounted vs open) | interactive predicate (`inert` when `!open`) | state during animated exit | behavior with **no** backdrop | ⇒ drop the body toggle? | +| ----------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------- | ------------------------------------ | -------------------------------------------- | -------------------------- | -------------------------------------------------------------- | --------------------------------------------------------- | +| Dialog (modal) | `InternalBackdrop` `position:fixed; inset:0`, rendered in the **portal** when `mounted && modal===true` (`dialog/portal/DialogPortal.tsx:35`) | yes — always for modal, independent of user `Backdrop` | **none** — full-viewport, no `cutout` passed (`DialogPortal.tsx:36`) | rendered while `mounted` (thru exit) | yes — `inert={inertValue(!open)}` (`:36`) | mounted but `inert` | non-modal Dialog → no backdrop; outside-press via `useDismiss` | **yes** — port `InternalBackdrop` | +| Popover (click / modal) | `InternalBackdrop` from the **positioner** when `mounted && modal===true && reason!==triggerHover` (`popover/positioner/PopoverPositioner.tsx:157`) | yes — independent of user `Backdrop` | **trigger** — `cutout={triggerElement}` (`:161`) ⟶ clipPath hole (`utils/InternalBackdrop.tsx:12`) | while `mounted` | yes (`:160`) | mounted but `inert` | non-modal → none; `useDismiss` outside-press | **yes** (with trigger cutout) | +| Popover (hover-open) | **nothing** — backdrop excluded (`reason!==triggerHover`, `:157`); user `Backdrop` forced `pointer-events:none` (`popover/backdrop/PopoverBackdrop.tsx:49`); dismissal via `useDismiss` | no | n/a | n/a | n/a | n/a | nothing blocks the background by design | **yes / N/A** — hover never blocked | +| Select | `InternalBackdrop` from the positioner when `mounted && modal` (`select/positioner/SelectPositioner.tsx:245`); `modal` default `true` (`select/root/SelectRoot.tsx:70`) | yes — independent of user `Backdrop` | **trigger** — `cutout={triggerElement}` (`:245`) | while `mounted` | yes (`:245`) | mounted but `inert` | non-modal → none | **yes** (with trigger cutout) | +| Combobox / Autocomplete | `InternalBackdrop` **only if `modal`** — default **`false`** (`combobox/positioner/ComboboxPositioner.tsx:129`; default `combobox/root/AriaCombobox.tsx:115`); else `useDismiss` (`AriaCombobox.tsx:1030`) | only when `modal` set `true` | **input/group** — `cutout={inputGroupElement ?? inputElement ?? triggerElement}` (`ComboboxPositioner.tsx:132`) | while `mounted` | yes (`:131`) | mounted but `inert` | default non-modal: no backdrop, background interactive | **conditional** — yes when modal; default needs no toggle | +| Menu — root | `InternalBackdrop` when `parent.type===undefined && modal && reason!==triggerHover` (`menu/positioner/MenuPositioner.tsx:283`); `modal` default `true` (`menu/root/MenuRoot.tsx:567`) | yes | **trigger** — `backdropCutout=triggerElement` (`:294,307`) | while `mounted` | yes (`:306`) | mounted but `inert` | non-modal root → none | **yes** (with trigger cutout) | +| Menu — submenu | **nothing** — backdrop never rendered (`parent.type==='menu'`, `:285`); outside-press via the tree / `useDismiss` | no | n/a | n/a | n/a | n/a | submenu never blocks the background | **yes** (no backdrop needed) | +| Menubar — child | `InternalBackdrop` when `parent.type==='menubar' && parent.context.modal` (`MenuPositioner.tsx:286`) | yes (when menubar modal) | **menubar content** — `backdropCutout=parent.context.contentElement` (`:292`) so all bar triggers stay hoverable | while `mounted` | yes (`:306`) | mounted but `inert` | non-modal menubar → none | **yes** (with content-element cutout) | +| Context Menu | `InternalBackdrop` when `modal`, ref wired to `parent.context.internalBackdropRef` (`MenuPositioner.tsx:301`) | yes | **none** — `backdropCutout` stays `null` for context-menu (`:290`); anchored at the cursor | while `mounted` | yes (`:306`) | mounted but `inert` | n/a (modal by default) | **yes** (full-viewport, no cutout) | + +> **Source-derived; browser-verify pending (jsdom cannot resolve clipPath / `getBoundingClientRect` / +> `inert` hit-testing):** that the clipPath cutout actually passes the pointer through to the +> trigger/input while blocking the rest (`InternalBackdrop.tsx:12`); that the menubar content-element +> cutout keeps every bar trigger hoverable for menu-switching (`:292`); that the input-group cutout keeps a +> modal Combobox typeable; that `inert` truly disables the backdrop during the animated exit; and that the +> cutout (computed once from `getBoundingClientRect`) does not go stale as the anchor moves/scrolls. These +> are the `RdxInternalBackdrop` acceptance checks for the Phase 5 Chromium matrix. **The backdrop primitive is infra, but the _decision to render it_ is per-primitive policy (#7, verified).** `RdxInternalBackdrop` is shared infrastructure (above), but **who renders it, where, and with what cutout @@ -660,88 +695,275 @@ while a nested popup is open leaves dismissal **and** focus ownership unchanged ### Phase 0: Parity characterization (mandatory — gates later phases) -Fill the source-derived + Chromium-verified tables/audit below. **No implementation starts until they are -complete.** - -0. **Low-level primitive parity audit (§6).** Audit `RdxFocusScope`, `RdxPortal` / `RdxPortalPresence`, - and `RdxFocusGuards` against Base UI's `FloatingFocusManager` + `FloatingPortal` + `FocusGuard`. Record - per primitive: owner-`Document` vs global `document`, module-global state, `contains()` vs - `composedPath`/Shadow DOM, `setTimeout` vs queued focus, portal guard/tabbability/`aria-owns`, and the - needed rework (focus-scope rework, the portal-focus bridge, owner-document guards). **Gate:** the - coordination contract and rework scope are decided before Phase 1. -1. **Pointer-interaction parity table** (§5) — per primitive: what intercepts a background press, internal - vs user backdrop, trigger/input cutout, backdrop **lifecycle (mounted vs open) / inert-when-`!open` / - animated-exit**, no-backdrop behavior. **It may conclude — and is expected to — that a shared - `RdxInternalBackdrop` primitive is required, with an assigned owner.** **Gate:** - `disableOutsidePointerEvents` is removed only for rows that prove parity (incl. a working - `RdxInternalBackdrop` where needed; §5 acceptance gate). -2. **Focus-out parity table** — per primitive **and modal/non-modal**, the **resulting observable focus - transition**, not the `closeOnFocusOut` input. It must also cover the `restoreFocus` edge cases Base UI - handles separately (critical for Presence / animated unmount and dynamic Menu/Combobox items): - - | Primitive / mode | focus → trigger/reference | focus → child popup | focus → outside | pointer-induced focus move | focused element **removed** | popup **kept-mounted while closed** | close during **queued initial-focus** frame | closes? | - | ----------------------------------------------------- | ------------------------- | ------------------- | --------------- | -------------------------- | --------------------------- | ----------------------------------- | ------------------------------------------- | ------- | - | _(Phase 0 fills each from Base UI source + Chromium)_ | | | | | | | | | - - **Gate:** each primitive's focus policy must match its row; the known Base-UI-Select-closes-on-focus-out - vs Radix-prevents divergence is recorded with its resolution. - -3. **`aria-owns` / content-roots audit (§6a, #4) — multi-IDREF is an Angular _adaptation_, not literal - parity.** Base UI emits a **single** `` pointing at the **one wrapper** - that owns the whole portal subtree (`FloatingPortal.tsx:267`). We have **no wrapper**, so listing - several `contentRootElements` IDREFs is a _reasonable adaptation_ — but **not proven equivalent**. - Phase 0 must validate in a real browser/AT, not assume: (a) do multiple IDREFs reproduce the intended - reading/tab order; (b) are descendants that live under a **non-content** root (e.g. inside the - positioner) lost; (c) is an **invisible semantic portal anchor** (one element wrapping the content roots, - mirroring Base UI's single wrapper) better than enumerating roots. Also record **who mints/owns the - stable IDs** (IDREF targets need SSR-stable `injectId` ids) and the `aria-owns` behavior on a - **container move**. A backdrop is a DOM root but is **never** `aria-owns`'d. **Gate:** - `contentRootElements` is defined separately from `ownRootElements`, and the IDREF-vs-anchor decision is - made from browser/AT evidence, before Phase 2; the backdrop is proven absent from any `aria-owns` set. -4. **Positioner lifecycle during animated exit (§6a, #6 — follow-up to ADR 0012).** Confirm Base UI's - positioning lifecycle on close (`useAnchorPositioning` runs `autoUpdate` while `mounted`, gated - `open: mounted` — `useAnchorPositioning.ts:441,507`) and decide our parity: **keep `autoUpdate` running - until unmount** (current `RdxPopperContentWrapper` behavior = parity, no `active` input) vs freeze on - `open=false`. If kept, characterize whether a late `flip`/`shift` can visibly jump the popup mid-exit and - whether any primitive needs to **pin the placement/transform for the exit** (a Popper capability, **not** - a dismissal/focus concern). **Gate:** the positioner's exit-time behavior is decided and, if "keep - running", a test asserts the exit animation is not broken by a placement change. -5. **Portal-ancestry vs custom-container audit (§6a, #1).** Characterize, per portaled primitive, the - resolved `portalParent`: implicit nesting (falls back to the enclosing portal context) vs an explicit - custom `container`. Verify against Base UI's `container ?? parentPortalNode ?? body` that a - custom-container child is **not** physically inside the parent portal subtree, and confirm our - `resolveRegisteredContainerParent(container)` returns the container's registration (or `null`), so the - parent's keep-sets exclude it. **Gate:** the resolved-parent algorithm is implemented and a child with - an explicit body-level container is **not** a keep-set descendant of its logical parent. -6. **Dismissal-inside vs portal-inside audit (§6a, #2).** Confirm `useDismiss` computes outside-press - "inside" from floating element + reference + **floating-tree children** + **trigger registry** + - **markers** (`useDismiss.ts:173,345,388,393`), **not** `PortalContext` descendants. **Gate:** the - dismissal engine (ADR 0015) treats portal-registry membership as **non-authoritative** for outside-press - — a contextless portal kept for `markOthers` is dismissal-inside **only** via floating descendant or - trigger/branch/inside-element registration. -7. **Focus-host resolution audit (§3, #1).** Pin the focusable marker (`FOCUSABLE_ATTRIBUTE` analog) and - the `getFloatingFocusElement(floating)` algorithm per primitive: where `floatingElement` === - `floatingFocusElement` (popup carries handlers) and where they **diverge** (positioner-is-floating, - Select item-aligned, wrapper compositions). **Gate:** the manager resolves the focus host explicitly - (trap / `initialFocus` / `returnFocus` / `tabIndex` operate on `floatingFocusElement`; tree/dismissal on - `floatingElement`); a divergent case (focus host is a child of the floating element) is covered by a test. -8. **Focus-return traversal filter (#4).** Confirm the focus-inside-tree check on unmount/close walks - descendants with `onlyOpen=false` (`FloatingFocusManager.tsx:842`), so focus inside a closed-but-mounted - descendant still counts as inside the tree. **Gate:** the return-focus path calls the shared traversal - with `onlyOpen: false` (ADR 0015 §1), never the dismissal default `onlyOpen: true`; tested with a - keep-mounted closed child holding focus. -9. **`insideReactTree`-analog capture audit (#5).** Base UI keeps an `insideReactTree` capture marker - (`useDismiss.ts:167,234,367`) separate from DOM/floating tree, guarding document-capture timing and - logical-tree interactions. Our Angular bubbling after DOM relocation does **not** follow the declaration - tree. **Gate:** prove (with tests) that the shared floating tree + owner-`Document` host listeners - **replace** this mechanism — pointer inside a portaled child while a document-capture listener is armed, - child handler `preventDefault`s, parent must **not** dismiss, including when dispatch moves/removes the - target. If they do not fully cover it, define the explicit capture-marker analog before Phase 1. +Fill the tables/audit below. **Two gate tiers** (they are sequenced differently, so do not conflate them): + +- **Tier A — source-derived decisions.** Every item's `Resolution` is decided from `~/git/base-ui` source + + an audit of our code. **These gate Phase 1**: no implementation starts until Tier A is complete (it now + is). Phase 1 proceeds on the Tier-A decision and its documented **fallback**. +- **Tier B — browser/AT verification** (the `‡` / "browser-pending" items: #3 `aria-owns`, #9 capture-race, + #10 WebKit blur). A real browser/AT/Safari is not available at characterization time, so these are + **pre-Acceptance gates verified in the Phase 5 matrix**, **not** Phase 1/2 blockers. Implementation builds + the Tier-A choice; the documented fallback is applied **only if** Phase 5 verification fails. The Tier-B + gate sentences below therefore read "**before Acceptance**" (decision + contingency recorded now; browser/AT + _confirmation_ in the Phase 5 matrix gates Acceptance) — they are **not** Phase 1/2 blockers. + +**Status (all 12 items resolved — each has a `Resolution` below, source-derived against `~/git/base-ui` + +an audit of our code).** Classification: + +- **Source-resolved / satisfied by the foundation** — #1, #2 (filled tables), #4 (positioner exit = parity, + no change), #6 (dismissal-inside = tree/registry, not portal), #8 (`children({onlyOpen:false})` exists), + #12 (`createFloatingRootContext` + node-optional capability exist). +- **Decided, needs net-new infra (Phase 1/4)** — #0 (rework `RdxFocusScope`; build the portal-focus bridge + / `RdxFocusGuards`), #5 (`resolveRegisteredContainerParent` + portal registry), #7 (pin the focus-host + marker), #11 (migrate the two Combobox layouts). +- **Source-derived, decision/behavior browser-or-AT-pending (pre-Acceptance gate, verified in Phase 5)** — + #3 (single `aria-owns` anchor vs multi-IDREF — AT), #9 (capture-timing race — Playwright; fallback = + explicit capture marker), #10 (WebKit blur-before-unmount — Safari matrix). + +Audit facts behind these: `RdxFocusScope` uses global `document` / module-global stack / `contains()` / +`setTimeout`; `RdxFocusGuards`, the portal-focus bridge, `resolveRegisteredContainerParent`, +`RdxInternalBackdrop`, `markOthers`, and the `contentRootElements`/`ownRootElements`/`descendantPortalRoots` +roles are **ADR-only (not yet implemented)**; the foundation already ships the floating tree, per-root-context +trigger registry, `children({onlyOpen})`, and `createFloatingRootContext`. + +0. **Low-level primitive parity audit (§6).** Audit `RdxFocusScope`, `RdxPortal` / `RdxPortalPresence`, + and `RdxFocusGuards` against Base UI's `FloatingFocusManager` + `FloatingPortal` + `FocusGuard`. Record + per primitive: owner-`Document` vs global `document`, module-global state, `contains()` vs + `composedPath`/Shadow DOM, `setTimeout` vs queued focus, portal guard/tabbability/`aria-owns`, and the + needed rework (focus-scope rework, the portal-focus bridge, owner-document guards). **Gate:** the + coordination contract and rework scope are decided before Phase 1. + + **Resolution (source-derived; audited our code + Base UI).** Base UI's `FloatingPortal` **is an active + focus participant** — it renders visually-hidden `tabindex=0` `FocusGuard` spans (marked + `data-base-ui-focus-guard`, `utils/FocusGuard.tsx:33`) **only when non-modal + open** + (`FloatingPortal.tsx:189`), toggles inside-tabbability via capture-phase `focusin`/`focusout` + (`:195–223`), and emits the single `aria-owns` span (`:266`). `FloatingFocusManager` uses the + **owner document** (`ownerDocument(floating)`, `FFM:338…893`), **queued** focus (`queueMicrotask` + + `enqueueFocus`→`requestAnimationFrame`), shadow-aware `getTarget`/`contains`, and a module-global + `previouslyFocusedElements` WeakRef list. **Our audit:** `RdxFocusScope` uses **global `document`** + (`focus-scope.ts:179`), a **module-global stack** (`stack.ts:14`), plain `container.contains()` (no + `composedPath`/shadow), and `setTimeout` return-focus (`:233`); **`RdxFocusGuards` does not exist**; + `RdxPortal`/`RdxPortalPresence` are **pure DOM movers** (no guards, no `aria-owns`, no focus + participation). **Decisions / rework scope:** (a) **rework `RdxFocusScope`** to owner-`Document`, + shadow/`composedPath`-aware containment, and queued (rAF/`afterRenderEffect`) focus — the module-global + stack is acceptable but its return-focus must be owner-document-scoped; (b) **build a new portal-focus + bridge** (`RdxFocusGuards` analog: leading/trailing guard spans + capture-phase tabbability toggle + + the `aria-owns` anchor) tied to `RdxPortal`/`RdxPortalPresence` — it does **not** exist today; (c) the + manager **composes** these three (trap + portal-focus bridge + owner-document) rather than extending + `RdxFocusScope`. Tab-order/focus behavior is **browser-verify pending** (Phase 5). + +1. **Pointer-interaction parity table** (§5) — per primitive: what intercepts a background press, internal + vs user backdrop, trigger/input cutout, backdrop **lifecycle (mounted vs open) / inert-when-`!open` / + animated-exit**, no-backdrop behavior. **It may conclude — and is expected to — that a shared + `RdxInternalBackdrop` primitive is required, with an assigned owner.** **Gate:** + `disableOutsidePointerEvents` is removed only for rows that prove parity (incl. a working + `RdxInternalBackdrop` where needed; §5 acceptance gate). +2. **Focus-out parity table** — per primitive **and modal/non-modal**, the **resulting observable focus + transition**, not the `closeOnFocusOut` input. It must also cover the `restoreFocus` edge cases Base UI + handles separately (critical for Presence / animated unmount and dynamic Menu/Combobox items): + + **Filled (source-derived against `mui/base-ui` master).** Legend: **FFM** = + `floating-ui-react/components/FloatingFocusManager.tsx`; the close decision is the `!modal` branch at + `FFM:531–547` (requires a set `relatedTarget`, a move to an _unrelated_ node, and `!isPointerDownRef`), + focus-out is **not** a `useDismiss` concern. ‡ = restore/timing cell that is source-correct but + **browser-verify pending** (needs real layout/focus, see below). + + **Two distinct tree walks — do not conflate (drives the Phase 3 traversal choice).** Focus-out + **containment** ("did focus move to an _unrelated_ node?", `movedToUnrelatedNode`) walks + `getNodeChildren` with the **default `onlyOpen=true`** + `getNodeAncestors` (`FFM:454–478`), so focus + moving into an **open** child popup is "related" and the parent stays open. The separate `onlyOpen=false` + walk (`FFM:842`) is **only** the focus-return / unmount path (it must also count a closed-but-mounted + descendant as inside, ADR 0015 §1). So the "focus → child popup" column cites **`FFM:466`** (containment), + **not** `FFM:842`. (`triggerElements` here is `store.context.triggerElements`, `FFM:439` — per-root, not + the shared tree, confirming the per-root-context trigger registry.) + + | Primitive / mode | focus → trigger/reference | focus → child popup | focus → outside | pointer-induced focus move | focused element **removed** | popup **kept-mounted while closed** | close during **queued initial-focus** frame | closes? | + | ---------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------- | -------------------------------------------------------------------------------------- | -------------------------- | ------------------------------------------------------------ | ------------------------------------------------------------------------------------------------- | ------------------------------------------- | -------------------------------------------------------- | + | Dialog (modal) | trapped — guards block it (`modal=true`, `FFM:935`) | stays — containment walk `onlyOpen=true` (`FFM:466`) | suppressed (`!modal` false, `FFM:532`) | suppressed (`FFM:535`) | `restoreFocus="popup"` re-focuses popup ‡ (`FFM:499`) | manager **enabled** (`disabled={!mounted}`); trap + return-focus persist; close moot | `restoreFocusFrame.request` ‡ (`FFM:504`) | **No** | + | Dialog (non-modal / `disablePointerDismissal`) | `closeOnFocusOut=!disablePointerDismissal`; if disabled, listener never attaches (`FFM:411`) | stays (`FFM:466`) | plain non-modal → closes (`FFM:531`); `disablePointerDismissal` → no close (`FFM:411`) | suppressed (`FFM:535`) | `restoreFocus="popup"` ‡ | manager enabled; return-focus persists; close moot | `restoreFocusFrame` ‡ | **only** if `closeOnFocusOut` & focus left tree | + | Popover (click) | modal → trapped; non-modal → trigger counts inside (`FFM:454`) | stays (`FFM:466`) | modal suppressed (`FFM:532`); non-modal closes (`FFM:531`) | suppressed (`FFM:535`) | `restoreFocus="popup"` ‡ (`PopoverPopup:138`) | manager enabled (`disabled={!mounted‖hover}`); trap (if modal) + return-focus persist; close moot | `restoreFocusFrame` ‡ | non-modal+left-tree **Yes**; modal **No** | + | Popover (hover-open) | manager fully `disabled` (`reason===triggerHover`) — no focus-out path | n/a (manager off) | no focus-manager close (hover/`useDismiss` drives close) | n/a | no restore (manager off) | n/a | n/a | **No** (via FFM; hover-driven) | + | Menu root | trigger counts inside (`previousFocusableElement=activeTrigger`, `FFM:462`) | stays — open submenu via containment walk (`FFM:466`) | closes (`!modal`, `FFM:531`) | suppressed (`FFM:535`) | `restoreFocus=true` → prev/last tabbable/popup ‡ (`FFM:511`) | manager enabled; return-focus persists; close moot | re-focus popup ‡ (`FFM:518`) | **Yes** to unrelated | + | Menu submenu | parent menu is an ancestor → focus→parent is inside (`FFM:471`) | stays (`FFM:466`) | closes (`FFM:531`) | suppressed | `restoreFocus=true` ‡ | manager enabled; return-focus persists; close moot | re-focus popup ‡ | **Yes** to unrelated; **No** to ancestor | + | Context Menu | trapped (`modal=true`, `MenuPopup:139`) | stays (`FFM:466`) | suppressed (modal, `FFM:532`) | suppressed | `restoreFocus=true` ‡ | manager enabled; trap + return-focus persist; close moot | re-focus popup ‡ | **No** (modal) | + | Menubar child | trigger registered inside → focus→trigger inside (`FFM:462`) | stays (`FFM:466`) | closes (`FFM:531`) | suppressed | `restoreFocus=true` ‡ | manager enabled; return-focus persists; close moot | re-focus popup ‡ | **Yes** to unrelated | + | **Select** | `modal=false`, `closeOnFocusOut` default **true** (`FFM:260`); trigger is `domReference`, inside (`FFM:454`) | n/a | **closes** — non-modal + unrelated + `relatedTarget` set (`FFM:531`) | suppressed (`FFM:535`) | `restoreFocus=true` ‡ (`SelectPopup:525`) | manager enabled; return-focus persists; close moot | re-focus popup ‡ | **Yes** — Base-UI-vs-Radix divergence (below) | + | Combobox input-inside | `inputInsidePopup=true` → `focusManagerModal=modal` (default **`false`**) → **non-modal, untrapped**; typeable ⇒ `isUntrappedTypeableCombobox` (`ComboboxPopup:117`, `FFM:284`); input is `domReference`, inside (`FFM:454`) | stays (`FFM:466`) | **closes** — untrapped typeable forces close to unrelated (`FFM:543`) | suppressed (`FFM:535`) | `restoreFocus` default `false` → none | manager enabled; return-focus persists; close moot | n/a | **Yes** to unrelated | + | Combobox input-outside | `inputInsidePopup=false` → `focusManagerModal=true` **even when `modal=false`** (`ComboboxPopup:117`); role `presentation`, `resolvedFinalFocus=false` (`:114`) → **modal, trapped** | stays (`FFM:466` + dismiss buttons inside, `ComboboxPopup:127`) | suppressed — modal (`FFM:532`) | suppressed | `restoreFocus=false` → none (focus stays in external input) | manager enabled; trap persists; close moot | n/a | **No** (modal) — closes via outside-press, not focus-out | + | Autocomplete | = Combobox per its `inputInsidePopup` (config-dependent ‡): input-inside ⇒ non-modal/untrapped; input-outside ⇒ modal/trapped | stays (`FFM:466`) | input-inside ⇒ **closes** (`FFM:543`); input-outside ⇒ suppressed (`FFM:532`) | suppressed | `restoreFocus=false` → none | manager enabled; close moot | n/a | input-inside **Yes**; input-outside **No** ‡ | + + **Resolution — Select focus-out divergence (gate).** Base UI Select **closes** on focus-out + (`modal=false`, `closeOnFocusOut` defaults `true`, `FFM:260` → the `!modal` close branch `FFM:531`); + Radix Select currently `preventDefault()`s focus-out and stays open. **Decision: adopt Base UI parity — + `RdxFloatingFocusManager` for Select uses `closeOnFocusOut` default `true` and closes on focus-out to an + unrelated node.** This is a deliberate breaking change for Radix Select, recorded here; the Phase 3 + focus-out implementation drops the Radix `preventDefault()`. (Focus returning to the trigger, staying in + the popup, a pointer-induced move, or a null `relatedTarget` still do **not** close — `FFM:454/535`.) + + **Browser-verify pending (‡ cells):** every `restoreFocus` / `restoreFocusFrame` final-landing outcome + depends on real layout (`isElementVisible`, `activeElement===body`, `FFM:488`) and animation-frame + timing; the WebKit blur-before-unmount (`FFM:889`) is Safari-only; "trigger counts inside" for + non-modal Popover / Menu / Menubar relies on runtime trigger-registration + `previousFocusableElement` + wiring; and **Autocomplete's layout** (`inputInsidePopup`) is set by the root config, not the popup, so + which row it follows (and thus whether it closes on focus-out) must be confirmed per usage. Source-derived + above; confirmed in the Phase 5 Chromium/WebKit matrix. + + **Gate:** each primitive's focus policy must match its row; the known Base-UI-Select-closes-on-focus-out + vs Radix-prevents divergence is recorded with its resolution (above). + +3. **`aria-owns` / content-roots audit (§6a, #4) — multi-IDREF is an Angular _adaptation_, not literal + parity.** Base UI emits a **single** `` pointing at the **one wrapper** + that owns the whole portal subtree (`FloatingPortal.tsx:267`). We have **no wrapper**, so listing + several `contentRootElements` IDREFs is a _reasonable adaptation_ — but **not proven equivalent**. + Phase 0 must validate in a real browser/AT, not assume: (a) do multiple IDREFs reproduce the intended + reading/tab order; (b) are descendants that live under a **non-content** root (e.g. inside the + positioner) lost; (c) is an **invisible semantic portal anchor** (one element wrapping the content roots, + mirroring Base UI's single wrapper) better than enumerating roots. Also record **who mints/owns the + stable IDs** (IDREF targets need SSR-stable `injectId` ids) and the `aria-owns` behavior on a + **container move**. A backdrop is a DOM root but is **never** `aria-owns`'d. **Gate:** + `contentRootElements` is defined separately from `ownRootElements`, and the IDREF-vs-anchor decision is + made from browser/AT evidence (Tier B — recorded now, AT-confirmed before Acceptance in the Phase 5 + matrix); the backdrop is proven absent from any `aria-owns` set. + + **Resolution (source-derived; AT decision browser-pending).** Confirmed: Base UI emits a **single** + `` (`FloatingPortal.tsx:266`) pointing at **one** wrapper + `
` (`:131`), and only in the **non-modal-open** state. We have **no + wrapper** today (`RdxPortalPresence` relocates root nodes with none). **Decision (preferred, source- + derived): adopt the single invisible semantic anchor** (option c) — mint **one** stable-id anchor + (`injectId`, SSR-stable) that owns the content roots, mirroring Base UI's single wrapper, **rather than** + enumerating multiple `contentRootElements` IDREFs (unproven for reading/tab order, and loses descendants + under non-content roots). `contentRootElements` is defined **separately** from `ownRootElements` + (footprint), and a **backdrop is never in any `aria-owns` set**. **Browser/AT-verify pending:** the + single-anchor-vs-multi-IDREF reading/tab-order equivalence and container-move behavior must be confirmed + against a real screen reader **before Acceptance** (Tier B, Phase 5 AT matrix); if multi-IDREF proves + necessary, that is the fallback. + +4. **Positioner lifecycle during animated exit (§6a, #6 — follow-up to ADR 0012).** Confirm Base UI's + positioning lifecycle on close (`useAnchorPositioning` runs `autoUpdate` while `mounted`, gated + `open: mounted` — `useAnchorPositioning.ts:441,507`) and decide our parity: **keep `autoUpdate` running + until unmount** (current `RdxPopperContentWrapper` behavior = parity, no `active` input) vs freeze on + `open=false`. If kept, characterize whether a late `flip`/`shift` can visibly jump the popup mid-exit and + whether any primitive needs to **pin the placement/transform for the exit** (a Popper capability, **not** + a dismissal/focus concern). **Gate:** the positioner's exit-time behavior is decided and, if "keep + running", a test asserts the exit animation is not broken by a placement change. + + **Resolution (source-derived; DECIDED).** Confirmed: Base UI keys positioning on **`mounted`, never + `open`** — `whileElementsMounted: autoUpdate` with `open: undefined` (default) and, for `keepMounted`, a + manual `autoUpdate` effect gated `keepMounted && mounted && reference && floating` + (`useAnchorPositioning.ts:441,506–511`) — so `flip`/`shift` continue through the exit. **Decision: keep + `autoUpdate` running until unmount** = parity, **no change** — our `RdxPopperContentWrapper.autoUpdate` + already lives for the directive's whole lifetime (ADR 0012), no `active` input, no freeze-on-`open=false`. + A "pin placement/transform for the exit" is an explicit **Popper** capability (out of scope here, not a + dismissal/focus concern) and is **not** added unless a primitive needs it. **Browser-verify pending:** a + visual-regression test that a late `flip`/`shift` does not visibly break the exit animation (layout- + dependent → Phase 5 Playwright). + +5. **Portal-ancestry vs custom-container audit (§6a, #1).** Characterize, per portaled primitive, the + resolved `portalParent`: implicit nesting (falls back to the enclosing portal context) vs an explicit + custom `container`. Verify against Base UI's `container ?? parentPortalNode ?? body` that a + custom-container child is **not** physically inside the parent portal subtree, and confirm our + `resolveRegisteredContainerParent(container)` returns the container's registration (or `null`), so the + parent's keep-sets exclude it. **Gate:** the resolved-parent algorithm is implemented and a child with + an explicit body-level container is **not** a keep-set descendant of its logical parent. + + **Resolution (source-derived; needs new infra).** Confirmed Base UI: + `resolvedContainer = container ?? parentPortalNode ?? document.body` (`FloatingPortal.tsx:110–113`) — an + explicit `container` is the portal target directly, so a custom-container child is **physically inside + that container, not the parent portal subtree**. **Our audit:** `resolveRegisteredContainerParent` **does + not exist**; the portal only has the stateless `resolvePortalContainer` (no registry tracking container + parents). **Decision:** mirror `container ?? parentPortal ?? body`, and **build** a portal registry + + `resolveRegisteredContainerParent(container)` that returns the container's registered owner node (or + `null`) so a parent's keep-sets exclude an explicit-body-container child. This is **net-new portal infra** + (Phase 1/4), not present today. Source-derived; the keep-set exclusion is asserted in a Phase 5 test. + +6. **Dismissal-inside vs portal-inside audit (§6a, #2).** Confirm `useDismiss` computes outside-press + "inside" from floating element + reference + **floating-tree children** + **trigger registry** + + **markers** (`useDismiss.ts:173,345,388,393`), **not** `PortalContext` descendants. **Gate:** the + dismissal engine (ADR 0015) treats portal-registry membership as **non-authoritative** for outside-press + — a contextless portal kept for `markOthers` is dismissal-inside **only** via floating descendant or + trigger/branch/inside-element registration. + + **Resolution (source-derived; satisfied by the foundation).** Confirmed: `useDismiss` computes + outside-press "inside" from floating + reference (`composedPath`, `useDismiss.ts:181`), **floating-tree + children** (`getNodeChildren`, `:344`), the **trigger registry** (`store.context.triggerElements`, + `:382`), and **inert markers** (`:373`) — there is **no `PortalContext`** reference in `useDismiss`. + **Decision:** the ADR 0015 engine treats portal-registry membership as **non-authoritative** for + outside-press. This already matches our foundation: containment reads `RdxFloatingTree.children()` + + the per-root-context `RdxTriggerRegistry`, never portal membership. A contextless portal kept only for + `markOthers` is dismissal-inside **only** via a floating descendant (tree child) or + trigger/branch/inside-element registration. Source-derived; gate met by the foundation's design. + +7. **Focus-host resolution audit (§3, #1).** Pin the focusable marker (`FOCUSABLE_ATTRIBUTE` analog) and + the `getFloatingFocusElement(floating)` algorithm per primitive: where `floatingElement` === + `floatingFocusElement` (popup carries handlers) and where they **diverge** (positioner-is-floating, + Select item-aligned, wrapper compositions). **Gate:** the manager resolves the focus host explicitly + (trap / `initialFocus` / `returnFocus` / `tabIndex` operate on `floatingFocusElement`; tree/dismissal on + `floatingElement`); a divergent case (focus host is a child of the floating element) is covered by a test. + + **Resolution (source-derived).** Confirmed: `getFloatingFocusElement(floating)` returns the floating + element when it `hasAttribute(FOCUSABLE_ATTRIBUTE)`, else its `querySelector([FOCUSABLE_ATTRIBUTE])` + match, else the floating element itself (`utils/element.ts:82`); `FOCUSABLE_ATTRIBUTE` is + `data-base-ui-focusable` (`utils/constants.ts:1`). **Decision:** pin a Radix focusable marker (e.g. + `data-rdx-focus-host`) and the identical resolution; the manager resolves the host **explicitly** — trap, + `initialFocus`, `returnFocus`, and `tabIndex` operate on **`floatingFocusElement`**, while tree/dismissal operate on + **`floatingElement`** (distinct roles, §6a five-role list). **Our divergent cases (audited):** Select's + popup **is** the floating element and the focus host (traps via `RdxFocusScope`, `select-popup.ts:101`), + with the positioner a separate geometry ancestor; **item-aligned** Select has no wrapper; Combobox does + **not** trap (host stays the input). A divergent case (focus host nested below the floating element) gets + a Phase-5 test. Source-derived; the marker name is our choice. + +8. **Focus-return traversal filter (#4).** Confirm the focus-inside-tree check on unmount/close walks + descendants with `onlyOpen=false` (`FloatingFocusManager.tsx:842`), so focus inside a closed-but-mounted + descendant still counts as inside the tree. **Gate:** the return-focus path calls the shared traversal + with `onlyOpen: false` (ADR 0015 §1), never the dismissal default `onlyOpen: true`; tested with a + keep-mounted closed child holding focus. + + **Resolution (source-derived; satisfied by the foundation).** Confirmed: the return-focus walk is + `getNodeChildren(tree, id, false)` at `FFM:842` (vs the default `onlyOpen=true` containment walk at + `FFM:466`). **Already implemented:** `RdxFloatingTree.children(node, { onlyOpen: false })` exists and is + documented for exactly this focus-return path (the dismissal default stays `onlyOpen: true`). The + Phase-3 return-focus path calls it with `onlyOpen: false`; a unit test covers a keep-mounted closed + child holding focus still counting as inside the tree. Gate met by the foundation. + +9. **`insideReactTree`-analog capture audit (#5).** Base UI keeps an `insideReactTree` capture marker + (`useDismiss.ts:167,234,367`) separate from DOM/floating tree, guarding document-capture timing and + logical-tree interactions. Our Angular bubbling after DOM relocation does **not** follow the declaration + tree. **Gate:** prove (with tests) that the shared floating tree + owner-`Document` host listeners + **replace** this mechanism — pointer inside a portaled child while a document-capture listener is armed, + child handler `preventDefault`s, parent must **not** dismiss, including when dispatch moves/removes the + target. If Phase 5 proves they do not fully cover it, add the explicit capture-marker analog as a + contingency (Tier B — before Acceptance, not a Phase 1 blocker). + + **Resolution (source-derived; capture-timing browser-pending).** Confirmed: Base UI's + `insideReactTree` is a **boolean on `dataRef.current`** set `true` by **capture-phase** handlers + (`onPointerDownCapture`/`onMouseDownCapture`/… on the floating element, `useDismiss.ts:729–740`), + auto-cleared on a `0ms` timeout (`:233`), and consulted by the document-level outside-press listener + (`if (insideReactTree) { clear(); return; }`, `:367`) — it suppresses one outside-press that bubbled + through the floating React subtree even across portals. **Decision:** for the **common** case the shared + floating tree (logical DI children via `RdxFloatingTree.children()`) + per-root-context trigger registry + + owner-`Document` host listeners **replace** it — an outside-press whose target is inside a portaled + **logical** child is "inside" via the tree walk regardless of DOM relocation. **Browser-pending (gated):** + the specific race it guards — a document-capture listener firing in the **same gesture** where a child + handler `preventDefault`s **and** the target is moved/removed mid-dispatch — is **not** proven by tree + membership alone and must be validated with a Playwright test in Phase 5; **if** it fails, an explicit + capture-marker analog is added as a contingency **before Acceptance** (Tier B, Phase 5). Recorded as the + gate condition. + 10. **Safari/WebKit blur-before-unmount (browser matrix).** Base UI force-`blur()`s a focused input inside a closing popup on WebKit before unmount to avoid a random scroll-to-bottom (`FloatingFocusManager.tsx:885–899`, gated `platform.engine.webkit && !open && floating`). **Gate:** the WebKit browser matrix asserts closing a popup with a focused inner input does **not** scroll the page and that return-focus stays correct. + + **Resolution (source-derived; browser-pending — WebKit only).** Confirmed: gate + `if (!platform.engine.webkit || open || !floating) return;` then blur the active element iff it is a + **typeable** element `contains`ed by `floating` (`FFM:889–900`). **Decision: adopt** — on WebKit only, + when `!open() && floatingElement` and `document.activeElement` is a typeable element inside the floating + element, the manager calls `.blur()` before unmount (platform detection via our browser guard). This is + **inherently WebKit/Safari-only**, so it is **browser-verify pending** — asserted in the Phase 5 + WebKit matrix (closing a popup with a focused inner input does not scroll the page; return-focus stays + correct). Source-derived. + 11. **Combobox input-inside vs input-outside — migrate the two layouts separately (#6, verified).** Base UI sets `focusManagerModal = !inputInsidePopup || modal` (`ComboboxPopup.tsx:117`), so an **input-outside** Combobox uses **modal** focus-manager behavior **even when `modal === false`**, with `returnFocus = false` @@ -752,6 +974,20 @@ complete.** the migration characterizes **both** layouts independently — `focusManagerModal`, `returnFocus`, `role`, the start/end dismiss buttons, and `getInsideElements` — and reconciles the popup's "does-not-trap" claim with Base UI's modal-for-input-outside behavior. + + **Resolution (source-derived).** Confirmed in `ComboboxPopup.tsx`: the focus-manager modality is + `!inputInsidePopup || modal` (`:117`); the resolved final focus is `undefined` for input-inside and + `false` for input-outside (`:114`); the role is `dialog` for input-inside, `presentation` otherwise + (`:81`); inside-elements are the start/end dismiss refs (`:127`); and the trailing dismiss button + renders only when the focus manager is modal (`:134`). **Our audit:** `combobox-popup.ts` does **not** + trap and carries the comment _"focus stays in the input throughout"_ — correct for input-**inside** + (non-modal/untrapped) but **wrong for input-outside**. **Decision (matches the corrected focus-out + table rows):** migrate the two layouts independently — **input-inside** is non-modal/untrapped + (`role=dialog`, default return focus); **input-outside** is modal/trapped even when `modal` is + false (`role=presentation`, return focus `false` so focus stays in the external input), and renders the + start/end dismiss buttons. The Radix "does-not-trap" comment is corrected in the Phase 4 migration. + Source-derived; reconciled with item #2. + 12. **Navigation Menu may run dismissal without a full floating node (#8, verified).** Base UI's Navigation Menu uses `useDismiss` with a **fallback empty floating context**, enabling interactions only when a positioner/value exists (`NavigationMenuList.tsx`). **Gate:** the shared dismissal API (ADR 0015) must @@ -759,9 +995,38 @@ complete.** **only when a popup exists**, or the capability tolerates a temporarily-absent node — so Navigation Menu is not forced to register a full node in every state. -### Phase 1: `RdxFloatingFocusManager` skeleton + **Resolution (source-derived; satisfied by the foundation).** Confirmed: Base UI's NavMenu runs + `useDismiss` with a **fallback empty floating context** (`getEmptyRootContext()` — a `FloatingRootStore` + with **no** tree node), enabling interactions only when a positioner/value exists. **Already + implemented:** the foundation provides `createFloatingRootContext()` (the `getEmptyRootContext` analog) + and the **node-optional capability model** — `RdxDismissableCapability` references a **root context + mandatorily** and a **node optionally** (ADR 0015 §1). So NavMenu registers a tree node **only when a + popup exists** and otherwise runs dismissal off a standalone root context with `node === null`. Gate met + by the foundation; the wiring lands in the Phase 4 migration. + +### Phase 1: Low-level focus foundation, then the `RdxFloatingFocusManager` skeleton + +This phase has **two ordered steps** — the foundation lands **before** the skeleton, and both before any +primitive migration (Phase 4). It is the home for the Phase 0 #0 / §6 rework, which today **does not exist**. + +**1a — low-level foundation (build / rework first):** + +- **Rework `RdxFocusScope`** (Phase 0 #0): owner-`Document` (not global `document`), shadow/`composedPath`-aware + containment (not bare `contains()`), and queued focus (rAF / `afterRenderEffect`, not `setTimeout`). The + **active-scope stack moves to `WeakMap`** — it pauses/resumes scopes, so it **is** + cross-document state (opening a scope in document B must not pause document A's scope) and cannot stay + process-global. Only a **passive** previously-focused-element history (a WeakRef list, no pause/resume + coordination) may remain module-global — that, not the active stack, is the true Base UI + `previouslyFocusedElements` analogue. +- **Build the portal-focus bridge + `RdxFocusGuards`** (Phase 0 #0 / §6 — net-new): leading/trailing + visually-hidden `tabindex=0` guard spans + capture-phase inside-tabbability toggle + the single `aria-owns` + anchor, tied to `RdxPortal` / `RdxPortalPresence`. +- Owner-`Document` focus guards (§6). + +**1b — manager skeleton:** -- Implement the independent policy set (§1, §2) composing `RdxFocusScope`; wire focus lifecycle + enabled. +- Implement the independent policy set (§1, §2) by **composing** the three low-level parts (reworked + `RdxFocusScope` + portal-focus bridge + owner-document guards); wire focus lifecycle + `enabled`. ### Phase 2: aria-hidden + marker passes @@ -936,9 +1201,15 @@ complete.** 10. `RdxFloatingFocusManager` reads the **shared** floating infrastructure (ADR 0015 §1) — nodes, traversal, the shared **trigger registry** (§2), and the typed **event channels** — not a focus-only tree or a duplicate trigger list. -11. Focus scope, focus guards, and portal-focus coordination are **owner-`Document`-scoped** (no - module-global `count`/`document`/stack), verified with two-document/iframe tests (§6) — the same - isolation this trilogy applies to pointer-events and scroll lock. +11. Focus scope, focus guards, and portal-focus coordination are **owner-`Document`-scoped**: no + module-global **`document` / `document.body`** references (listeners, return-focus fallbacks, and + tabbable queries all key off the owner document), verified with two-document/iframe tests (§6). The + **active-scope stack** (which pauses/resumes scopes) is **per-`Document`** — `WeakMap`, **not** process-global — because pausing document A's scope when a scope opens in + document B is exactly the cross-document corruption this forbids. Only a **passive** previously-focused + history (a WeakRef list, no pause/resume coordination) may stay module-global; that is the real Base UI + `previouslyFocusedElements` analogue, **not** the active stack. The hard isolation requirement is the + same one this trilogy applies to pointer-events and scroll lock: per-`Document`, not process-global. 12. The **per-effect lifecycle split** is honored exactly (verified line-by-line, §3/§6a): **focus-trap structure, close-on-focus-out, and return-focus follow `mounted`** (persist while mounted-but-closed, return-focus fires on that lifecycle, not the raw `open` flip), while **marker, `aria-hidden`, diff --git a/packages/primitives/alert-dialog/__tests__/alert-dialog.spec.ts b/packages/primitives/alert-dialog/__tests__/alert-dialog.spec.ts index e18f97e03..ea37a64fd 100644 --- a/packages/primitives/alert-dialog/__tests__/alert-dialog.spec.ts +++ b/packages/primitives/alert-dialog/__tests__/alert-dialog.spec.ts @@ -137,16 +137,18 @@ describe('AlertDialog', () => { }); it('locks body scroll while open (always modal)', () => { - expect(document.body.style.overflow).toBe(''); + // `useScrollLock` marks `` with `data-rdx-scroll-locked` (strategy-independent signal). + const locked = () => document.documentElement.hasAttribute('data-rdx-scroll-locked'); + expect(locked()).toBe(false); trigger.click(); fixture.detectChanges(); - expect(document.body.style.overflow).toBe('hidden'); + expect(locked()).toBe(true); const close: HTMLButtonElement = document.body.querySelector('[rdxAlertDialogClose]')!; close.click(); fixture.detectChanges(); - expect(document.body.style.overflow).toBe(''); + expect(locked()).toBe(false); }); it('supports controlled open via the model', () => { diff --git a/packages/primitives/autocomplete/src/autocomplete-clear.ts b/packages/primitives/autocomplete/src/autocomplete-clear.ts index 9c5a66952..689efa4c6 100644 --- a/packages/primitives/autocomplete/src/autocomplete-clear.ts +++ b/packages/primitives/autocomplete/src/autocomplete-clear.ts @@ -1,5 +1,5 @@ import { computed, Directive, inject } from '@angular/core'; -import { RdxDismissableLayerBranch } from '@radix-ng/primitives/dismissable-layer'; +import { RdxFloatingInsideElement } from '@radix-ng/primitives/dismissable-layer'; import { RdxAutocompleteRoot } from './autocomplete-root'; /** @@ -10,7 +10,7 @@ import { RdxAutocompleteRoot } from './autocomplete-root'; @Directive({ selector: 'button[rdxAutocompleteClear]', exportAs: 'rdxAutocompleteClear', - hostDirectives: [RdxDismissableLayerBranch], + hostDirectives: [RdxFloatingInsideElement], host: { type: 'button', tabindex: '-1', diff --git a/packages/primitives/autocomplete/src/autocomplete-input.ts b/packages/primitives/autocomplete/src/autocomplete-input.ts index b02491257..f0341901e 100644 --- a/packages/primitives/autocomplete/src/autocomplete-input.ts +++ b/packages/primitives/autocomplete/src/autocomplete-input.ts @@ -10,7 +10,7 @@ import { input } from '@angular/core'; import { BooleanInput, injectId } from '@radix-ng/primitives/core'; -import { RdxDismissableLayerBranch } from '@radix-ng/primitives/dismissable-layer'; +import { RdxFloatingInsideElement } from '@radix-ng/primitives/dismissable-layer'; import { injectFieldRootContext } from '@radix-ng/primitives/field'; import { RdxPopperAnchor } from '@radix-ng/primitives/popper'; import { RdxAutocompletePositioner } from './autocomplete-positioner'; @@ -28,7 +28,7 @@ const attr = (value: boolean) => (value ? '' : undefined); @Directive({ selector: 'input[rdxAutocompleteInput]', exportAs: 'rdxAutocompleteInput', - hostDirectives: [RdxPopperAnchor, RdxDismissableLayerBranch], + hostDirectives: [RdxPopperAnchor, RdxFloatingInsideElement], host: { role: 'combobox', autocomplete: 'off', @@ -254,7 +254,7 @@ export class RdxAutocompleteInput { } else if (!this.root.popupMounted()) { // Base UI: Escape on a closed autocomplete clears the input value (a no-op while // read-only / disabled). Guard on `popupMounted` so the same Escape that just closed - // an open popup (dismissable layer, capture phase) doesn't also clear. + // an open popup (the `open` branch above) doesn't also clear. this.root.clearValue(); } break; diff --git a/packages/primitives/autocomplete/src/autocomplete-popup.ts b/packages/primitives/autocomplete/src/autocomplete-popup.ts index d4131b0a7..7e2128539 100644 --- a/packages/primitives/autocomplete/src/autocomplete-popup.ts +++ b/packages/primitives/autocomplete/src/autocomplete-popup.ts @@ -1,6 +1,11 @@ -import { afterRenderEffect, DestroyRef, Directive, ElementRef, inject } from '@angular/core'; -import { useScrollLock } from '@radix-ng/primitives/core'; -import { provideRdxDismissableLayerConfig, RdxDismissableLayer } from '@radix-ng/primitives/dismissable-layer'; +import { afterRenderEffect, computed, DestroyRef, Directive, ElementRef, inject } from '@angular/core'; +import { + RDX_FLOATING_REGISTRATION, + RDX_FLOATING_ROOT_CONTEXT, + RdxFloatingNodeRegistration, + useAnchoredScrollLock +} from '@radix-ng/primitives/core'; +import { RdxDismiss } from '@radix-ng/primitives/dismissable-layer'; import { injectPopperContentWrapperContext, RdxPopperContent } from '@radix-ng/primitives/popper'; import { RdxAutocompleteRoot } from './autocomplete-root'; @@ -15,12 +20,7 @@ import { RdxAutocompleteRoot } from './autocomplete-root'; @Directive({ selector: '[rdxAutocompletePopup]', exportAs: 'rdxAutocompletePopup', - hostDirectives: [RdxPopperContent, RdxDismissableLayer], - providers: [ - provideRdxDismissableLayerConfig(() => ({ - disableOutsidePointerEvents: inject(RdxAutocompleteRoot).modal - })) - ], + hostDirectives: [RdxPopperContent, RdxFloatingNodeRegistration], host: { // Base UI: a `dialog` (focusable, tabindex -1) when the input lives inside the popup, otherwise // a presentational wrapper around the `listbox` (the List part owns the listbox role). @@ -36,12 +36,23 @@ import { RdxAutocompleteRoot } from './autocomplete-root'; }) export class RdxAutocompletePopup { protected readonly root = inject(RdxAutocompleteRoot); - private readonly dismissableLayer = inject(RdxDismissableLayer); + private readonly floatingContext = inject(RDX_FLOATING_ROOT_CONTEXT); + private readonly registration = inject(RDX_FLOATING_REGISTRATION, { optional: true }); private readonly popper = injectPopperContentWrapperContext(); private readonly element = inject>(ElementRef).nativeElement; constructor() { - useScrollLock(this.root.modal); + // Activation policy (ADR 0016 §2 + §3): lock page scroll while a modal popup is OPEN, gated on + // `open` (not mounted) so the lock releases at close-start. For a **touch** open the anchored + // helper only locks when the popup is effectively viewport-width (a small dropdown stays + // swipe-to-dismissable on mobile, §3). + useAnchoredScrollLock( + computed(() => this.root.open() && this.root.modal()), + { + touchOpen: () => this.root.openedByTouch(), + element: () => this.element + } + ); const unregister = this.root.registerTransitionElement(this.element); // Track mounted state so Escape can tell "closing this open popup" from "already closed". @@ -51,7 +62,19 @@ export class RdxAutocompletePopup { this.root.setPopupMounted(false); }); - this.dismissableLayer.dismiss.subscribe(() => this.root.closePopup(true)); + // The popup is this layer's floating element (the inside surface for containment checks). + this.floatingContext.setFloatingElement(this.element); + + // Dismissal (ADR 0015): an outside press, or focus leaving everything, closes the autocomplete. The + // input / trigger / clear are registered as "inside" (RdxFloatingInsideElement), so the input keeping + // focus — or a press on those parts — never self-dismisses. Escape is owned by the input (it + // preventDefaults + closes), so the capability does not handle it (`escapeKey: false`). + new RdxDismiss(this.floatingContext, () => this.registration?.node() ?? null, { + escapeKey: () => false, + outsidePress: () => true, + focusOutside: () => true, + onDismiss: () => this.root.closePopup(true) + }); // For the "input inside the popup" pattern, move focus to the input once positioned. Use // `afterRenderEffect` (not `effect`): when `isPositioned` flips true the popup's final diff --git a/packages/primitives/autocomplete/src/autocomplete-positioner.ts b/packages/primitives/autocomplete/src/autocomplete-positioner.ts index c89e11b8b..910a95ce7 100644 --- a/packages/primitives/autocomplete/src/autocomplete-positioner.ts +++ b/packages/primitives/autocomplete/src/autocomplete-positioner.ts @@ -1,9 +1,11 @@ -import { Directive } from '@angular/core'; +import { afterNextRender, Directive, ElementRef, inject, Injector } from '@angular/core'; +import { setupInternalBackdrop } from '@radix-ng/primitives/core'; import { provideRdxPopperContentConfig, provideRdxPopperContentWrapper, RdxPopperContentWrapper } from '@radix-ng/primitives/popper'; +import { RdxAutocompleteRoot } from './autocomplete-root'; /** * Positions the autocomplete popup relative to the input anchor using the popper engine. @@ -23,4 +25,20 @@ import { provideRdxPopperContentConfig({ sideOffset: 4, align: 'start' }) ] }) -export class RdxAutocompletePositioner extends RdxPopperContentWrapper {} +export class RdxAutocompletePositioner extends RdxPopperContentWrapper { + constructor() { + super(); + const root = inject(RdxAutocompleteRoot); + const injector = inject(Injector); + const host = inject>(ElementRef).nativeElement; + // A modal autocomplete isolates the background with an internal backdrop (Base UI); the input stays + // clickable through a cutout. (Autocomplete is non-modal by default — usually no backdrop.) + afterNextRender(() => + setupInternalBackdrop(host, injector, { + isOpen: () => root.open(), + shouldRender: () => root.modal(), + cutout: () => root.inputElement() ?? null + }) + ); + } +} diff --git a/packages/primitives/autocomplete/src/autocomplete-root.ts b/packages/primitives/autocomplete/src/autocomplete-root.ts index d1f3a0174..68a6f97d6 100644 --- a/packages/primitives/autocomplete/src/autocomplete-root.ts +++ b/packages/primitives/autocomplete/src/autocomplete-root.ts @@ -3,6 +3,7 @@ import { computed, Directive, effect, + ElementRef, inject, Injector, input, @@ -26,11 +27,15 @@ import { import { AcceptableValue, BooleanInput, + createFloatingRootContext, itemToStringLabel as defaultItemToStringLabel, Direction, isItemEqualToValue as itemsEqual, ItemValueComparator, - rdxDevWarning + provideFloatingRootContext, + provideFloatingTree, + rdxDevWarning, + RdxFloatingRootContext } from '@radix-ng/primitives/core'; import { RdxPopper } from '@radix-ng/primitives/popper'; @@ -182,7 +187,10 @@ function coerceAutoHighlight(value: BooleanInput | 'always'): boolean | 'always' exportAs: 'rdxAutocompleteRoot', providers: [ provideComboboxRootContext(context), - { provide: NG_VALUE_ACCESSOR, useExisting: RdxAutocompleteRoot, multi: true } + { provide: NG_VALUE_ACCESSOR, useExisting: RdxAutocompleteRoot, multi: true }, + // New floating foundation (ADR 0015/0017) — the dismissal capability reads this shared context. + provideFloatingTree(), + provideFloatingRootContext(() => inject(RdxAutocompleteRoot).floatingContext) ], hostDirectives: [RdxPopper], host: { @@ -192,6 +200,12 @@ function coerceAutoHighlight(value: BooleanInput | 'always'): boolean | 'always' export class RdxAutocompleteRoot implements ControlValueAccessor { private readonly injector = inject(Injector); + /** Per-popup floating root context (ADR 0015) — `open` / `triggers` / reference for the dismissal engine. */ + readonly floatingContext: RdxFloatingRootContext = createFloatingRootContext({ + ownerDocument: inject(ElementRef).nativeElement.ownerDocument, + open: () => this.open() + }); + /** The input text. This is the form value (controlled / uncontrolled via {@link defaultValue}). */ readonly value = model(''); @@ -409,6 +423,10 @@ export class RdxAutocompleteRoot implements ControlValueAccessor { constructor() { engineRegistry.set(this, this.engine); + // Keep the dismissal reference in sync with the input (the anchor) so a press / focus on it counts + // as "inside" and never dismisses (ADR 0015). + effect(() => this.floatingContext.setReferenceElement(this.engine.inputElement() ?? null)); + // Apply uncontrolled defaults once. effect(() => { const initial = this.defaultValue(); diff --git a/packages/primitives/combobox/__tests__/combobox-modal.spec.ts b/packages/primitives/combobox/__tests__/combobox-modal.spec.ts index 123bc6cda..665d5ae9e 100644 --- a/packages/primitives/combobox/__tests__/combobox-modal.spec.ts +++ b/packages/primitives/combobox/__tests__/combobox-modal.spec.ts @@ -47,13 +47,13 @@ describe('Combobox modal', () => { }); it('locks body scroll while a modal popup is open and restores it on close', async () => { - expect(document.body.style.overflow).not.toBe('hidden'); + expect(document.documentElement.hasAttribute('data-rdx-scroll-locked')).toBe(false); host.open.set(true); await settle(); - expect(document.body.style.overflow).toBe('hidden'); + expect(document.documentElement.hasAttribute('data-rdx-scroll-locked')).toBe(true); host.open.set(false); await settle(); - expect(document.body.style.overflow).not.toBe('hidden'); + expect(document.documentElement.hasAttribute('data-rdx-scroll-locked')).toBe(false); }); it('does not lock scroll when not modal', async () => { @@ -61,7 +61,7 @@ describe('Combobox modal', () => { await settle(); host.open.set(true); await settle(); - expect(document.body.style.overflow).not.toBe('hidden'); + expect(document.documentElement.hasAttribute('data-rdx-scroll-locked')).toBe(false); }); it('renders a backdrop with data-open while open', async () => { @@ -72,10 +72,16 @@ describe('Combobox modal', () => { expect(backdrop.hasAttribute('data-open')).toBe(true); }); - it('keeps the popup itself interactive (pointer-events auto) while outside is inert', async () => { + it('renders an internal backdrop for a modal popup (Base UI; replaces the global body pointer-lock)', async () => { host.open.set(true); await settle(); + + // A modal combobox isolates the background with a full-viewport internal backdrop (the + // outside-press target) instead of a global `body { pointer-events: none }` lock. + expect(document.querySelector('[data-rdx-internal-backdrop]')).toBeTruthy(); + + // With no body lock, the popup needs no `pointer-events: auto` opt-back-in. const popup = document.querySelector('[rdxComboboxPopup]') as HTMLElement; - expect(popup.style.pointerEvents).toBe('auto'); + expect(popup.style.pointerEvents).toBe(''); }); }); diff --git a/packages/primitives/combobox/src/combobox-chips.ts b/packages/primitives/combobox/src/combobox-chips.ts index adeef8f63..dae061f38 100644 --- a/packages/primitives/combobox/src/combobox-chips.ts +++ b/packages/primitives/combobox/src/combobox-chips.ts @@ -1,5 +1,5 @@ import { DestroyRef, Directive, ElementRef, inject } from '@angular/core'; -import { RdxDismissableLayerBranch } from '@radix-ng/primitives/dismissable-layer'; +import { RdxFloatingInsideElement } from '@radix-ng/primitives/dismissable-layer'; import { injectComboboxRootContext } from './combobox-root'; /** @@ -13,7 +13,7 @@ import { injectComboboxRootContext } from './combobox-root'; @Directive({ selector: '[rdxComboboxChips]', exportAs: 'rdxComboboxChips', - hostDirectives: [RdxDismissableLayerBranch], + hostDirectives: [RdxFloatingInsideElement], host: { role: 'toolbar' } diff --git a/packages/primitives/combobox/src/combobox-clear.ts b/packages/primitives/combobox/src/combobox-clear.ts index 4920affa9..2325e4116 100644 --- a/packages/primitives/combobox/src/combobox-clear.ts +++ b/packages/primitives/combobox/src/combobox-clear.ts @@ -1,5 +1,5 @@ import { booleanAttribute, computed, Directive, input } from '@angular/core'; -import { RdxDismissableLayerBranch } from '@radix-ng/primitives/dismissable-layer'; +import { RdxFloatingInsideElement } from '@radix-ng/primitives/dismissable-layer'; import { injectComboboxRootContext } from './combobox-root'; /** @@ -10,7 +10,7 @@ import { injectComboboxRootContext } from './combobox-root'; @Directive({ selector: 'button[rdxComboboxClear]', exportAs: 'rdxComboboxClear', - hostDirectives: [RdxDismissableLayerBranch], + hostDirectives: [RdxFloatingInsideElement], host: { type: 'button', tabindex: '-1', diff --git a/packages/primitives/combobox/src/combobox-engine.ts b/packages/primitives/combobox/src/combobox-engine.ts index 8b258d4cc..771d224e7 100644 --- a/packages/primitives/combobox/src/combobox-engine.ts +++ b/packages/primitives/combobox/src/combobox-engine.ts @@ -134,7 +134,7 @@ export function useComboboxEngine(config: ComboboxEngineConfig) { // Whether the popup directive is currently mounted (open through the close/exit animation, until the // presence machine unmounts it). Distinguishes "Escape closed the popup" (still mounted this tick) // from "Escape on an already-closed combobox" (unmounted) — Base UI's `mounted`, since `open()` - // flips synchronously when the dismissable layer closes in the capture phase. + // flips synchronously when the input's Escape handler (or the dismiss mechanism) closes the popup. const popupMounted = signal(false); let triggerElement: HTMLElement | null = null; // Tracks whether the last interaction was the keyboard, so the highlight doesn't jump to an item diff --git a/packages/primitives/combobox/src/combobox-input.ts b/packages/primitives/combobox/src/combobox-input.ts index b656fef9c..9dca3b87a 100644 --- a/packages/primitives/combobox/src/combobox-input.ts +++ b/packages/primitives/combobox/src/combobox-input.ts @@ -9,7 +9,7 @@ import { input } from '@angular/core'; import { BooleanInput, injectId } from '@radix-ng/primitives/core'; -import { RdxDismissableLayerBranch } from '@radix-ng/primitives/dismissable-layer'; +import { RdxFloatingInsideElement } from '@radix-ng/primitives/dismissable-layer'; import { injectFieldRootContext } from '@radix-ng/primitives/field'; import { RdxPopperAnchor } from '@radix-ng/primitives/popper'; import { RdxComboboxPositioner } from './combobox-positioner'; @@ -26,7 +26,7 @@ const attr = (value: boolean) => (value ? '' : undefined); @Directive({ selector: 'input[rdxComboboxInput]', exportAs: 'rdxComboboxInput', - hostDirectives: [RdxPopperAnchor, RdxDismissableLayerBranch], + hostDirectives: [RdxPopperAnchor, RdxFloatingInsideElement], host: { role: 'combobox', autocomplete: 'off', @@ -215,9 +215,9 @@ export class RdxComboboxInput { } else if (!this.rootContext.popupMounted()) { // Base UI: Escape on a closed combobox clears the input text and the selection // (`clearSelection` resets both, a no-op while read-only / disabled). Guard on - // `popupMounted`: the dismissable layer closes in the capture phase, so `open()` is - // already false here when this same Escape just closed an open popup — in that case - // the popup is still mounted (exiting) and we must not also clear. + // `popupMounted`: the input's own Escape handler (the `open` branch above) already set + // `open()` false when this same Escape just closed an open popup — in that case the + // popup is still mounted (exiting) and we must not also clear. this.rootContext.clearSelection(); } break; diff --git a/packages/primitives/combobox/src/combobox-popup.ts b/packages/primitives/combobox/src/combobox-popup.ts index 0217fcd9c..6262da9c8 100644 --- a/packages/primitives/combobox/src/combobox-popup.ts +++ b/packages/primitives/combobox/src/combobox-popup.ts @@ -1,6 +1,11 @@ -import { afterRenderEffect, DestroyRef, Directive, ElementRef, inject } from '@angular/core'; -import { useScrollLock } from '@radix-ng/primitives/core'; -import { provideRdxDismissableLayerConfig, RdxDismissableLayer } from '@radix-ng/primitives/dismissable-layer'; +import { afterRenderEffect, computed, DestroyRef, Directive, ElementRef, inject } from '@angular/core'; +import { + RDX_FLOATING_REGISTRATION, + RDX_FLOATING_ROOT_CONTEXT, + RdxFloatingNodeRegistration, + useAnchoredScrollLock +} from '@radix-ng/primitives/core'; +import { RdxDismiss } from '@radix-ng/primitives/dismissable-layer'; import { injectPopperContentWrapperContext, RdxPopperContent } from '@radix-ng/primitives/popper'; import { injectComboboxRootContext } from './combobox-root'; @@ -13,11 +18,7 @@ import { injectComboboxRootContext } from './combobox-root'; @Directive({ selector: '[rdxComboboxPopup]', exportAs: 'rdxComboboxPopup', - hostDirectives: [RdxPopperContent, RdxDismissableLayer], - providers: [ - // In modal mode, make content outside the popup inert (Base UI's `modal`). - provideRdxDismissableLayerConfig(() => ({ disableOutsidePointerEvents: injectComboboxRootContext().modal })) - ], + hostDirectives: [RdxPopperContent, RdxFloatingNodeRegistration], host: { // Base UI: a `dialog` (focusable, tabindex -1) when the input lives inside the popup, otherwise // a presentational wrapper around the `listbox` (the List part owns the listbox role). @@ -33,14 +34,24 @@ import { injectComboboxRootContext } from './combobox-root'; }) export class RdxComboboxPopup { protected readonly rootContext = injectComboboxRootContext(); - private readonly dismissableLayer = inject(RdxDismissableLayer); + private readonly floatingContext = inject(RDX_FLOATING_ROOT_CONTEXT); + private readonly registration = inject(RDX_FLOATING_REGISTRATION, { optional: true }); private readonly popper = injectPopperContentWrapperContext(); private readonly element = inject>(ElementRef).nativeElement; constructor() { - // The popup mounts only while open, so locking on `modal` locks scroll for as long as a modal - // popup is open and releases it on close. - useScrollLock(this.rootContext.modal); + // Activation policy (ADR 0016 §2 + §3): lock page scroll while a modal popup is OPEN. The gate + // keys on `open` (not mounted), so the lock releases at close-start — before the exit animation + // finishes — even though the popup stays mounted through it. For a **touch** open the anchored + // helper only locks when the popup is effectively viewport-width, so a small dropdown stays + // swipe-to-dismissable on mobile (§3). + useAnchoredScrollLock( + computed(() => this.rootContext.open() && this.rootContext.modal()), + { + touchOpen: () => this.rootContext.openedByTouch(), + element: () => this.element + } + ); // The popup's animation determines when the open/close transition (onOpenChangeComplete) is done. const unregister = this.rootContext.registerTransitionElement(this.element); @@ -51,10 +62,19 @@ export class RdxComboboxPopup { this.rootContext.setPopupMounted(false); }); - // The input keeps focus while the popup is open; it is registered as a layer branch, so - // focus/pointer interactions on it don't count as "outside" and won't self-dismiss. Escape - // is handled by the input (which calls preventDefault), so the layer won't dismiss for it. - this.dismissableLayer.dismiss.subscribe(() => this.rootContext.closePopup(true)); + // The popup is this layer's floating element (the inside surface for containment checks). + this.floatingContext.setFloatingElement(this.element); + + // Dismissal (ADR 0015): an outside press, or focus leaving everything, closes the combobox. The + // input / trigger / chips / clear are registered as "inside" (RdxFloatingInsideElement), so the + // input keeping focus — or a press on those parts — never self-dismisses. Escape is owned by the + // input (it preventDefaults + closes), so the capability does not handle it (`escapeKey: false`). + new RdxDismiss(this.floatingContext, () => this.registration?.node() ?? null, { + escapeKey: () => false, + outsidePress: () => true, + focusOutside: () => true, + onDismiss: () => this.rootContext.closePopup(true) + }); // For the "input inside the popup" pattern, move focus to the input once the popup is // positioned. Use `afterRenderEffect` (not `effect`): when `isPositioned` flips true the diff --git a/packages/primitives/combobox/src/combobox-positioner.ts b/packages/primitives/combobox/src/combobox-positioner.ts index 24c84932b..23ccdd0fa 100644 --- a/packages/primitives/combobox/src/combobox-positioner.ts +++ b/packages/primitives/combobox/src/combobox-positioner.ts @@ -1,9 +1,11 @@ -import { Directive } from '@angular/core'; +import { afterNextRender, Directive, ElementRef, inject, Injector } from '@angular/core'; +import { setupInternalBackdrop } from '@radix-ng/primitives/core'; import { provideRdxPopperContentConfig, provideRdxPopperContentWrapper, RdxPopperContentWrapper } from '@radix-ng/primitives/popper'; +import { injectComboboxRootContext } from './combobox-root'; /** * Positions the combobox popup relative to the input anchor using the popper engine. @@ -24,4 +26,20 @@ import { provideRdxPopperContentConfig({ sideOffset: 4, align: 'start' }) ] }) -export class RdxComboboxPositioner extends RdxPopperContentWrapper {} +export class RdxComboboxPositioner extends RdxPopperContentWrapper { + constructor() { + super(); + const rootContext = injectComboboxRootContext(); + const injector = inject(Injector); + const host = inject>(ElementRef).nativeElement; + // A modal combobox isolates the background with an internal backdrop (Base UI); the input stays + // clickable through a cutout. (Combobox is non-modal by default — usually no backdrop.) + afterNextRender(() => + setupInternalBackdrop(host, injector, { + isOpen: () => rootContext.open(), + shouldRender: () => rootContext.modal(), + cutout: () => rootContext.inputElement() ?? null + }) + ); + } +} diff --git a/packages/primitives/combobox/src/combobox-root.ts b/packages/primitives/combobox/src/combobox-root.ts index f9596f6ed..7c408dfce 100644 --- a/packages/primitives/combobox/src/combobox-root.ts +++ b/packages/primitives/combobox/src/combobox-root.ts @@ -3,6 +3,7 @@ import { computed, Directive, effect, + ElementRef, inject, Injector, input, @@ -18,11 +19,15 @@ import { AcceptableValue, BooleanInput, createContext, + createFloatingRootContext, itemToStringLabel as defaultItemToStringLabel, Direction, isNullish, isItemEqualToValue as itemsEqual, - ItemValueComparator + ItemValueComparator, + provideFloatingRootContext, + provideFloatingTree, + RdxFloatingRootContext } from '@radix-ng/primitives/core'; import { RdxPopper } from '@radix-ng/primitives/popper'; import { @@ -159,7 +164,10 @@ function coerceAutoHighlight(value: BooleanInput | 'always' | 'input-change'): b exportAs: 'rdxComboboxRoot', providers: [ provideComboboxRootContext(context), - { provide: NG_VALUE_ACCESSOR, useExisting: RdxComboboxRoot, multi: true } + { provide: NG_VALUE_ACCESSOR, useExisting: RdxComboboxRoot, multi: true }, + // New floating foundation (ADR 0015/0017) — the dismissal capability reads this shared context. + provideFloatingTree(), + provideFloatingRootContext(() => inject(RdxComboboxRoot).floatingContext) ], hostDirectives: [RdxPopper], host: { @@ -169,6 +177,12 @@ function coerceAutoHighlight(value: BooleanInput | 'always' | 'input-change'): b export class RdxComboboxRoot implements ControlValueAccessor { private readonly injector = inject(Injector); + /** Per-popup floating root context (ADR 0015) — `open` / `triggers` / reference for the dismissal engine. */ + readonly floatingContext: RdxFloatingRootContext = createFloatingRootContext({ + ownerDocument: inject(ElementRef).nativeElement.ownerDocument, + open: () => this.open() + }); + /** Selected value(s). A single value in single mode, an array in `multiple` mode. */ readonly value = model(null); @@ -399,6 +413,10 @@ export class RdxComboboxRoot implements ControlValueAccessor { // Expose the (private) engine to the context factory, which is a free function. engineRegistry.set(this, this.engine); + // Keep the dismissal reference in sync with the input (the anchor) so a press / focus on it counts + // as "inside" and never dismisses (ADR 0015). + effect(() => this.floatingContext.setReferenceElement(this.engine.inputElement() ?? null)); + // Apply uncontrolled defaults once. effect(() => { const initial = this.defaultValue(); diff --git a/packages/primitives/combobox/src/combobox-trigger.ts b/packages/primitives/combobox/src/combobox-trigger.ts index cbb6723a5..b839f54ef 100644 --- a/packages/primitives/combobox/src/combobox-trigger.ts +++ b/packages/primitives/combobox/src/combobox-trigger.ts @@ -1,5 +1,5 @@ import { DestroyRef, Directive, ElementRef, inject } from '@angular/core'; -import { RdxDismissableLayerBranch } from '@radix-ng/primitives/dismissable-layer'; +import { RdxFloatingInsideElement } from '@radix-ng/primitives/dismissable-layer'; import { injectComboboxRootContext } from './combobox-root'; /** @@ -20,7 +20,7 @@ import { injectComboboxRootContext } from './combobox-root'; @Directive({ selector: 'button[rdxComboboxTrigger]', exportAs: 'rdxComboboxTrigger', - hostDirectives: [RdxDismissableLayerBranch], + hostDirectives: [RdxFloatingInsideElement], host: { type: 'button', '[attr.tabindex]': 'rootContext.inputLayout() === "outside" ? "-1" : "0"', diff --git a/packages/primitives/context-menu/__tests__/context-menu.spec.ts b/packages/primitives/context-menu/__tests__/context-menu.spec.ts index 2b35d46f2..6dd1b1256 100644 --- a/packages/primitives/context-menu/__tests__/context-menu.spec.ts +++ b/packages/primitives/context-menu/__tests__/context-menu.spec.ts @@ -2,6 +2,7 @@ import { Component } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { RdxContextMenuModule } from '@radix-ng/primitives/context-menu'; import { RdxMenuModule } from '@radix-ng/primitives/menu'; +import { afterEach, vi } from 'vitest'; function rightClick(target: Element, clientX = 120, clientY = 80) { target.dispatchEvent(new MouseEvent('contextmenu', { bubbles: true, cancelable: true, clientX, clientY })); @@ -19,6 +20,10 @@ function flushRaf() { return new Promise((resolve) => requestAnimationFrame(() => resolve())); } +afterEach(() => { + vi.useRealTimers(); +}); + @Component({ imports: [RdxContextMenuModule, RdxMenuModule], template: ` @@ -115,6 +120,36 @@ describe('ContextMenu', () => { expect(event.defaultPrevented).toBe(true); }); + it('does not cancel opening on mouseup before the 500ms grace window elapses', () => { + vi.useFakeTimers(); + + pointerDown(trigger); + rightClick(trigger); + fixture.detectChanges(); + + vi.advanceTimersByTime(499); + document.body.dispatchEvent(new MouseEvent('mouseup', { bubbles: true })); + fixture.detectChanges(); + + expect(trigger.getAttribute('data-state')).toBe('open'); + expect(fixture.nativeElement.querySelector('[rdxMenuPopup]')).not.toBeNull(); + }); + + it('cancels opening on mouseup outside after the 500ms grace window', () => { + vi.useFakeTimers(); + + pointerDown(trigger); + rightClick(trigger); + fixture.detectChanges(); + + vi.advanceTimersByTime(500); + document.body.dispatchEvent(new MouseEvent('mouseup', { bubbles: true })); + fixture.detectChanges(); + + expect(trigger.getAttribute('data-state')).toBe('closed'); + expect(fixture.nativeElement.querySelector('[rdxMenuPopup]')).toBeNull(); + }); + it('closes on Escape and stays closed', () => { rightClick(trigger); fixture.detectChanges(); diff --git a/packages/primitives/context-menu/src/context-menu-root.ts b/packages/primitives/context-menu/src/context-menu-root.ts index ab0c0f53e..d520b496d 100644 --- a/packages/primitives/context-menu/src/context-menu-root.ts +++ b/packages/primitives/context-menu/src/context-menu-root.ts @@ -1,7 +1,7 @@ import { Directive, inject, Signal } from '@angular/core'; import type { VirtualElement } from '@floating-ui/dom'; import { createContext } from '@radix-ng/primitives/core'; -import { RdxMenuAutoFocusInput, RdxMenuRoot } from '@radix-ng/primitives/menu'; +import { RdxMenuAutoFocusInput, RdxMenuOpenChangeReason, RdxMenuRoot } from '@radix-ng/primitives/menu'; import { RdxPopper } from '@radix-ng/primitives/popper'; export interface RdxContextMenuRootContext { @@ -10,9 +10,9 @@ export interface RdxContextMenuRootContext { /** Whether the whole menu is disabled. */ disabled: Signal; /** Open the menu anchored at the given viewport coordinates. */ - openAt: (clientX: number, clientY: number, autoFocus?: RdxMenuAutoFocusInput) => void; + openAt: (clientX: number, clientY: number, autoFocus?: RdxMenuAutoFocusInput, event?: Event) => void; /** Close the menu. */ - close: () => void; + close: (reason?: RdxMenuOpenChangeReason, event?: Event) => void; } export const [injectRdxContextMenuRootContext, provideRdxContextMenuRootContext] = @@ -23,8 +23,8 @@ const contextFactory = (): RdxContextMenuRootContext => { return { isOpen: root.menuRoot.open, disabled: root.menuRoot.disabled, - openAt: (clientX, clientY, autoFocus) => root.openAt(clientX, clientY, autoFocus), - close: () => root.menuRoot.close() + openAt: (clientX, clientY, autoFocus, event) => root.openAt(clientX, clientY, autoFocus, event), + close: (reason, event) => root.menuRoot.close(reason, event) }; }; @@ -52,13 +52,21 @@ export class RdxContextMenuRoot { readonly menuRoot = inject(RdxMenuRoot); private readonly popper = inject(RdxPopper); + constructor() { + // Tell the composed menu root it is a Context Menu, so its per-kind policy (modal focus trap, + // backdrop, outside-press grace) differs from a plain dropdown (Base UI `MenuParent.type`). + this.menuRoot.markAsContextMenu(); + } + /** * Open the menu with the popup anchored at the given viewport coordinates. * * `autoFocus` defaults to `'popup'` so a right-click opens with the popup focused but no item - * highlighted (matching Base UI's pointer behavior). Pass `'first'` for keyboard opening. + * highlighted (matching Base UI's pointer behavior). Pass `'first'` for keyboard opening. `event` is + * the originating pointer event (threaded to the menu so a touch long-press is recorded for the + * anchored scroll-lock policy, ADR 0016 §3). */ - openAt(clientX: number, clientY: number, autoFocus: RdxMenuAutoFocusInput = 'popup'): void { + openAt(clientX: number, clientY: number, autoFocus: RdxMenuAutoFocusInput = 'popup', event?: Event): void { if (this.menuRoot.disabled()) { return; } @@ -80,6 +88,6 @@ export class RdxContextMenuRoot { this.popper.anchorOverride.set(anchor); // Move focus into the popup so keyboard navigation and outside-dismiss work immediately. - this.menuRoot.show(autoFocus); + this.menuRoot.show(autoFocus, 'trigger-press', event); } } diff --git a/packages/primitives/context-menu/src/context-menu-trigger.ts b/packages/primitives/context-menu/src/context-menu-trigger.ts index c11dca31f..877c600fa 100644 --- a/packages/primitives/context-menu/src/context-menu-trigger.ts +++ b/packages/primitives/context-menu/src/context-menu-trigger.ts @@ -1,5 +1,6 @@ -import { booleanAttribute, DestroyRef, Directive, inject, input, numberAttribute } from '@angular/core'; +import { booleanAttribute, DestroyRef, Directive, ElementRef, inject, input, numberAttribute } from '@angular/core'; import { BooleanInput, NumberInput } from '@radix-ng/primitives/core'; +import { RdxMenuRoot } from '@radix-ng/primitives/menu'; import { injectRdxContextMenuRootContext } from './context-menu-root'; /** @@ -24,6 +25,8 @@ import { injectRdxContextMenuRootContext } from './context-menu-root'; }) export class RdxContextMenuTrigger { protected readonly rootContext = injectRdxContextMenuRootContext(); + private readonly menuRoot = inject(RdxMenuRoot); + private readonly elementRef = inject>(ElementRef); /** Whether the trigger is disabled. */ readonly disabled = input(false, { transform: booleanAttribute }); @@ -32,11 +35,33 @@ export class RdxContextMenuTrigger { readonly longPressDelay = input(500, { transform: numberAttribute }); private longPressTimer: ReturnType | undefined; + private allowMouseUpTimer: ReturnType | undefined; private longPressOrigin: { x: number; y: number } | undefined; private lastPointerDownTime = 0; + private allowMouseUp = false; + private readonly handleDocumentMouseUp = (event: MouseEvent): void => { + this.clearContextMenuMouseUpGuard(); + + if (!this.allowMouseUp) { + return; + } + + this.allowMouseUp = false; + const target = event.target as Node | null; + const popup = this.menuRoot.popupElement(); + + if (target && popup?.contains(target)) { + return; + } + + this.rootContext.close('cancel-open', event); + }; constructor() { - inject(DestroyRef).onDestroy(() => this.cancelLongPress()); + inject(DestroyRef).onDestroy(() => { + this.cancelLongPress(); + this.clearContextMenuMouseUpGuard(); + }); } protected onContextMenu(event: MouseEvent): void { @@ -52,7 +77,9 @@ export class RdxContextMenuTrigger { // pointerdown, so it opens with the first item highlighted; a pointer opens the popup // without highlighting an item. const fromKeyboard = event.timeStamp - this.lastPointerDownTime > 300; - this.rootContext.openAt(event.clientX, event.clientY, fromKeyboard ? 'first' : 'popup'); + // A right-click `contextmenu` event has no `pointerType`, so this records a non-touch open. + this.rootContext.openAt(event.clientX, event.clientY, fromKeyboard ? 'first' : 'popup', event); + this.armContextMenuMouseUpGuard(event.currentTarget as HTMLElement); } protected onPointerDown(event: PointerEvent): void { @@ -67,7 +94,8 @@ export class RdxContextMenuTrigger { this.cancelLongPress(); this.longPressTimer = setTimeout(() => { this.longPressTimer = undefined; - this.rootContext.openAt(clientX, clientY); + // Pass the touch pointer event so the menu records a touch open (ADR 0016 §3). + this.rootContext.openAt(clientX, clientY, 'popup', event); }, this.longPressDelay()); } @@ -89,4 +117,21 @@ export class RdxContextMenuTrigger { this.longPressTimer = undefined; this.longPressOrigin = undefined; } + + private armContextMenuMouseUpGuard(trigger: HTMLElement): void { + this.clearContextMenuMouseUpGuard(); + this.allowMouseUp = false; + this.allowMouseUpTimer = setTimeout(() => { + this.allowMouseUpTimer = undefined; + this.allowMouseUp = true; + }, 500); + + trigger.ownerDocument.addEventListener('mouseup', this.handleDocumentMouseUp, { once: true }); + } + + private clearContextMenuMouseUpGuard(): void { + clearTimeout(this.allowMouseUpTimer); + this.allowMouseUpTimer = undefined; + this.elementRef?.nativeElement?.ownerDocument?.removeEventListener('mouseup', this.handleDocumentMouseUp); + } } diff --git a/packages/primitives/context-menu/stories/context-menu-default.ts b/packages/primitives/context-menu/stories/context-menu-default.ts index 044b203c0..abfe8f27f 100644 --- a/packages/primitives/context-menu/stories/context-menu-default.ts +++ b/packages/primitives/context-menu/stories/context-menu-default.ts @@ -77,8 +77,8 @@ import { cn, demoMenu } from '../../storybook/styles';
-
People
+
People
-