From bd030124f7c020e9ebdb8134a48be4231e217090 Mon Sep 17 00:00:00 2001 From: xarmian Date: Sun, 26 Jul 2026 05:55:08 +0000 Subject: [PATCH 1/7] feat(web): slim the item header to three bands + the two-tier container-query strip (TASK-2329) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 3 of PLAN-2326, finishing IDEA-2299. Header measured 152px -> 127px on the tagless full-page case (194px before TASK-2328 landed) — IDEA-2299's target exactly. Three bands remain: breadcrumb, title, tab strip. Header slimming - Saved / Synced move out of `.meta-info` into `.strip-actions`, leading the group so the icon controls stay flush right. `.collab-state-synced` keeps its class and its visibility at every tier — it is SYNCED_BADGE_SELECTOR, asserted visible by four suites. - `.meta-info` is screen-hidden WHOLESALE, not span-by-span: a spans-only hide would strand the `.meta-sep` "·" and the band's bottom margin. - The `.item-ref` chip is screen-hidden; the breadcrumb tail carries identity on the full page and `.pane-header-ref-text` in the pane. Print (BUG-626) is byte-for-byte preserved - `.title-row .item-ref` had print styling but NO `display`, so the screen hide would have survived into print and dropped the ref from the page-1 document header. Added the explicit reset. `.meta-info` already reset to `display: block`. - Verified by rendering, not by reading CSS: `emulateMedia({media:'print'})` + PDF, before and after. Every element's print geometry is identical (display/order/w/h/x/y), and a pixel diff of the whole print rendering differs in exactly one 49x12 box — the relative-time string ("just now" -> "35m ago") that ticked over between the two runs. PDF page-1 text matches, ref included. Two-tier rule — `@container item-tab-strip (max-width: 699px)` - Repo's first container query. `@media` cannot see the docked pane: its width is dragged and persisted, decoupled from the viewport (DR-2). - Threshold 700px, NOT the mock's 900px, and confirmed by measurement rather than arithmetic (DR-10). The strip caps at 912px on the full page at ANY monitor width, so 900 would have made compact the near-universal state. Measured: full page 892-912 -> full; full page with a pane docked 526-678, docked pane 312-672, mobile 342 -> compact. - Compact drops the badge icons to bare counts, folds the star (`display: none`, node retained per DR-11), folds the collab label, and drops an idle save-status out of the flow — which triples the tab scrollport at the 360px pane floor (38px -> 94px) with no button movement. Folded in (both found in review, both caused by this change) - Star gets a row in the ⋯ menu. The compact tier folds the chip and the mobile overlay is ALWAYS compact, so without it starring became unreachable on phones. PLAN-2326's premise is zero affordances removed. - The collab label folds sr-only, not `display: none`. The dot is `aria-hidden`, so the label is the badge's only textual state; hiding it outright left a colour-only status indicator. - The backlinks badge gains an `aria-label`. Its accessible name was the computed "📎 4"; folding the icon would have degraded it to a bare "4". Tests - `pane-full-page-capstone.spec.ts` star assertions: `toBeEnabled()` requires attached, not visible, so a CSS fold would have left both green while asserting nothing — inside the test whose purpose is the BUG-2263 invisible-freeze guarantee (DR-11). Pre-peek (892px, full tier) is now `toBeVisible()`. While peeking the master is 526px and genuinely compact, so that one asserts what the freeze actually owns — node retained and enabled — and the reachability half moves to `.pane-more-btn`, which never folds and is now asserted visible. - New `pane-tab-strip-tiers.spec.ts`: the tier flips in place when a docked pane squeezes the full-page master at a FIXED viewport (a media query would sail through this), and the DR-9 worst case at the 360px pane floor — one row, all four tabs reachable through the scroll, ⋯ unclipped, and the folded star still operable from the menu. Mutation-tested: swapping `@container` for `@media` turns both red. Gates: `npm run check` 0 errors; `make check` exit 0 (Go tests, govulncheck clean, 490 web unit tests); 25/25 on the pane suites (tiers, capstone, host, link-anchors) against `make install`; full e2e 78 passed with one BUG-2334 toast-overlay flake (passes in isolation). Codex CLEAN. Claude-Session: https://claude.ai/code/session_01E2fRi12n8rARczvdEa2LYT --- web/e2e/pane-full-page-capstone.spec.ts | 27 +- web/e2e/pane-tab-strip-tiers.spec.ts | 266 ++++++++++++++++++ .../lib/components/items/ItemDetail.svelte | 244 +++++++++++++--- 3 files changed, 501 insertions(+), 36 deletions(-) create mode 100644 web/e2e/pane-tab-strip-tiers.spec.ts diff --git a/web/e2e/pane-full-page-capstone.spec.ts b/web/e2e/pane-full-page-capstone.spec.ts index 01ee8761..a32a2f46 100644 --- a/web/e2e/pane-full-page-capstone.spec.ts +++ b/web/e2e/pane-full-page-capstone.spec.ts @@ -306,11 +306,18 @@ test.describe('full-page pane host CAPSTONE (PLAN-2154 Phase 2 / TASK-2175)', () const masterEditor = col.locator(EDITOR_SELECTOR); await expect(masterEditor).toBeVisible({ timeout: SYNC_TIMEOUT }); await expect(masterEditor).toHaveAttribute('contenteditable', 'true'); - // Action bar (above the tabs): Star enabled; Quick-actions visible; + // Tab strip (PLAN-2326): Star VISIBLE; Quick-actions visible; // Share/Move/Delete live in the pane ⋯ overflow (PLAN-2290 Phase 4 PR B) — // open it (master is active; the menu is an activating control) and // verify the rows, then dismiss. - await expect(col.locator('button.star-btn')).toBeEnabled(); + // + // `toBeVisible`, not `toBeEnabled` (PLAN-2326 DR-11): the star now folds + // via CSS in the strip's compact tier with the node RETAINED, so + // `toBeEnabled` — which only requires *attached* — would pass on a folded + // star and assert nothing. No pane is open yet, so the master strip is + // 892px at this suite's 1200px viewport: the FULL tier, where the star is + // genuinely visible and this is the stronger claim it reads as. + await expect(col.locator('button.star-btn')).toBeVisible(); await expect(col.locator('button.trigger-btn[title="Quick actions"]')).toBeVisible(); await col.locator('button.pane-more-btn').click(); await expect(col.getByRole('menuitem', { name: 'Share…' })).toBeVisible(); @@ -357,11 +364,23 @@ test.describe('full-page pane host CAPSTONE (PLAN-2154 Phase 2 / TASK-2175)', () await expect(col.locator('.field-row', { hasText: 'Note' }).locator('.readonly-display')).toHaveCount(0); // Comment composer: still present (not peeking-gated — ItemTimeline frozen={false}). await expect(col.locator('.compose')).toHaveCount(1); - // Star: still enabled (never gated on peeking anymore). + // Star: still live while peeking (never gated on peeking anymore). + // + // Deliberately NOT `toBeVisible` here, unlike the pre-peek assertion + // above. With the pane docked, this master column's strip measures 526px + // at the suite's 1200px viewport, so PLAN-2326's compact tier CSS-folds + // the star. Visibility at this width is a TIER property, not a freeze + // property — asserting it would be testing the container query, and would + // go red the day the default pane width changes. What BUG-2263 owns is + // that peeking neither removes the node nor disables it; that is what is + // asserted. The reachability half of the guarantee is carried by the ⋯ + // trigger below, which never folds at any tier. + await expect(col.locator('button.star-btn')).toHaveCount(1); await expect(col.locator('button.star-btn')).toBeEnabled(); // Share/Move/Delete live in the ⋯ overflow now: the BUG-2263 guarantee is // that the trigger stays LIVE on the peeking side (opening it would - // activate this side, so assert enabled, not open). + // activate this side, so assert reachable-but-unopened, not open). + await expect(col.locator('button.pane-more-btn')).toBeVisible(); await expect(col.locator('button.pane-more-btn')).toBeEnabled(); // Add-relationship + per-link remove: still present (side-independent single-item REST — no freeze). await expect(col.locator('button.add-relationship-btn')).toHaveCount(1); diff --git a/web/e2e/pane-tab-strip-tiers.spec.ts b/web/e2e/pane-tab-strip-tiers.spec.ts new file mode 100644 index 00000000..f398fd31 --- /dev/null +++ b/web/e2e/pane-tab-strip-tiers.spec.ts @@ -0,0 +1,266 @@ +import { test, expect } from './fixtures'; +import { browserLogin, seedDoc, SYNCED_BADGE_SELECTOR } from './lib/collab-helpers'; +import type { APIRequestContext, Locator, Page } from '@playwright/test'; +import type { SuiteFixture } from './fixtures'; + +/** + * Tab-strip TIER rule (PLAN-2326 DR-7 / DR-9 / DR-10 / TASK-2329). + * + * The strip carries `container-type: inline-size` and a single + * `@container item-tab-strip (max-width: 699px)` rule: below 700px of STRIP + * width the badge icons drop to bare counts, the star folds away (node + * retained), the collab badge keeps its dot but drops its label, and an idle + * save-status leaves the flow. + * + * Two things are worth a real browser here, and neither is unit-testable: + * + * 1. The tier keys off the CONTAINER, not the viewport. This is the whole + * reason DR-2 chose `@container`: the docked pane's width is dragged by + * the user and persisted (`PaneHost.svelte` — `clamp(360px, 38%, 640px)` + * by CSS, `PANE_WIDTH_MIN 360` / `PANE_WIDTH_MAX 720` by JS), so it is + * decoupled from the viewport entirely. A `@media (min-width: 700px)` + * rule would sail through every assertion in test 1 — the viewport never + * changes size in it. Squeezing the FULL-PAGE master by opening a pane + * beside it drives the flip in place, at a fixed viewport, with no + * reload and no remount. + * + * 2. DR-9's width allocation actually holds at the 360px pane floor. Four + * tabs, two count badges, quick actions and the ⋯ overflow have to share + * a ~312px strip; `.strip-actions` is `flex: 0 0 auto` and `.pane-tabs` + * scrolls, so the contract is "one row, every tab still reachable" — not + * "everything fits". That is a layout property of the real box model. + * + * Measured strip widths this rule was calibrated against (TASK-2329): + * full page 892-912px (capped by `--content-max-width`, identical at 1440 + * and 2560) · full page with a pane docked 526-678px · docked pane 312-672px + * · mobile overlay 342px. Only the pane-free full page reaches the full tier. + * + * Desktop-split concern, so the desktop project alone covers it. + */ + +const DESKTOP = { width: 1200, height: 900 }; +const WIDE = { width: 1440, height: 1000 }; +const SYNC_TIMEOUT = 20_000; + +function authHeaders(fixture: SuiteFixture) { + return { Authorization: `Bearer ${fixture.apiToken}`, 'Content-Type': 'application/json' }; +} + +interface SeededItem { + id: string; + slug: string; + title: string; +} + +async function seedDocWith( + fixture: SuiteFixture, + request: APIRequestContext, + titlePrefix: string, + { content = '', parent = '' }: { content?: string; parent?: string } = {}, +): Promise { + const title = `${titlePrefix} ${Date.now()}${Math.floor(Math.random() * 1000)}`; + const resp = await request.post( + `/api/v1/workspaces/${fixture.workspaceSlug}/collections/docs/items`, + { + headers: authHeaders(fixture), + data: { title, fields: parent ? JSON.stringify({ parent }) : '{}', content }, + }, + ); + if (!resp.ok()) throw new Error(`doc create failed (${resp.status()}): ${await resp.text()}`); + return { ...((await resp.json()) as { id: string; slug: string }), title }; +} + +async function seedRelatedLink( + fixture: SuiteFixture, + request: APIRequestContext, + sourceSlug: string, + targetId: string, +): Promise { + const resp = await request.post( + `/api/v1/workspaces/${fixture.workspaceSlug}/items/${sourceSlug}/links`, + { headers: authHeaders(fixture), data: { target_id: targetId, link_type: 'related' } }, + ); + if (!resp.ok()) throw new Error(`link create failed (${resp.status()}): ${await resp.text()}`); +} + +/** The full-page master column — the `.item-page` the host renders directly. */ +function masterCol(page: Page): Locator { + return page.locator('.item-page-host > .item-page'); +} + +/** Rendered width of a strip, straight off the box model. */ +async function stripWidth(scope: Locator): Promise { + return Math.round( + await scope.locator('.tab-strip').evaluate((n) => n.getBoundingClientRect().width), + ); +} + +test.describe('tab-strip tiers (PLAN-2326 / TASK-2329)', () => { + test.beforeEach(({}, testInfo) => { + test.skip( + testInfo.project.name !== 'desktop-chromium', + 'the pane is a desktop-split concern; one project is enough', + ); + }); + + test('the tier keys off the STRIP CONTAINER, not the viewport: squeezing the full-page master with a docked pane flips it compact in place — and closing the pane flips it back (DR-2 / DR-10)', async ({ + page, + fixture, + request, + }) => { + await page.setViewportSize(DESKTOP); + await browserLogin(page); + + // Master with a CHILD (renders the 🌳 badge) plus a RELATED item to open + // in the pane. The child badge is what proves `.badge-icon` folds while + // `.badge-count` survives — a text-node split TASK-2328 made possible. + const master = await seedDocWith(fixture, request, 'Tier master'); + await seedDocWith(fixture, request, 'Tier child', { parent: master.id }); + const related = await seedDocWith(fixture, request, 'Tier related'); + await seedRelatedLink(fixture, request, master.slug, related.id); + + await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/docs/${master.slug}`); + + const col = masterCol(page); + const star = col.locator('button.star-btn'); + const badgeIcon = col.locator('.strip-actions .badge-icon'); + const badgeCount = col.locator('.strip-actions .badge-count'); + + // ── FULL tier. 892px of strip at this viewport, comfortably over 700. ── + await expect(col.locator('.tab-strip')).toBeVisible(); + await expect(badgeCount.first()).toBeVisible(); // children badge has arrived + expect(await stripWidth(col)).toBeGreaterThanOrEqual(700); + await expect(star).toBeVisible(); + await expect(badgeIcon.first()).toBeVisible(); + await expect(col.locator('.collab-state-label')).toBeVisible(); + + // ── Squeeze it. Open the pane from the Relationships tab — same route, + // same viewport, no reload: ONLY the master column's width changes. ── + await col.getByRole('tab', { name: 'Relationships' }).click(); + await col + .locator('.relationship-group', { hasText: 'Related' }) + .locator('a.link-target', { hasText: 'Tier related' }) + .click(); + await expect(page.locator('.item-pane')).toBeVisible(); + + // The viewport is byte-identical to what it was in the full tier above. + // A media query cannot distinguish these two states; a container query can. + expect(page.viewportSize()).toEqual(DESKTOP); + + // ── COMPACT tier, driven purely by the container shrinking. ── + await expect(star).toBeHidden(); + expect(await stripWidth(col)).toBeLessThan(700); + await expect(badgeIcon.first()).toBeHidden(); + // DR-11: the star FOLDS, it is not removed. The BUG-2263 freeze + // assertions in pane-full-page-capstone.spec.ts depend on the node. + await expect(star).toHaveCount(1); + // The count survives the icon — the point of DR-9's span split. + await expect(badgeCount.first()).toBeVisible(); + // The collab badge keeps its class AND its box (SYNCED_BADGE_SELECTOR is + // asserted visible by four other suites); only the label word folds — + // and VISUALLY only. The dot is `aria-hidden`, so the label is the + // badge's only textual state; it folds sr-only (clipped to a 1px box, + // still in the accessibility tree) rather than `display: none`, which + // would leave a colour-only status indicator. + await expect(col.locator(SYNCED_BADGE_SELECTOR)).toBeVisible({ timeout: SYNC_TIMEOUT }); + const foldedLabel = col.locator('.collab-state-label'); + expect(Math.round((await foldedLabel.boundingBox())?.width ?? -1)).toBeLessThanOrEqual(1); + await expect(foldedLabel).toHaveText('Synced'); + // ⋯ never folds at any tier — it is the compact tier's escape hatch, and + // it carries the Star the strip just folded away. + await expect(col.locator('button.pane-more-btn')).toBeVisible(); + await col.locator('button.pane-more-btn').click(); + await expect(col.getByRole('menuitem', { name: /^(Star|Unstar)$/ })).toBeVisible(); + await page.keyboard.press('Escape'); + + // ── Close the pane → the container widens again and the tier reverts. + // A one-way rule would pass every assertion above. ── + await page.locator('.item-pane').getByRole('button', { name: 'Close pane' }).click(); + await expect(page.locator('.item-pane')).toHaveCount(0); + expect(await stripWidth(col)).toBeGreaterThanOrEqual(700); + await expect(star).toBeVisible(); + await expect(badgeIcon.first()).toBeVisible(); + }); + + test('worst case at the 360px pane floor — children + backlinks + quick actions + owner rights: the strip stays ONE row and every tab is still reachable through the scroll (DR-9)', async ({ + page, + fixture, + request, + }) => { + await page.setViewportSize(WIDE); + await browserLogin(page); + + // Every action the strip can hold at once: a child badge, a backlink + // badge, the owner quick-actions trigger (the fixture user owns the + // workspace, so it renders even with no seeded prompt actions), and ⋯. + const master = await seedDocWith(fixture, request, 'Tier worst'); + await seedDocWith(fixture, request, 'Tier worst child', { parent: master.id }); + await seedDocWith(fixture, request, 'Tier worst mention', { + content: `mentions [[${master.title}]] here`, + }); + + await page.goto( + `/${fixture.adminUsername}/${fixture.workspaceSlug}/docs?item=${master.slug}`, + ); + const pane = page.locator('.item-pane'); + await expect(pane.locator('.tab-strip')).toBeVisible(); + + // Drag the divider hard right — `clampPaneWidth` pins it at the 360px + // floor, the narrowest the pane can ever be on a roomy container. + const divider = page.locator('.pane-divider'); + const box = await divider.boundingBox(); + if (!box) throw new Error('pane divider has no box'); + await page.mouse.move(box.x + box.width / 2, box.y + 200); + await page.mouse.down(); + await page.mouse.move(box.x + box.width / 2 + 600, box.y + 200, { steps: 20 }); + await page.mouse.up(); + await expect + .poll(async () => Math.round(await pane.evaluate((n) => n.getBoundingClientRect().width))) + .toBe(360); + + // Both badges present (the strip is at its most crowded). + await expect(pane.locator('.strip-actions .badge-count')).toHaveCount(2); + await expect(pane.locator('button.trigger-btn[title="Quick actions"]')).toBeVisible(); + await expect(pane.locator('button.pane-more-btn')).toBeVisible(); + // Compact tier throughout. + await expect(pane.locator('button.star-btn')).toBeHidden(); + await expect(pane.locator('button.star-btn')).toHaveCount(1); + await expect(pane.locator('.strip-actions .badge-icon').first()).toBeHidden(); + + // ── ONE ROW. `.strip-actions` holds its size and `.pane-tabs` scrolls + // rather than wrapping, so all four tabs share a single offsetTop. ── + const tabTops = await pane.locator('.pane-tabs').evaluate((n) => { + const tabs = Array.from(n.querySelectorAll('[role="tab"]')); + return Array.from(new Set(tabs.map((t) => Math.round(t.getBoundingClientRect().top)))); + }); + expect(tabTops).toHaveLength(1); + + // ── EVERY TAB REACHABLE. The tablist is a scrollport narrower than its + // content; each tab must scroll into it and take the selection. ── + const tabs = pane.getByRole('tab'); + await expect(tabs).toHaveCount(4); + for (const name of ['Details', 'Relationships', 'Activity', 'Versions']) { + const tab = pane.getByRole('tab', { name }); + await tab.scrollIntoViewIfNeeded(); + await expect(tab).toBeInViewport(); + await tab.click(); + await expect(tab).toHaveAttribute('aria-selected', 'true'); + } + + // The ⋯ panel is anchored inside `.strip-actions`, a sibling of the + // scrolling tablist. Nothing on `.tab-strip` clips it — proving the + // "no overflow/contain on the container" constraint still holds at the + // narrowest width, where a clip would be most visible. + await pane.locator('button.pane-more-btn').click(); + await expect(pane.getByRole('menuitem', { name: 'Move to collection…' })).toBeVisible(); + + // ── The folded star's escape hatch actually WORKS here. This is the + // width the mobile overlay always runs at, so "Star lives in ⋯" has + // to be a working action, not just a rendered row. ── + await pane.getByRole('menuitem', { name: 'Star' }).click(); + await expect(pane.getByRole('menuitem', { name: 'Star' })).toHaveCount(0); // menu closed + await pane.locator('button.pane-more-btn').click(); + await expect(pane.getByRole('menuitem', { name: 'Unstar' })).toBeVisible(); + await page.keyboard.press('Escape'); + }); +}); diff --git a/web/src/lib/components/items/ItemDetail.svelte b/web/src/lib/components/items/ItemDetail.svelte index b60e4df6..52ccd7c5 100644 --- a/web/src/lib/components/items/ItemDetail.svelte +++ b/web/src/lib/components/items/ItemDetail.svelte @@ -4328,29 +4328,22 @@ {/if} - +
Created {relativeTime(item.created_at)} by {item.created_by || 'unknown'} · Updated {relativeTime(item.updated_at)} - - {#if saveStatus === 'saving'}Saving...{:else}✓ Saved{/if} - - {#if collabProvider} - - - - {collabStateLabel(collabProvider.state)} - - {/if}
+ + + {#if saveStatus === 'saving'}Saving...{:else}✓ Saved{/if} + + {#if collabProvider} + + + + {collabStateLabel(collabProvider.state)} + + {/if} @@ -4536,14 +4560,19 @@ sync without a separate API call. Split into icon + count spans for TASK-2329's compact tier, - exactly as the children badge above. This badge has NO - `aria-label`, so its accessible name is computed from the - content — hence no `aria-hidden` on the icon and the literal - space between the spans: both would change the name. + exactly as the children badge above. + + TASK-2328 left this badge without an `aria-label` so its + accessible name stayed the computed "📎 4". TASK-2329's + compact tier `display:none`s `.badge-icon`, which drops it + out of the accessibility tree too and would degrade that + name to a bare "4". An explicit label — mirroring the + children badge's — pins the name across BOTH tiers instead. -->
{/if} - +
Created {relativeTime(item.created_at)} by {item.created_by || 'unknown'} · Updated {relativeTime(item.updated_at)} + + {#if saveStatus === 'saving'}Saving...{:else}✓ Saved{/if} + + {#if collabProvider} + + + + {collabStateLabel(collabProvider.state)} + + {/if}
- - - {#if saveStatus === 'saving'}Saving...{:else}✓ Saved{/if} - - {#if collabProvider} - - - - {collabStateLabel(collabProvider.state)} - - {/if} @@ -5673,16 +5649,6 @@ min-width: 0; } - /* Saved / Synced were relocated here out of `.meta-info` (TASK-2329). They - each carried their own `margin-left` for spacing inside that band; the - group's `gap` owns spacing now, so the margin would just double it. - Overridden under `.strip-actions` rather than removed from the base - rules so the badges keep their standalone styling. */ - .strip-actions .save-status, - .strip-actions .collab-state { - margin-left: 0; - } - /* ── Compact tier (PLAN-2326 DR-7 / DR-9 / DR-10) ────────────────────── `@container`, NOT `@media`: the docked pane is `flex: 0 0 var(--pane-width, clamp(360px, 38%, 640px))` — a width the @@ -5721,11 +5687,7 @@ required duplicate badge markup outside this subtree. The star folds by `display: none` with the node RETAINED (DR-11) — - `.star-btn` must stay in the DOM for the BUG-2263 freeze assertions. - `.collab-state` itself never folds (it is `SYNCED_BADGE_SELECTOR`, - asserted visible inside narrow panes); only its label word folds, and - VISUALLY only — see the sr-only note on that rule. The coloured dot, - the primary signal, always renders. */ + `.star-btn` must stay in the DOM for the BUG-2263 freeze assertions. */ @container item-tab-strip (max-width: 699px) { .badge-icon { display: none; @@ -5733,39 +5695,6 @@ .star-btn { display: none; } - /* VISUALLY hidden, not `display: none` (Codex round 2). The dot is - `aria-hidden`, so this label is the badge's ONLY textual state — - `display: none` would drop it out of the accessibility tree and - leave a colour-only indicator, which is exactly the failure mode - the label exists to prevent. The clip-rect idiom takes it out of - the visual flow (0 layout width, no paint) while keeping it - readable by assistive tech. `clip-path` here is on a LEAF span — - not on `.tab-strip` or any ancestor of `.menu-anchor`, where it - would clip the anchored panels. */ - .collab-state-label { - position: absolute; - width: 1px; - height: 1px; - padding: 0; - margin: -1px; - overflow: hidden; - clip: rect(0 0 0 0); - clip-path: inset(50%); - white-space: nowrap; - border: 0; - } - /* `.save-status` sits at `opacity: 0` when idle, which still RESERVES - its ~48px. Harmless in the 912px full tier; in a 360px pane that is - 18% of the strip taken by something invisible, and every px here - comes straight out of the tab list's scrollport (38px -> 94px with - this rule). Dropping it from the flow costs no button movement: the - group is right-aligned and `.save-status` LEADS it, so a save - extends the group's left edge and nothing to its right shifts. The - only casualty is the 0.2s fade-OUT at the end of the 2s `saved` - window (`display: none` snaps); the fade-IN still plays. */ - .save-status:not(.visible) { - display: none; - } } .pane-tab { @@ -6130,20 +6059,15 @@ line-height: 1; } - /* Meta — the created/updated provenance line. - - SCREEN-HIDDEN WHOLESALE (TASK-2329 / DR-5). Hiding the band rather than - its spans is deliberate: a spans-only hide would leave the `.meta-sep` - "·" orphaned on its own row plus the band's bottom margin. Activity and - Versions are the tabs that own provenance now. - - The element stays in the DOM because print needs it: the `@media print` - block restores `display: block` and re-declares every property it cares - about there (font-size, colour, margins, and the `border-bottom` that - divides the page-1 document header from the properties card — BUG-626), - so none of the old screen-side flex properties are missed. */ + /* Meta */ .meta-info { - display: none; + display: flex; + align-items: center; + gap: var(--space-2); + font-size: 0.8em; + color: var(--text-muted); + margin-bottom: var(--space-2); + flex-wrap: wrap; } .meta-sep { color: var(--text-muted); } .save-status { @@ -6913,14 +6837,7 @@ /* Meta info subtitle (created / updated by). Border-bottom separates the page-1 document header block from the - properties card below. BUG-626. - - `display: block` was already here before TASK-2329 and is what - makes the band's wholesale SCREEN hide safe — it un-hides the - provenance line for print. Do not drop it. Saved / Synced used to - live inside this band and moved to `.strip-actions`; they were - already print-hidden by the global selectors above, so the printed - text is unchanged. */ + properties card below. BUG-626. */ .meta-info { font-size: 9pt; color: #555; From e97d74253c81e08a3ea42ff12ad7bdb038861423 Mon Sep 17 00:00:00 2001 From: xarmian Date: Sun, 26 Jul 2026 06:41:54 +0000 Subject: [PATCH 3/7] fix(web): un-combine the tab strip below 560px so four tabs fit on a phone (TASK-2329) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Combining the tablist and the action group on one row is not survivable at phone widths, and folding does not fix it. The four tab labels are a fixed 321px; even a hypothetical action group of nothing but `⋯` (38px) needs 359px against the 342px a 390px phone has. Measured on the WIDEST common phone, 430px, before this change: 382px of strip, 179px of tab scrollport, TWO of four tabs clipped — under `scrollbar-width: none`, so with no hint they existed at all. Below 560px of strip, `.strip-actions` now takes a row of its own and the tablist gets the full width. Re-keyed from one `<700` fold to three rules: < 560 split — actions own a row, ABOVE the tabs 560 - 699 fold — one row, badge icons -> bare counts, star folds <= 330 tab padding 11px -> 8px (the last 24px needed at 312px) Consequence worth stating: below 560 the star and the badge icons come BACK. The previous single `<700` fold hid them on every phone and in the default-width docked pane — the widths with the LEAST need to economise once the actions have their own row. Actions go ABOVE the tabs (`order: -1`, visual only — DOM order and the tablist's roving tabindex are untouched). `.tab-strip` owns the bottom divider and `.pane-tab.on` overlaps it by 1px via `margin-bottom: -1px`; keeping the tablist as the LAST row preserves that. Measured identical (offset 0) in both the shared-row and split shapes. Actions-below would orphan the active-tab underline ~31px above the divider. `flex-wrap` / `column-gap` / `row-gap` are in the BASE `.tab-strip` rule, not the `@container` block. A container query cannot style its own query container and `.tab-strip` IS `item-tab-strip`, so a `flex-wrap: wrap` written in there is silently dropped. Verified by mutation rather than asserted: moving it inside leaves `nowrap` in force while `flex: 0 0 100%` still applies to `.strip-actions`, and `.pane-tabs` collapses to **0px** with all four tabs clipped at every phone width. Both new tests go red on it. The `<= 330` rule likewise MUST sit after the base `.pane-tab` rule — a container query adds no specificity, so the selectors tie and source order decides. Caught by measurement: written above the base rule the tabs kept `scrollWidth: 321` inside a 312px strip and one tab stayed clipped. MEASURED (worst-case item: both badges + quick actions + owner) desktop, must be unchanged — and is, to the pixel: 1440 / 1200 one row, header 148.8px, tabs fit, underline offset 0 1024 / 768 one row, header 174.8px, tabs fit, underline offset 0 phone full page + overlay pane, tabs FIT where two were clipped before: 360 strip 312, tabs 297/297 (was 321 needed / 109 shown, 3 clipped) 390 strip 342, tabs 321/321 (was 321 / 139, 3 clipped) 412 strip 364, tabs 321/321 (was 321 / 161, 3 clipped) 430 strip 382, tabs 321/321 (was 321 / 179, 2 clipped) docked pane 360 / 448 / 600 -> split; 720 (PANE_WIDTH_MAX) -> fold band container proof: a 360px pane on a 1920px viewport splits while the full page at that same viewport stays on one row — no media query can do that Cost: the strip is 33px -> 67px on phones, so the header grows ~34px there. That is the trade for four reachable tabs. Also verified: `.menu-anchor` gained no `overflow`/`contain`/`transform`/ `filter`/`clip-path`/`will-change` ancestor at any width (audited by walking the computed styles), the desktop ⋯ panel and the 430/360 bottom sheets own all four of their corners by hit-test, and the arrow-key roving walk still visits exactly the four tabs in a 360px pane. pane-tab-strip-tiers.spec.ts rewritten: its old "strip stays ONE row" assertion contradicted this design. Now three tests — the container-driven split at a fixed viewport, the 360px worst case asserting tabs FIT rather than "reachable by scrolling", and the 560-699 fold band. Gates: `npm run check` 0 errors; `make check` exit 0 (Go tests, govulncheck clean, 490 web unit tests); tier spec 3/3; full e2e 79 passed with one cross-spec flake per run (pane-collab-teardown:137 then pane-content-link-anchors:238 — different test each run, both green in isolation; BUG-2334's shared-instance interference). Claude-Session: https://claude.ai/code/session_01E2fRi12n8rARczvdEa2LYT --- web/e2e/pane-tab-strip-tiers.spec.ts | 250 ++++++++++++------ .../lib/components/items/ItemDetail.svelte | 179 ++++++++----- 2 files changed, 288 insertions(+), 141 deletions(-) diff --git a/web/e2e/pane-tab-strip-tiers.spec.ts b/web/e2e/pane-tab-strip-tiers.spec.ts index 5cc3ec88..fa8a61de 100644 --- a/web/e2e/pane-tab-strip-tiers.spec.ts +++ b/web/e2e/pane-tab-strip-tiers.spec.ts @@ -1,38 +1,44 @@ import { test, expect } from './fixtures'; -import { browserLogin, seedDoc, SYNCED_BADGE_SELECTOR } from './lib/collab-helpers'; +import { browserLogin, SYNCED_BADGE_SELECTOR } from './lib/collab-helpers'; import type { APIRequestContext, Locator, Page } from '@playwright/test'; import type { SuiteFixture } from './fixtures'; /** - * Tab-strip TIER rule (PLAN-2326 DR-7 / DR-9 / DR-10 / TASK-2329). + * Tab-strip TIER rules (PLAN-2326 / TASK-2329). * - * The strip carries `container-type: inline-size` and a single - * `@container item-tab-strip (max-width: 699px)` rule: below 700px of STRIP - * width the badge icons drop to bare counts and the star folds away, node - * retained. + * `.tab-strip` is `container-type: inline-size` and carries three rules keyed + * on its OWN width, not the viewport's: * - * Two things are worth a real browser here, and neither is unit-testable: + * < 560px SPLIT — `.strip-actions` takes a full row of its own, `order: + * -1` so it sits ABOVE the tablist, and the tabs get the full + * width. Everything the shared row folded comes back. + * 560-699 FOLD — still one row, but tight: badge icons drop to bare + * counts and the star folds (node retained, DR-11). + * <= 330px the tabs' horizontal padding trims 11px -> 8px, the last 24px + * needed to fit four tabs in a 312px strip. * - * 1. The tier keys off the CONTAINER, not the viewport. This is the whole - * reason DR-2 chose `@container`: the docked pane's width is dragged by - * the user and persisted (`PaneHost.svelte` — `clamp(360px, 38%, 640px)` - * by CSS, `PANE_WIDTH_MIN 360` / `PANE_WIDTH_MAX 720` by JS), so it is - * decoupled from the viewport entirely. A `@media (min-width: 700px)` - * rule would sail through every assertion in test 1 — the viewport never - * changes size in it. Squeezing the FULL-PAGE master by opening a pane - * beside it drives the flip in place, at a fixed viewport, with no - * reload and no remount. + * Why the split exists at all: combining tabs and actions on one row is not + * survivable at phone widths and no amount of folding fixes it. The four tab + * labels are a fixed 321px, and even a hypothetical action group of nothing + * but `⋯` (38px) needs 359px against the 342px a 390px phone has. Measured on + * the WIDEST common phone before this change, 430px: 382px of strip, 179px of + * tab scrollport, two of four tabs clipped — under `scrollbar-width: none`, + * so with no hint they existed. * - * 2. DR-9's width allocation actually holds at the 360px pane floor. Four - * tabs, two count badges, quick actions and the ⋯ overflow have to share - * a ~312px strip; `.strip-actions` is `flex: 0 0 auto` and `.pane-tabs` - * scrolls, so the contract is "one row, every tab still reachable" — not - * "everything fits". That is a layout property of the real box model. + * Three things here need a real browser: * - * Measured strip widths this rule was calibrated against (TASK-2329): - * full page 892-912px (capped by `--content-max-width`, identical at 1440 - * and 2560) · full page with a pane docked 526-678px · docked pane 312-672px - * · mobile overlay 342px. Only the pane-free full page reaches the full tier. + * 1. The rules key off the CONTAINER. The docked pane's width is dragged by + * the user and persisted (`PaneHost.svelte`: `PANE_WIDTH_MIN 360` / + * `PANE_WIDTH_MAX 720`), so it is decoupled from the viewport entirely, + * and the full page's own strip is capped at 912px by + * `--content-max-width` at ANY monitor size. Test 1 drives the split at + * a FIXED viewport by squeezing the full-page master with a docked pane; + * a `@media` rule would sail through every assertion in it. + * 2. Whether four tabs actually fit once they own a row. That is a box-model + * fact, and the ≤330px padding trim is what makes it true at 360. + * 3. That the split did not break the active tab's 1px overlap of the + * strip's bottom divider — the reason the actions go ABOVE rather than + * below. * * Desktop-split concern, so the desktop project alone covers it. */ @@ -87,13 +93,49 @@ function masterCol(page: Page): Locator { return page.locator('.item-page-host > .item-page'); } -/** Rendered width of a strip, straight off the box model. */ +/** Rendered width of the strip inside `scope`, straight off the box model. */ async function stripWidth(scope: Locator): Promise { return Math.round( await scope.locator('.tab-strip').evaluate((n) => n.getBoundingClientRect().width), ); } +interface StripLayout { + /** true when `.strip-actions` sits on its own row above the tablist. */ + split: boolean; + /** every tab shares one offsetTop — the tablist itself never wraps. */ + tabsOnOneRow: boolean; + /** the tablist is not overflowing: all four labels are actually shown. */ + tabsFit: boolean; + /** active tab's border-box bottom minus the strip's — 0 means it covers + * the 1px divider exactly, which is the designed overlap. */ + underlineOffset: number; + /** the action group must never become a child of the tablist (DR-4). */ + tablistContainsActions: boolean; +} + +async function stripLayout(scope: Locator): Promise { + return scope.locator('.tab-strip').evaluate((strip) => { + const tabs = strip.querySelector('.pane-tabs') as HTMLElement; + const acts = strip.querySelector('.strip-actions') as HTMLElement; + const tabEls = Array.from(tabs.querySelectorAll('[role="tab"]')); + const active = tabs.querySelector('.pane-tab.on'); + const sr = strip.getBoundingClientRect(); + const tr = tabs.getBoundingClientRect(); + const ar = acts.getBoundingClientRect(); + return { + split: ar.bottom <= tr.top + 1, + tabsOnOneRow: + new Set(tabEls.map((t) => Math.round(t.getBoundingClientRect().top))).size === 1, + tabsFit: tabs.scrollWidth <= tabs.clientWidth, + underlineOffset: active + ? Math.round(active.getBoundingClientRect().bottom - sr.bottom) + : NaN, + tablistContainsActions: tabs.contains(acts), + }; + }); +} + test.describe('tab-strip tiers (PLAN-2326 / TASK-2329)', () => { test.beforeEach(({}, testInfo) => { test.skip( @@ -102,7 +144,7 @@ test.describe('tab-strip tiers (PLAN-2326 / TASK-2329)', () => { ); }); - test('the tier keys off the STRIP CONTAINER, not the viewport: squeezing the full-page master with a docked pane flips it compact in place — and closing the pane flips it back (DR-2 / DR-10)', async ({ + test('the SPLIT keys off the strip CONTAINER, not the viewport: docking a pane beside the full-page master splits its strip in place at a fixed viewport, and closing the pane rejoins it', async ({ page, fixture, request, @@ -111,8 +153,7 @@ test.describe('tab-strip tiers (PLAN-2326 / TASK-2329)', () => { await browserLogin(page); // Master with a CHILD (renders the 🌳 badge) plus a RELATED item to open - // in the pane. The child badge is what proves `.badge-icon` folds while - // `.badge-count` survives — a text-node split TASK-2328 made possible. + // in the pane. const master = await seedDocWith(fixture, request, 'Tier master'); await seedDocWith(fixture, request, 'Tier child', { parent: master.id }); const related = await seedDocWith(fixture, request, 'Tier related'); @@ -125,10 +166,15 @@ test.describe('tab-strip tiers (PLAN-2326 / TASK-2329)', () => { const badgeIcon = col.locator('.strip-actions .badge-icon'); const badgeCount = col.locator('.strip-actions .badge-count'); - // ── FULL tier. 892px of strip at this viewport, comfortably over 700. ── + // ── SHARED ROW. 892px of strip at this viewport, way over 699. ── await expect(col.locator('.tab-strip')).toBeVisible(); await expect(badgeCount.first()).toBeVisible(); // children badge has arrived expect(await stripWidth(col)).toBeGreaterThanOrEqual(700); + let layout = await stripLayout(col); + expect(layout.split).toBe(false); + expect(layout.tabsFit).toBe(true); + expect(layout.tablistContainsActions).toBe(false); + const sharedRowUnderline = layout.underlineOffset; await expect(star).toBeVisible(); await expect(badgeIcon.first()).toBeVisible(); @@ -141,41 +187,41 @@ test.describe('tab-strip tiers (PLAN-2326 / TASK-2329)', () => { .click(); await expect(page.locator('.item-pane')).toBeVisible(); - // The viewport is byte-identical to what it was in the full tier above. + // The viewport is byte-identical to what it was on the shared row above. // A media query cannot distinguish these two states; a container query can. expect(page.viewportSize()).toEqual(DESKTOP); - // ── COMPACT tier, driven purely by the container shrinking. ── - await expect(star).toBeHidden(); - expect(await stripWidth(col)).toBeLessThan(700); - await expect(badgeIcon.first()).toBeHidden(); - // DR-11: the star FOLDS, it is not removed. The BUG-2263 freeze - // assertions in pane-full-page-capstone.spec.ts depend on the node. - await expect(star).toHaveCount(1); - // The count survives the icon — the point of DR-9's span split. - await expect(badgeCount.first()).toBeVisible(); - // The collab badge is untouched by the tier rule — it lives in - // `.meta-info`, outside the query container entirely. Asserted here - // only because SYNCED_BADGE_SELECTOR is load-bearing for four other - // suites and this is the narrowest surface any of them exercise. + // ── SPLIT, driven purely by the container shrinking to ~526px. ── + await expect + .poll(async () => (await stripLayout(col)).split, { timeout: 5000 }) + .toBe(true); + expect(await stripWidth(col)).toBeLessThan(560); + layout = await stripLayout(col); + expect(layout.tabsOnOneRow).toBe(true); + expect(layout.tabsFit).toBe(true); + expect(layout.tablistContainsActions).toBe(false); + // Actions ABOVE, not below: the strip owns the bottom divider and the + // active tab overlaps it by 1px. Keeping the tablist last preserves that, + // so the underline offset is unchanged from the shared row. + expect(layout.underlineOffset).toBe(sharedRowUnderline); + // Below 560 the actions have room again, so what the FOLD band hides is + // visible here. This is the behaviour a single <700 fold got wrong. + await expect(star).toBeVisible(); + await expect(badgeIcon.first()).toBeVisible(); + // The collab badge lives in `.meta-info`, outside the query container — + // asserted only because SYNCED_BADGE_SELECTOR is load-bearing for four + // other suites and this is the narrowest surface any of them exercises. await expect(col.locator(SYNCED_BADGE_SELECTOR)).toBeVisible({ timeout: SYNC_TIMEOUT }); - // ⋯ never folds at any tier — it is the compact tier's escape hatch, and - // it carries the Star the strip just folded away. - await expect(col.locator('button.pane-more-btn')).toBeVisible(); - await col.locator('button.pane-more-btn').click(); - await expect(col.getByRole('menuitem', { name: /^(Star|Unstar)$/ })).toBeVisible(); - await page.keyboard.press('Escape'); - // ── Close the pane → the container widens again and the tier reverts. - // A one-way rule would pass every assertion above. ── + // ── Close the pane → the container widens and the rows rejoin. A + // one-way rule would pass every assertion above. ── await page.locator('.item-pane').getByRole('button', { name: 'Close pane' }).click(); await expect(page.locator('.item-pane')).toHaveCount(0); + await expect.poll(async () => (await stripLayout(col)).split, { timeout: 5000 }).toBe(false); expect(await stripWidth(col)).toBeGreaterThanOrEqual(700); - await expect(star).toBeVisible(); - await expect(badgeIcon.first()).toBeVisible(); }); - test('worst case at the 360px pane floor — children + backlinks + quick actions + owner rights: the strip stays ONE row and every tab is still reachable through the scroll (DR-9)', async ({ + test('worst case at the 360px pane floor — children + backlinks + quick actions + owner rights: actions take their own row and all four tabs FIT without scrolling', async ({ page, fixture, request, @@ -192,9 +238,7 @@ test.describe('tab-strip tiers (PLAN-2326 / TASK-2329)', () => { content: `mentions [[${master.title}]] here`, }); - await page.goto( - `/${fixture.adminUsername}/${fixture.workspaceSlug}/docs?item=${master.slug}`, - ); + await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/docs?item=${master.slug}`); const pane = page.locator('.item-pane'); await expect(pane.locator('.tab-strip')).toBeVisible(); @@ -211,47 +255,91 @@ test.describe('tab-strip tiers (PLAN-2326 / TASK-2329)', () => { .poll(async () => Math.round(await pane.evaluate((n) => n.getBoundingClientRect().width))) .toBe(360); - // Both badges present (the strip is at its most crowded). + // The strip is at its most crowded: both badges + quick actions + ⋯. await expect(pane.locator('.strip-actions .badge-count')).toHaveCount(2); await expect(pane.locator('button.trigger-btn[title="Quick actions"]')).toBeVisible(); await expect(pane.locator('button.pane-more-btn')).toBeVisible(); - // Compact tier throughout. - await expect(pane.locator('button.star-btn')).toBeHidden(); - await expect(pane.locator('button.star-btn')).toHaveCount(1); - await expect(pane.locator('.strip-actions .badge-icon').first()).toBeHidden(); + // And the star is VISIBLE here, not folded — below 560 the actions own a + // row, so there is room for it. (A single <700 fold hid it on every + // phone; that was the bug this keying fixes.) + await expect(pane.locator('button.star-btn')).toBeVisible(); + await expect(pane.locator('.strip-actions .badge-icon').first()).toBeVisible(); - // ── ONE ROW. `.strip-actions` holds its size and `.pane-tabs` scrolls - // rather than wrapping, so all four tabs share a single offsetTop. ── - const tabTops = await pane.locator('.pane-tabs').evaluate((n) => { - const tabs = Array.from(n.querySelectorAll('[role="tab"]')); - return Array.from(new Set(tabs.map((t) => Math.round(t.getBoundingClientRect().top)))); - }); - expect(tabTops).toHaveLength(1); + const layout = await stripLayout(pane); + expect(layout.split).toBe(true); + expect(layout.tabsOnOneRow).toBe(true); + // The whole point: FIT, not "reachable by scrolling". At 312px of strip + // the ≤330px padding rule trims the tabs from 321px to 297px. + expect(layout.tabsFit).toBe(true); + expect(layout.tablistContainsActions).toBe(false); - // ── EVERY TAB REACHABLE. The tablist is a scrollport narrower than its - // content; each tab must scroll into it and take the selection. ── + // Every tab genuinely on screen and selectable. const tabs = pane.getByRole('tab'); await expect(tabs).toHaveCount(4); for (const name of ['Details', 'Relationships', 'Activity', 'Versions']) { const tab = pane.getByRole('tab', { name }); - await tab.scrollIntoViewIfNeeded(); await expect(tab).toBeInViewport(); await tab.click(); await expect(tab).toHaveAttribute('aria-selected', 'true'); } - // The ⋯ panel is anchored inside `.strip-actions`, a sibling of the - // scrolling tablist. Nothing on `.tab-strip` clips it — proving the - // "no overflow/contain on the container" constraint still holds at the - // narrowest width, where a clip would be most visible. + // `order: -1` is visual only — DOM order is untouched, so the tablist's + // arrow-key roving tabindex still walks exactly the four tabs. + await pane.locator('.pane-tab').first().focus(); + const walk: string[] = []; + for (let i = 0; i < 4; i++) { + walk.push(await page.evaluate(() => (document.activeElement?.textContent || '').trim())); + await page.keyboard.press('ArrowRight'); + } + expect(walk).toEqual(['Details', 'Relationships', 'Activity', 'Versions']); + + // The ⋯ panel is anchored inside `.strip-actions`. Nothing on + // `.tab-strip` clips it — the "no overflow/contain on the container" + // constraint still holds at the narrowest width, where a clip would be + // most visible. await pane.locator('button.pane-more-btn').click(); await expect(pane.getByRole('menuitem', { name: 'Move to collection…' })).toBeVisible(); + await page.keyboard.press('Escape'); + }); + + test('the FOLD band (560-699px) still folds: a pane dragged to its 720px maximum keeps one row and drops the badge icons + star, with Star still reachable in the ⋯ menu', async ({ + page, + fixture, + request, + }) => { + // A 720px pane (PANE_WIDTH_MAX) is a 672px strip — inside the fold band + // and the ONLY way to reach it from a docked pane. + await page.addInitScript(() => localStorage.setItem('pad-pane-width', '720')); + await page.setViewportSize({ width: 2000, height: 1000 }); + await browserLogin(page); + + const master = await seedDocWith(fixture, request, 'Tier fold'); + await seedDocWith(fixture, request, 'Tier fold child', { parent: master.id }); + + await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/docs?item=${master.slug}`); + const pane = page.locator('.item-pane'); + await expect(pane.locator('.tab-strip')).toBeVisible(); + await expect(pane.locator('.strip-actions .badge-count').first()).toBeVisible(); + + const width = await stripWidth(pane); + expect(width).toBeGreaterThanOrEqual(560); + expect(width).toBeLessThan(700); - // ── The folded star's escape hatch actually WORKS here. This is the - // width the mobile overlay always runs at, so "Star lives in ⋯" has - // to be a working action, not just a rendered row. ── + const layout = await stripLayout(pane); + expect(layout.split).toBe(false); + expect(layout.tabsFit).toBe(true); + + // Folded: icons gone, counts kept, star hidden but RETAINED (DR-11 — the + // BUG-2263 freeze assertions in pane-full-page-capstone.spec.ts need the + // node to exist). + await expect(pane.locator('.strip-actions .badge-icon').first()).toBeHidden(); + await expect(pane.locator('.strip-actions .badge-count').first()).toBeVisible(); + await expect(pane.locator('button.star-btn')).toBeHidden(); + await expect(pane.locator('button.star-btn')).toHaveCount(1); + + // The folded star still has a home, and it works. + await pane.locator('button.pane-more-btn').click(); await pane.getByRole('menuitem', { name: 'Star' }).click(); - await expect(pane.getByRole('menuitem', { name: 'Star' })).toHaveCount(0); // menu closed await pane.locator('button.pane-more-btn').click(); await expect(pane.getByRole('menuitem', { name: 'Unstar' })).toBeVisible(); await page.keyboard.press('Escape'); diff --git a/web/src/lib/components/items/ItemDetail.svelte b/web/src/lib/components/items/ItemDetail.svelte index 74c590e1..fd57e59b 100644 --- a/web/src/lib/components/items/ItemDetail.svelte +++ b/web/src/lib/components/items/ItemDetail.svelte @@ -5573,32 +5573,53 @@ color: var(--text-muted); } - /* ── Tab strip (PLAN-2326 / TASK-2328) ──────────────────────────────── - One band: the `.pane-tabs` tablist plus the right-aligned - `.strip-actions` group, which before TASK-2328 was its own - `.meta-actions` row above. The divider that used to belong to - `.pane-tabs` lives here now so it runs the full width of the strip - rather than stopping where the tabs do. - - `container-type: inline-size` makes this the query container for - TASK-2329's tier rule. It has to be here rather than on an ancestor: - the docked pane's width is user-dragged and persisted, decoupled from - the viewport, so a media query cannot see it (DR-2). + /* ── Tab strip (PLAN-2326 / TASK-2328, re-keyed by TASK-2329) ───────── + The `.pane-tabs` tablist plus the `.strip-actions` group, which before + TASK-2328 was its own `.meta-actions` row above. The divider that used + to belong to `.pane-tabs` lives here now so it runs the full width of + the strip rather than stopping where the tabs do. + + ONE band when there is room, TWO when there is not. Combining them + unconditionally does not work below ~560px and no amount of folding + fixes it: the four tab labels are a fixed 321px, and even the most + aggressive action group anyone proposed (⋯ alone, 38px) needs 359px + against the 342px a 390px phone actually has. Measured on the widest + common phone, 430px: 382px of strip, 179px of tab scrollport, two of + four tabs clipped — under `scrollbar-width: none`, so with no hint + they exist. Below 560 the actions therefore take a row of their own + and the tablist gets the full width. + + `container-type: inline-size` makes this the query container. It has + to be here rather than on an ancestor: the docked pane's width is + user-dragged and persisted, decoupled from the viewport, so a media + query cannot see it (DR-2). A 360px pane on a 1920px monitor splits; + the full page behind it does not. + + `flex-wrap` / `column-gap` / `row-gap` MUST live in THIS rule and not + in the `@container` block below. A container query cannot style its + own query container, and `.tab-strip` IS `item-tab-strip` — a + `flex-wrap: wrap` written in there is silently dropped. Not inert: + with `flex-basis: 100%` on `.strip-actions` while `nowrap` is still in + force, `.pane-tabs` collapses to 0px. Deliberately NO `overflow`, `contain`, `clip-path`, `transform`, `filter` or `will-change` on this element — it is an ancestor of both `.menu-anchor` and QuickActionsMenu's anchor, and any of those would - clip the anchored panels they position. DR-9's scroll rule goes on + clip the anchored panels they position. The scroll rule goes on `.pane-tabs`, which is their SIBLING. (`container-type` itself is safe: it applies layout/style/inline-size containment but not PAINT containment, so the panels still escape — DR-3, re-verified in Chromium against this markup, including the mobile BottomSheet's - `position: fixed` overlay, which still resolves against the - viewport.) */ + `position: fixed` overlay, which still resolves against the viewport. + Chromium reports `contain: none` here despite `container-type`; that + is expected — do NOT "fix" it by adding `contain: layout`, which DOES + trap the sheet's `position: fixed`.) */ .tab-strip { display: flex; align-items: center; - gap: var(--space-3); + column-gap: var(--space-3); + row-gap: var(--space-2); + flex-wrap: wrap; container-type: inline-size; container-name: item-tab-strip; border-bottom: 1px solid var(--border-subtle); @@ -5609,10 +5630,15 @@ .pane-tabs { display: flex; gap: 2px; - /* DR-9 width allocation: `.strip-actions` never shrinks, so the tab - list scrolls horizontally instead of wrapping to a second row or - crushing the actions. `min-width: 0` is what lets a flex item - shrink below its content width at all. */ + /* `.strip-actions` never shrinks, so whatever room is left goes to the + tab list and it scrolls rather than crushing the actions. + `min-width: 0` is what lets a flex item shrink below its content + width at all. Above 560px the two share one row and the scroll is + the overflow valve; below 560 the actions take their own row and + the tablist gets the full width, at which point the four tabs fit + outright at every phone width (the ≤330px rule trims tab padding + for the last 9px). The scroll stays as the backstop for a future + fifth tab or a longer label. */ min-width: 0; overflow-x: auto; scrollbar-width: none; @@ -5631,9 +5657,11 @@ display: none; } - /* `margin-left: auto` right-aligns the group however few tabs there are; - `flex: 0 0 auto` is DR-9's other half — the actions hold their size and - the tab list gives way. */ + /* Shared-row shape (>=560px of strip): `margin-left: auto` right-aligns + the group however few tabs there are, and `flex: 0 0 auto` holds its + size so the tab list is what gives way. Both are overridden in the + split tier below, where the group owns a full row and right-aligns via + `justify-content` instead. */ .strip-actions { display: flex; align-items: center; @@ -5649,46 +5677,58 @@ min-width: 0; } - /* ── Compact tier (PLAN-2326 DR-7 / DR-9 / DR-10) ────────────────────── - `@container`, NOT `@media`: the docked pane is - `flex: 0 0 var(--pane-width, clamp(360px, 38%, 640px))` — a width the - user drags and we persist, decoupled from the viewport — so a 360px - pane on a 1440px screen still matches `@media (min-width: 900px)`. - Only the container knows how much room the strip actually has. - - THRESHOLD — 700px, measured rather than assumed (DR-10). The strip's - content box tops out at 912px on the full page at ANY monitor width - (`--content-max-width: 960px` less 2x `--space-6` — verified identical - at 1440 and 2560), so the mock's 900px boundary would have made the - compact tier the near-universal state. Measured strip widths bracket - 700 cleanly: - - full page 892-912px -> full - full page + docked pane 526-678px -> compact - docked pane 312-672px -> compact - mobile overlay 342px -> compact - - The pane's 672px ceiling is `PaneHost.svelte`'s JS `PANE_WIDTH_MAX = - 720` less the pane's 2x24px padding — a harder bound than the CSS - `clamp()`'s 640, and the tightest margin the threshold has (28px). If - that constant ever grows past ~750 a dragged-wide pane would enter the - full tier, which is the correct outcome anyway: it would have the room. - - So the rule is "does the full tier fit", not "which surface is this": - a full page whose master column has been squeezed by an open pane - compacts too, which is the point of querying the container. (The full - tier's own worst-case content measures ~722px — 321px of tabs plus the - widest observed action group. 700 is 22px optimistic, which only bites - in the ~30px-wide viewport band where a pane-squeezed master lands - between the two; there the tab list just scrolls a few px, silently, - exactly as DR-9 designed it to.) - - TWO tiers, not the mock's three (DR-7): the `<600px` tier would have - required duplicate badge markup outside this subtree. + /* ── Tier rules, all keyed on the STRIP's own width ──────────────────── + Measured strip widths, which is what the three boundaries below are + drawn against (worst-case item: both count badges + quick actions + + owner rights): + + full page 892-912px (capped by + --content-max-width; identical at 1440 + and 2560, so a viewport media query + could never have keyed this) + full page @1024 / @768 716-720px + full page + a docked pane 526-678px + docked pane (360-720px wide) 312-672px + phone full page / overlay 312-382px + + Note the docked pane reaches 672, not the 592 the CSS `clamp(...640px)` + implies: `PaneHost.svelte`'s JS `PANE_WIDTH_MAX = 720` is the real + ceiling and it wins. + + ── SPLIT, below 560px ── + The actions take a row of their own and the tablist gets the full + width. This is the fix for the phone case, where sharing a row is not + survivable at any action-group size (see the note on `.tab-strip`). + 560 is the boundary because the shared row needs 321px of tabs plus a + ~190px compact action group plus the 12px column gap. + + ABOVE the tabs, not below (`order: -1`, visual only — DOM order and + therefore the tablist's roving tabindex are untouched). `.tab-strip` + owns the bottom divider and `.pane-tab.on` overlaps it by 1px via + `margin-bottom: -1px`; keeping the tablist as the LAST row preserves + that overlap. Actions-below would orphan the active-tab underline + ~31px above the divider. */ + @container item-tab-strip (max-width: 559px) { + .strip-actions { + order: -1; + flex: 0 0 100%; + margin-left: 0; + justify-content: flex-end; + flex-wrap: wrap; + } + } + + /* ── FOLD, 560-699px ── + Only in this band. Here the two still share a row but it is tight, so + the badge icons drop to bare counts and the star folds. Below 560 they + both come BACK: the actions have their own row and the space to show + them. Above 699 everything fits outright. The star folds by `display: none` with the node RETAINED (DR-11) — - `.star-btn` must stay in the DOM for the BUG-2263 freeze assertions. */ - @container item-tab-strip (max-width: 699px) { + `.star-btn` must stay in the DOM for the BUG-2263 freeze assertions in + pane-full-page-capstone.spec.ts. Star is also a row in the `⋯` menu, + so the action still has a home while the chip is folded. */ + @container item-tab-strip (min-width: 560px) and (max-width: 699px) { .badge-icon { display: none; } @@ -5720,6 +5760,25 @@ border-bottom-color: var(--accent-primary, var(--accent-blue)); } + /* ── TAB PADDING, below 330px of strip ── + At the 360px pane floor / a 360px phone the strip is 312px and the four + tabs need 321px even with a full row to themselves. Trimming 11px -> 8px + of horizontal padding buys 24px, which clears it (321 -> 297). Vertical + padding is untouched so the strip's height and the active tab's 1px + divider overlap do not move. + + Placement is load-bearing: this MUST sit after the base `.pane-tab` + rule. A container query adds no specificity, so the two `.pane-tab` + selectors tie and source order decides — written above the base rule it + is silently overridden and the tabs still overflow. (Caught by + measurement: `scrollWidth` stayed 321 at a 312px strip.) */ + @container item-tab-strip (max-width: 330px) { + .pane-tab { + padding-left: 8px; + padding-right: 8px; + } + } + /* Hidden tab panels stay MOUNTED (collab editor, SSE feeds, backlink counts — see the PaneTab script block). display:none only. */ .tab-hidden { From 2c05297f8ed44519f1a63ceef0ccca2703704ad8 Mon Sep 17 00:00:00 2001 From: xarmian Date: Sun, 26 Jul 2026 07:01:28 +0000 Subject: [PATCH 4/7] fix(web): re-derive the fold boundary post-salvage and gate the Star menu row to it (TASK-2329) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A third commit rather than an amend: commits 1 and 2 are already pushed, and rewriting pushed history — even on a draft PR — is not mine to do unasked. ── The upper boundary was measuring the wrong thing ── 699 was derived while Saved/Synced still lived in `.strip-actions`, and they were worth 115px of the group. Re-measured on the worst-case item (children badge AND backlinks badge AND quick actions AND owner rights) now that the salvage has put them back in `.meta-info`: action group FOLDED (icons off, star off) 191px -> shared row needs 524px action group FULL 274px -> shared row needs 607px action group FULL, counts inflated to an absurd "199/199" + "999" 301px -> shared row needs 634px So the shared row needs 607px, not the 722px the old figure implied. A 699 boundary was folding the star and the badge icons on the two WIDEST surfaces that fold at all — a pane dragged to its 672px maximum, and a 1440px full page with a pane docked at 678px — both of which have ~65px to spare. Boundary moves 699 -> 639 (full tier at 640+): 6px over the absurd case, 33px over the real one. The split boundary stays at 560, which measurement supports — the folded row needs 524px, so 560 carries 36px of slack. Net effect: a max-drag pane and a pane-squeezed 1440 master now show the star and both badge icons on one row instead of folding them. ── Star menu row is now gated to exactly the fold band ── It was unconditional, which cost every user a permanent extra row in the ⋯ menu even where the chip beside it was visible. Now `.strip-star-row` is hidden by default and shown ONLY by the same query that folds `.star-btn`, so the affordance exists in exactly one place at every width — never zero, never two, and `display: none` keeps the hidden one out of the accessibility tree so it is never announced twice. Two things this needed that reading the CSS would not have caught: 1. `.menu-anchor :global(.strip-star-row)`, not a bare `:global(...)`. MenuItem's own `.mi { display: flex }` carries that component's scoping hash and is (0,2,0); a bare global class selector is (0,1,0) and LOSES. Measured: the row rendered `display: flex` at all eight probe widths — symmetry broken in the direction that shows the star twice. Prefixing with this component's own `.menu-anchor` makes both rules (0,3,0). That is the THIRD specificity/order trap in this strip's container rules, after `flex-wrap` in the base rule and the ≤330 `.pane-tab` rule needing to follow its base. Container queries add no specificity; every rule in here has to be checked against what it is overriding. 2. `MenuItem` gained an optional `class` prop so the row can be addressed at all. Additive; no existing caller passes it. ── Verified in the browser, both directions, eight widths ── phone 390 (strip 342, split) chip visible, menu row display:none pane 360 (strip 312, split) chip visible, row none pane 448 (strip 400, split) chip visible, row none pane 620 (strip 572, FOLD) chip none, row visible pane 680 (strip 632, FOLD) chip none, row visible pane 720 (strip 672, full) chip visible, row none full page 1024 (strip 716) chip visible, row none full page 1440 (strip 912) chip visible, row none Exactly one of the two at every width. The phone case is the mobile BottomSheet rather than the anchored panel, confirming the container query reaches into both — neither portals out of `.tab-strip`. ── Slow drag through the boundaries ── Dragging the pane 563 -> 688px in 5px steps produced THREE distinct states across 27 samples and nothing in between: split (strip 515, 67px tall, star shown) -> fold at strip 560 (33px, star folded) -> full at strip 640 (33px, star shown). Tabs fit in every single sample. The transitions land exactly on the declared boundaries and read as deliberate, not glitchy. Desktop is still unchanged: 1440/1200 one row, header 148.8px; 1024/768 one row, 174.8px; underline offset 0 everywhere. ── Codex round 2: the boundaries had a sub-pixel gap ── Encoded as three adjacent integer bands (max-width 559 / 560..639 / 640+), a strip measuring 559.5px matched NEITHER the split nor the fold query and fell back to the full shared row that needs 607px. Container widths are not integers — the pane default is `clamp(360px, 38%, 640px)` and PaneHost's JS floors are `usable * 0.4` — so that width is reachable, and every test here used integers so none of them saw it. Fixed two ways at once. The blocks now NEST rather than abut: FOLD covers everything at or below the full threshold and SPLIT is a strict subset that also UN-folds what the outer block hid, which bounds the damage on its own (measured at 559.5 pre-fix: folded instead of split, and the folded row needs only 524px so the tabs still fit). And the bounds are .99 rather than integers, leaving a [559.99, 560) window no engine can land in — layout precision is 1/64px in Chromium and WebKit, 1/60px in Gecko. Range syntax `(560px <= width < 640px)` would be exact and read better, but a browser that cannot parse it drops the whole block, and on a phone that means the broken shared row returns. `max-width` is supported everywhere `@container` is. Verified by forcing the strip to 559.50 / 559.99 / 560.50 / 639.50 / 639.99 / 640.50 and checking the tier and tab fit at each — all six correct, where four were wrong before. Added as a regression test; it is the only one here that produces fractional widths. Gates: `npm run check` 0 errors; `make check` exit 0 (Go tests, govulncheck clean, 490 web unit tests); tier spec 5/5 and capstone 8/8; full e2e 81 passed with one BUG-2334 toast-overlay flake in pane-content-link-anchors (green in isolation, and a clean 81/0 run earlier in this same commit). Claude-Session: https://claude.ai/code/session_01E2fRi12n8rARczvdEa2LYT --- web/e2e/pane-tab-strip-tiers.spec.ts | 127 ++++++++++++-- web/src/lib/components/common/MenuItem.svelte | 9 +- .../lib/components/items/ItemDetail.svelte | 160 ++++++++++++++---- 3 files changed, 252 insertions(+), 44 deletions(-) diff --git a/web/e2e/pane-tab-strip-tiers.spec.ts b/web/e2e/pane-tab-strip-tiers.spec.ts index fa8a61de..b560f946 100644 --- a/web/e2e/pane-tab-strip-tiers.spec.ts +++ b/web/e2e/pane-tab-strip-tiers.spec.ts @@ -12,11 +12,17 @@ import type { SuiteFixture } from './fixtures'; * < 560px SPLIT — `.strip-actions` takes a full row of its own, `order: * -1` so it sits ABOVE the tablist, and the tabs get the full * width. Everything the shared row folded comes back. - * 560-699 FOLD — still one row, but tight: badge icons drop to bare - * counts and the star folds (node retained, DR-11). + * 560-639 FOLD — still one row, but tight: badge icons drop to bare + * counts and the star folds (node retained, DR-11) with its + * affordance moving to a `.strip-star-row` in the ⋯ menu, gated + * by the same query so exactly one of the two ever shows. * <= 330px the tabs' horizontal padding trims 11px -> 8px, the last 24px * needed to fit four tabs in a 312px strip. * + * The boundaries are measured, not chosen: the action group is 191px folded + * (row needs 524px), 274px full (607px), and 301px with absurd badge counts + * like "199/199" + "999" (634px). + * * Why the split exists at all: combining tabs and actions on one row is not * survivable at phone widths and no amount of folding fixes it. The four tab * labels are a fixed 321px, and even a hypothetical action group of nothing @@ -166,7 +172,7 @@ test.describe('tab-strip tiers (PLAN-2326 / TASK-2329)', () => { const badgeIcon = col.locator('.strip-actions .badge-icon'); const badgeCount = col.locator('.strip-actions .badge-count'); - // ── SHARED ROW. 892px of strip at this viewport, way over 699. ── + // ── SHARED ROW. 892px of strip at this viewport, way over 640. ── await expect(col.locator('.tab-strip')).toBeVisible(); await expect(badgeCount.first()).toBeVisible(); // children badge has arrived expect(await stripWidth(col)).toBeGreaterThanOrEqual(700); @@ -302,15 +308,16 @@ test.describe('tab-strip tiers (PLAN-2326 / TASK-2329)', () => { await page.keyboard.press('Escape'); }); - test('the FOLD band (560-699px) still folds: a pane dragged to its 720px maximum keeps one row and drops the badge icons + star, with Star still reachable in the ⋯ menu', async ({ + test('the FOLD band (560-639px) folds the badge icons + star, and the Star affordance moves into the ⋯ menu — exactly one of the two exists at any width', async ({ page, fixture, request, }) => { - // A 720px pane (PANE_WIDTH_MAX) is a 672px strip — inside the fold band - // and the ONLY way to reach it from a docked pane. - await page.addInitScript(() => localStorage.setItem('pad-pane-width', '720')); - await page.setViewportSize({ width: 2000, height: 1000 }); + // A 640px pane is a 592px strip: inside the fold band. The band is only + // reachable by dragging the pane to 608-687px, so it has to be set up + // rather than stumbled into. + await page.addInitScript(() => localStorage.setItem('pad-pane-width', '640')); + await page.setViewportSize({ width: 1800, height: 1000 }); await browserLogin(page); const master = await seedDocWith(fixture, request, 'Tier fold'); @@ -323,7 +330,7 @@ test.describe('tab-strip tiers (PLAN-2326 / TASK-2329)', () => { const width = await stripWidth(pane); expect(width).toBeGreaterThanOrEqual(560); - expect(width).toBeLessThan(700); + expect(width).toBeLessThan(640); const layout = await stripLayout(pane); expect(layout.split).toBe(false); @@ -337,11 +344,111 @@ test.describe('tab-strip tiers (PLAN-2326 / TASK-2329)', () => { await expect(pane.locator('button.star-btn')).toBeHidden(); await expect(pane.locator('button.star-btn')).toHaveCount(1); - // The folded star still has a home, and it works. + // ...and the menu row takes over, gated by the SAME query. The folded + // star must have exactly one home: here. await pane.locator('button.pane-more-btn').click(); + await expect(page.locator('.strip-star-row')).toBeVisible(); await pane.getByRole('menuitem', { name: 'Star' }).click(); await pane.locator('button.pane-more-btn').click(); await expect(pane.getByRole('menuitem', { name: 'Unstar' })).toBeVisible(); await page.keyboard.press('Escape'); }); + + test('the tier boundaries have no sub-pixel gap: a fractional strip width either side of 560 and 640 still lands in the right tier', async ({ + page, + fixture, + request, + }) => { + // Container widths are NOT integers — the docked pane's default is + // `clamp(360px, 38%, 640px)` and PaneHost's JS floors are `usable * 0.4` + // — so a strip can genuinely measure 559.5px. Encoded as three adjacent + // integer bands (`max-width: 559` / `560..639` / `640+`) that width + // matches NOTHING and silently falls back to the full shared row, which + // needs 607px. The fix is nested ranges plus `.99` bounds; this is the + // regression guard, and it needs fractional widths that no other test + // here produces. + await page.setViewportSize({ width: 1800, height: 1000 }); + await browserLogin(page); + const master = await seedDocWith(fixture, request, 'Tier subpixel'); + await seedDocWith(fixture, request, 'Tier subpixel child', { parent: master.id }); + await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/docs?item=${master.slug}`); + const pane = page.locator('.item-pane'); + await expect(pane.locator('.tab-strip')).toBeVisible(); + + // pane width - 48px of padding = strip width. + const cases: Array<[number, 'split' | 'fold' | 'full']> = [ + [607.5, 'split'], // strip 559.50 — was in the gap + [607.99, 'split'], // strip 559.99 — was in the gap + [608.5, 'fold'], // strip 560.50 + [687.5, 'fold'], // strip 639.50 — was in the gap + [687.99, 'fold'], // strip 639.99 — was in the gap + [688.5, 'full'], // strip 640.50 + ]; + + for (const [paneWidth, expected] of cases) { + await page.addStyleTag({ content: `.item-pane{flex:0 0 ${paneWidth}px !important;}` }); + const state = await expect + .poll( + async () => + pane.locator('.tab-strip').evaluate((strip) => { + const tabs = strip.querySelector('.pane-tabs') as HTMLElement; + const acts = strip.querySelector('.strip-actions') as HTMLElement; + const split = + acts.getBoundingClientRect().bottom <= tabs.getBoundingClientRect().top + 1; + const starHidden = + getComputedStyle(strip.querySelector('.star-btn') as HTMLElement).display === + 'none'; + return `${split ? 'split' : starHidden ? 'fold' : 'full'}|${ + tabs.scrollWidth <= tabs.clientWidth + }`; + }), + { timeout: 3000 }, + ) + .toBe(`${expected}|true`) + .then(() => expected); + expect(state).toBe(expected); + } + }); + + test('the Star affordance is never duplicated and never absent: the ⋯ menu row appears ONLY where the chip is folded', async ({ + page, + fixture, + request, + }) => { + await browserLogin(page); + const master = await seedDocWith(fixture, request, 'Tier symmetry'); + await seedDocWith(fixture, request, 'Tier symmetry child', { parent: master.id }); + const paneUrl = `/${fixture.adminUsername}/${fixture.workspaceSlug}/docs?item=${master.slug}`; + + // [label, viewport, stored pane width, expected tier] + const widths: Array<[string, { width: number; height: number }, number, 'split' | 'fold' | 'full']> = [ + ['360px pane (strip 312)', { width: 1440, height: 1000 }, 360, 'split'], + ['448px pane (strip 400)', { width: 1440, height: 1000 }, 448, 'split'], + ['620px pane (strip 572)', { width: 1600, height: 1000 }, 620, 'fold'], + ['720px pane (strip 672)', { width: 2000, height: 1000 }, 720, 'full'], + ]; + + for (const [label, viewport, paneWidth, tier] of widths) { + await page.setViewportSize(viewport); + await page.addInitScript((w) => localStorage.setItem('pad-pane-width', String(w)), paneWidth); + await page.goto(paneUrl); + const pane = page.locator('.item-pane'); + await expect(pane.locator('.tab-strip')).toBeVisible(); + + const chip = pane.locator('button.star-btn'); + const row = page.locator('.strip-star-row'); + // Open the ⋯ so the row is mounted and measurable. + await pane.locator('button.pane-more-btn').click(); + await expect(row).toHaveCount(1); // always in the DOM, gated by CSS only + + if (tier === 'fold') { + await expect(chip, label).toBeHidden(); + await expect(row, label).toBeVisible(); + } else { + await expect(chip, label).toBeVisible(); + await expect(row, label).toBeHidden(); + } + await page.keyboard.press('Escape'); + } + }); }); diff --git a/web/src/lib/components/common/MenuItem.svelte b/web/src/lib/components/common/MenuItem.svelte index eae41daa..dedd8816 100644 --- a/web/src/lib/components/common/MenuItem.svelte +++ b/web/src/lib/components/common/MenuItem.svelte @@ -15,6 +15,12 @@ * destructive confirmation, which is presentational and otherwise * never announced when the row takes focus (PLAN-2326). */ describedBy?: string; + /** Extra class on the row, for callers that need to address it from + * their own stylesheet — e.g. a container query that shows a row only + * at widths where the equivalent toolbar button is folded away + * (ItemDetail's Star row, TASK-2329). Reach it with `:global()`: the + * caller's scoping hash is not applied to this element. */ + class?: string; onclick?: (e: MouseEvent) => void; children: Snippet; } @@ -26,6 +32,7 @@ checked, disabled = false, describedBy, + class: extraClass, onclick, children }: Props = $props(); @@ -33,7 +40,7 @@ {/each}
@@ -5676,12 +5737,24 @@ width at all. Above 560px the two share one row and the scroll is the overflow valve; below 560 the actions take their own row and the tablist gets the full width, at which point the four tabs fit - outright at every phone width (the ≤330px rule trims tab padding - for the last 9px). The scroll stays as the backstop for a future - fifth tab or a longer label. */ + outright at every phone width (the short `Links` label plus the + ≤330px padding trim buy the slack). The scroll stays as the backstop + for a future fifth tab, a longer label, a translation, or a font we + have not measured. + + That backstop USED to be silent: `scrollbar-width: none` hides the + only native signal that content is cut off, which is exactly the + discoverability defect that started this whole episode (two of four + tabs invisible at 430px with nothing to indicate it). The + `background` below restores a signal without giving the scrollbar + back — see the note on it. */ min-width: 0; overflow-x: auto; scrollbar-width: none; + /* Fade widths for the overflow affordance below — 0 unless the script + has measured content past that edge. */ + --fade-l: 0px; + --fade-r: 0px; /* `overflow-x: auto` computes `overflow-y` to `auto` too, which would clip `.pane-tab`'s `margin-bottom: -1px` overlap and leave the active tab with a 1px accent sitting on 1px of divider instead of a @@ -5697,6 +5770,44 @@ display: none; } + /* ── OVERFLOW AFFORDANCE (TASK-2329) ────────────────────────────────── + `scrollbar-width: none` hides the only native signal that content is + cut off — which IS the defect that started this episode: two of four + tabs invisible at 430px with nothing to say so. The label fix means + overflow now needs a font ~30% wider than CI's before it happens at + all, but "needs an unusual font" is not "cannot happen" (translations, + user zoom, a future fifth tab), so it must degrade visibly rather than + silently truncate. + + A soft edge fade, applied ONLY on an edge that actually has more + content past it, so at rest there is no mask at all. + + Why this is script-measured and not a container query: CSS cannot ask + "is this element scrolling". That is the opposite situation to the tier + rules, where CSS expresses the question natively and DR-2 rightly + rejected JS — here JS is the only tool that can answer it. It reads + `scrollWidth`/`clientWidth` and writes two booleans; it does not + participate in the tier logic at all. + + A pure-CSS attempt came first and was rejected on measurement, not + taste: the two-layer `background-attachment: local/scroll` scroll-shadow + idiom is conditional by construction, but backgrounds paint BEHIND the + tab text, and sampling the rendered edge pixels showed no legible + signal in either theme. Recorded so nobody re-tries it. + + `mask-image` on `.pane-tabs` is safe: it is a paint effect (the -1px + divider overlap still measures 0 in both tiers) and `.pane-tabs` is a + SIBLING of `.menu-anchor`, so it cannot clip the anchored panels. */ + .pane-tabs.masked { + mask-image: linear-gradient( + to right, + transparent 0, + #000 var(--fade-l), + #000 calc(100% - var(--fade-r)), + transparent 100% + ); + } + /* Shared-row shape (>=560px of strip): `margin-left: auto` right-aligns the group however few tabs there are, and `flex: 0 0 auto` holds its size so the tab list is what gives way. Both are overridden in the @@ -5915,6 +6026,45 @@ } } + /* ── TAB LABELS: short form below 560px of strip ─────────────────────── + `Relationships` -> `Links`, visible text only. The accessible name is + pinned to the full label by `aria-label` on the tab, so + `getByRole('tab', { name: 'Relationships' })` — used by ~5 specs — keeps + resolving, and a screen reader never hears "Links". + + This is what buys the fit REAL SLACK instead of a tuned zero. CI proved + the previous derivation was environment-specific: `--font-ui` is + `-apple-system, BlinkMacSystemFont, 'Segoe UI', system-ui, sans-serif`, + every entry of which misses on Linux, so `system-ui` resolves through + fontconfig to different font FILES on a dev box and on ubuntu-latest. + Same declaration, 17.5% wider glyphs, measured: + + dev box CI delta + four tab labels 297px 337px +40px + strip at 360 pane 312px 312px + slack +15 -25 <-- CI overflowed by 25px + + Ten candidate families forced locally spanned 273-297px, so CI is wider + than the entire local envelope — this was not findable without the + runner. `Relationships` alone was 84px of glyphs locally and 98px on CI. + Swapping it for `Links` takes 51px off the row (measured), which turns + CI's -25px into roughly +34px and every local font into +60px or more. + + ⚠ Same ordering rule as the padding block above: these tie with the base + `.tab-label-*` rules at (0,2,0), so the query MUST stay after them. */ + .tab-label-short { + display: none; + } + + @container item-tab-strip (max-width: 559.99px) { + .tab-label-full { + display: none; + } + .tab-label-short { + display: inline; + } + } + /* Hidden tab panels stay MOUNTED (collab editor, SSE feeds, backlink counts — see the PaneTab script block). display:none only. */ .tab-hidden {