From bba2da68252afbbd69f1b7fb13865a7a8cb97f05 Mon Sep 17 00:00:00 2001 From: xarmian Date: Mon, 14 Sep 2026 00:18:50 -0400 Subject: [PATCH] fix(web): the user menu's Sign out is inside the panel, not below its edge (BUG-2985) (#1355) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sign-out button was there the whole time. It was rendering 59px below the bottom of the panel that was supposed to contain it. Measured in a browser before touching anything, three viewport heights, identical at all of them: .user-dropdown height 393 .menu-panel max-height 340, clientH 338, overflow-y auto Sign out top 409, bottom 443 panel bottom 384 So the panel ends at 384 with a clean edge and no scrollbar — overlay scrollbars paint nothing until you scroll — and the menu LOOKS complete. "Connect a project…" is cut off too. Not viewport-dependent, not cloud-only: that measurement is a self-hosted instance with no Billing row, so cloud is worse. The cap stays, since it is what keeps a menu off a short viewport. What changes is that the panel stops being the SCROLLER for this one menu: `Menu` takes an opt-in `bodyScroll` that makes it a clipped flex column, and the dropdown becomes a scrolling body plus a pinned divider and Sign out. Shortening the menu would only move the cliff — Billing, Admin and the Resources block all come and go — so the account action is kept out of the scrolling region entirely. Every other consumer of the primitive is untouched; the flag defaults false. `min-height: 0` on both flex boxes is load-bearing: a flex item defaults to `min-height: auto` and refuses to shrink below its content, which would push the pinned row back out of the panel — the bug restored, with the markup looking right. The behavioural instrument is the e2e leg, and it is deliberately NOT a `toBeVisible()` check: an element scrolled out of an `overflow: auto` container still has a box, so the pre-fix menu would have PASSED one. It asserts the item's bottom edge against the panel's, and it goes red on a binary built from the unfixed tree — 442.6 vs 384.5, the same 58px. Claude-Session: https://claude.ai/code/session_019EmDd3Wy8b3cPG1TZBSzBu --- web/e2e/bug-2985-signout-reachable.spec.ts | 73 ++++++++++++ web/src/lib/components/common/Menu.svelte | 33 +++++- web/src/lib/components/layout/TopBar.svelte | 39 +++++++ .../layout/topBarSignOutPinned.test.ts | 107 ++++++++++++++++++ 4 files changed, 251 insertions(+), 1 deletion(-) create mode 100644 web/e2e/bug-2985-signout-reachable.spec.ts create mode 100644 web/src/lib/components/layout/topBarSignOutPinned.test.ts diff --git a/web/e2e/bug-2985-signout-reachable.spec.ts b/web/e2e/bug-2985-signout-reachable.spec.ts new file mode 100644 index 000000000..e5a38a701 --- /dev/null +++ b/web/e2e/bug-2985-signout-reachable.spec.ts @@ -0,0 +1,73 @@ +import { expect } from '@playwright/test'; +import { test } from './fixtures'; + +/** + * BUG-2985 — "Sign out" has to be INSIDE the panel, not merely in the DOM. + * + * The user menu is taller than the Menu primitive's 340px cap, which used to + * be the scroller. The panel's bottom edge then looked like the end of the + * menu: overlay scrollbars show nothing until you scroll, so there was no cue + * that "Connect a project…" and "Sign out" existed below it. Measured before + * the fix at 393px of content in a 338px client box, with Sign out laid out at + * y=409 while the box ended at 384 — at every viewport height tested, 900 down + * to 600, because the cap is fixed rather than viewport-relative. + * + * WHY `toBeVisible()` IS NOT THE ASSERTION, which is the whole reason this spec + * is geometric: an element scrolled out of an `overflow: auto` container still + * has a non-empty box and is not `display:none`, so Playwright calls it + * visible. The pre-fix menu would have PASSED a visibility check. What was + * wrong is where it sat relative to the panel, so that is what is asserted. + */ +test('BUG-2985: Sign out renders within the user menu panel, not below its edge', async ({ + page, + request, + fixture, +}) => { + const auth = { Authorization: `Bearer ${fixture.apiToken}` }; + const slug = `b2985-${test.info().workerIndex}-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 6)}`; + const wsResp = await request.post('/api/v1/workspaces', { + headers: auth, + data: { name: 'BUG-2985 probe', slug, template: 'startup' }, + }); + expect(wsResp.ok(), await wsResp.text()).toBeTruthy(); + const ws = (await wsResp.json()) as { slug: string }; + + await page.goto(`/${fixture.adminUsername}/${ws.slug}/tasks`); + + // SKIPPED BY PROJECT NAME, not by probing the DOM. The first version asked + // `trigger.count() === 0` immediately after `goto`, which races hydration: + // it read 0 on a page that renders the trigger a moment later and skipped + // BOTH projects, so the spec reported `2 skipped` with exit 0 — a green + // gate covering nothing. A skip condition has to be something the run + // KNOWS, not something it might be early to observe. + test.skip( + test.info().project.name.includes('mobile'), + 'desktop top bar only — mobile signs out through YouSheet, a different surface', + ); + + const trigger = page.locator('[aria-label="User menu"]'); + await expect(trigger).toBeVisible({ timeout: 15_000 }); + await trigger.click(); + + const panel = page.locator('.menu-panel').filter({ hasText: 'Sign out' }).first(); + await expect(panel).toBeVisible(); + const signOut = page.getByRole('menuitem', { name: 'Sign out' }); + await expect(signOut).toBeVisible(); + + const panelBox = await panel.boundingBox(); + const signOutBox = await signOut.boundingBox(); + expect(panelBox, 'no panel box').not.toBeNull(); + expect(signOutBox, 'no sign-out box').not.toBeNull(); + + // THE LEG: the item's bottom edge is inside the panel's. A 1px slack for + // sub-pixel layout; the pre-fix gap was 59px, so nothing about this is + // close to the tolerance. + expect(signOutBox!.y + signOutBox!.height).toBeLessThanOrEqual( + panelBox!.y + panelBox!.height + 1, + ); + + // And it WORKS from there — the point is a reachable control, not a + // well-placed one. Without this, a fix that pinned an inert row would pass. + await signOut.click(); + await page.waitForURL(/\/login(\?|$)/, { timeout: 15_000 }); +}); diff --git a/web/src/lib/components/common/Menu.svelte b/web/src/lib/components/common/Menu.svelte index e791fd71b..4354ab642 100644 --- a/web/src/lib/components/common/Menu.svelte +++ b/web/src/lib/components/common/Menu.svelte @@ -36,6 +36,21 @@ * Forwarded to BottomSheet in `sheetOnMobile` mode, which owns focus * there. PLAN-2326 DR-8. */ focusKey?: string | number; + /** + * Let the CONSUMER own scrolling instead of the panel (BUG-2985). + * + * The panel caps itself at 340px and scrolls, which silently truncates + * any menu taller than that: the panel's bottom edge looks like the end + * of the menu, and overlay scrollbars mean there is no cue at all. The + * user menu lost its "Sign out" that way — measured at 393px of content + * in a 338px client box, with Sign out rendering at y=409 and the box + * ending at 384. + * + * With this set, the panel keeps its cap but does NOT scroll; the + * consumer lays out a scrolling body and whatever must stay visible + * beside it. Opt-in, so every other menu keeps today's behaviour. + */ + bodyScroll?: boolean; /** Return true to suppress outside-close for this event (e.g. while * a drag interaction is in flight — cf. TopBar pill drags). */ suppressOutside?: () => boolean; @@ -55,6 +70,7 @@ exempt, suppressOutside, focusKey, + bodyScroll = false, children }: Props = $props(); @@ -193,7 +209,7 @@ {:else if open} {#if mode === 'portal'} `. The first version of this + // leg took `indexOf('', …'Connect a project')` as the body's close + // and SURVIVED the mutant that moves Sign out back inside — that index + // landed on the divider's own closing tag, which precedes Sign out in + // both layouts, so the comparison was true either way. The mutant is + // faithful; the instrument was not. + const bodyOpen = code.indexOf('class="user-dropdown-body"'); + expect(bodyOpen, 'no scrolling body').toBeGreaterThan(-1); + expect(matchingCloseIndex(code, bodyOpen), 'body never closes').toBeGreaterThan(-1); + const signOut = code.indexOf('Sign out'); + expect(signOut).toBeGreaterThan(bodyOpen); + expect(signOut, 'Sign out is inside the scrolling body').toBeGreaterThan( + matchingCloseIndex(code, bodyOpen), + ); + }); + + it('gives the body the min-height that lets it shrink', () => { + // `min-height: 0` is load-bearing: a flex item defaults to `min-height: + // auto` and refuses to shrink below its content, which would push the + // pinned row back out of the panel — the bug, restored, with the markup + // looking correct. + expect(code).toMatch(/\.user-dropdown-body\s*\{[^}]*min-height:\s*0/); + expect(code).toMatch(/\.user-dropdown\s*\{[^}]*min-height:\s*0/); + }); +}); + +describe('the Menu primitive', () => { + it('stops scrolling, and still CLIPS, under bodyScroll', () => { + // `overflow: hidden`, not `visible`: a too-tall menu must still be + // clipped to the panel, or it paints over the page with no way to reach + // the rest. + expect(menuSrc).toMatch(/\.menu-panel\.body-scroll\s*\{[^}]*overflow:\s*hidden/); + expect(menuSrc).toMatch(/\.menu-panel\.body-scroll\s*\{[^}]*display:\s*flex/); + }); + + it('leaves every other menu scrolling the panel', () => { + // The opt-in is the compatibility story: this primitive backs many menus + // and none of them was asked to change. + expect(menuSrc).toMatch(/\.menu-panel\s*\{[^}]*max-height:\s*340px/); + expect(menuSrc).toMatch(/\.menu-panel\s*\{[^}]*overflow-y:\s*auto/); + expect(menuSrc).toContain('bodyScroll = false'); + }); +});