From 8e42deafa1dab07e3338c981170a337cd6fd7f3a Mon Sep 17 00:00:00 2001 From: mektpoy Date: Thu, 24 Sep 2026 16:01:54 +0800 Subject: [PATCH] fix(client): keep the sidebar toggle's focus ring unclipped The sidebar toggle is flush with `.logoRow`, which clips its own overflow, so the standard outward ring lost every pixel that left the row: the reviewer saw the ring cut on the toggle in both the expanded header and the settled rail. The 36px rail toggle is also 1px wider than its 35px row, so the control's own right edge was clipped there. Draw this control's ring inside its own box and stop clipping the row once the rail has settled; the ring keeps the standard `--dsw-focus-ring-width` colour and width, and pointer modality still resolves `--dsw-focus-ring-color` to transparent. `focus-rings.e2e.ts` asserts the whole ring against every clipping ancestor in both layouts, in both themes. --- apps/web/tests/focus-rings.e2e.ts | 76 +++++++++++++++++++ .../src/client/SidebarRoot.module.css | 10 +++ 2 files changed, 86 insertions(+) diff --git a/apps/web/tests/focus-rings.e2e.ts b/apps/web/tests/focus-rings.e2e.ts index 39e23048c0..3f7d10b666 100644 --- a/apps/web/tests/focus-rings.e2e.ts +++ b/apps/web/tests/focus-rings.e2e.ts @@ -56,6 +56,42 @@ async function expectFocusRing(target: Locator, kind: 'outline' | 'shadow'): Pro }).toEqual({ active: true, visible: true, modality: 'keyboard', painted: true }) } +/** + * Name every ancestor that cuts the ring's painted box. A control flush with an + * `overflow: hidden` ancestor keeps its outline geometrically but loses the pixels that leave + * that ancestor, so compare the layout box grown by the ring against each clipping ancestor. + */ +async function ringCutBy(target: Locator): Promise { + return target.evaluate((element) => { + const style = getComputedStyle(element) + const width = Number.parseFloat(style.outlineWidth) || 0 + const offset = Number.parseFloat(style.outlineOffset) || 0 + const box = element.getBoundingClientRect() + const ring = { + left: box.left - width - offset, + top: box.top - width - offset, + right: box.right + width + offset, + bottom: box.bottom + width + offset, + } + const cuts: string[] = [] + for (let node = element.parentElement; node !== null; node = node.parentElement) { + const overflow = getComputedStyle(node) + if (overflow.overflow === 'visible' && overflow.overflowX === 'visible' && overflow.overflowY === 'visible') continue + const rect = node.getBoundingClientRect() + const cut = { + left: Math.max(0, rect.left - ring.left), + top: Math.max(0, rect.top - ring.top), + right: Math.max(0, ring.right - rect.right), + bottom: Math.max(0, ring.bottom - rect.bottom), + } + if (cut.left + cut.top + cut.right + cut.bottom > 0.5) { + cuts.push(`${node.tagName.toLowerCase()}${node.className === '' ? '' : `.${node.className}`} cuts ${JSON.stringify(cut)}`) + } + } + return cuts + }) +} + it.each(['light', 'dark'] as const)('assembled app (%s): pointer keys stay silent; navigation and menu activation retain feedback', async (theme) => { const scaffold = await launchWebScaffold() onTestFinished(() => scaffold.close()) @@ -116,6 +152,46 @@ it.each(['light', 'dark'] as const)('assembled app (%s): pointer keys stay silen expect(consoleWatch.pageErrors).toEqual([]) }) +it.each(['light', 'dark'] as const)('assembled app (%s): the sidebar toggle keeps its whole ring in the expanded and rail layouts', async (theme) => { + const scaffold = await launchWebScaffold() + onTestFinished(() => scaffold.close()) + const browser = await chromium.launch({ headless: true }) + onTestFinished(() => browser.close()) + const page = await newEnglishPage(browser) + await page.emulateMedia({ colorScheme: theme }) + await page.goto(scaffold.authenticatedUrl) + await connectFreshWorkspace(page, scaffold.workspaceCwd) + + const toggle = page.getByRole('button', { name: 'Collapse sidebar' }) + const rail = page.getByRole('button', { name: 'Open sidebar' }) + /** Sample the ring box once the collapse animation stops moving the control. */ + const settle = async (target: Locator, width: number): Promise => { + await expect.poll(() => target.evaluate(element => element.getBoundingClientRect().width)).toBe(width) + await target.evaluate(async (element) => { + for (let node: Element | null = element; node !== null; node = node.parentElement) { + await Promise.all(node.getAnimations().map(animation => animation.finished)) + } + }) + } + + await toggle.click() + await rail.waitFor() + // The rail only replaces the frozen wide layout once the collapse settles. + await settle(rail, 36) + await expectSilent(rail) + await page.keyboard.press('Home') + await expectFocusRing(rail, 'outline') + expect(await ringCutBy(rail)).toEqual([]) + + await rail.click() + await toggle.waitFor() + await settle(toggle, 28) + await expectSilent(toggle) + await page.keyboard.press('Home') + await expectFocusRing(toggle, 'outline') + expect(await ringCutBy(toggle)).toEqual([]) +}) + /** * Supplementary fixture, not an assembled app: Vite compiles the actual Menu, * Switch, input-modality module and CSS Modules in memory. Only arrangement and diff --git a/packages/client/ui-sidebar/src/client/SidebarRoot.module.css b/packages/client/ui-sidebar/src/client/SidebarRoot.module.css index 05a3d39597..2bfdc1509b 100644 --- a/packages/client/ui-sidebar/src/client/SidebarRoot.module.css +++ b/packages/client/ui-sidebar/src/client/SidebarRoot.module.css @@ -235,6 +235,9 @@ padding: 0; margin-bottom: 12px; justify-content: flex-start; + /* The settled rail's 36px toggle is wider than this 35px row; the base row's clip would cut + the control's right edge, so the rail stops clipping once the slide has settled. */ + overflow: visible; } /* Brand group (figma I133:7632): the full wordmark rides the text ink @@ -332,6 +335,13 @@ background: var(--dsw-alias-interactive-bg-hover); } +/* The toggle is flush with .logoRow, which clips its own overflow, so an outward ring loses + the pixels that leave the row. Draw this control's ring inside its own box instead. */ +.iconButton.toggle:focus-visible { + outline: var(--dsw-focus-ring-width) solid var(--dsw-focus-ring-color, var(--dsw-alias-state-business-primary)); + outline-offset: -2px; +} + .collapsed .iconButton { width: 36px; height: 36px;