From 9bfbac9491241f5a6992b345a406ed3db81a1652 Mon Sep 17 00:00:00 2001 From: Hako Date: Tue, 6 Oct 2026 01:07:33 +0900 Subject: [PATCH 1/2] fix(tabs): a tab strip the user scrolled is not pulled back to the open tab when a typeface arrives Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 +- docs/development.md | 2 +- scripts/font-swap-demo-regression.ts | 72 ++++++++++++++++++++++++++-- scripts/mobile-tabs-regression.ts | 61 +++++++++++++++-------- src/components/TabStrip.tsx | 36 ++++++++++---- src/lib/tabStripScroll.test.ts | 45 +++++++++++++++++ src/lib/tabStripScroll.ts | 33 +++++++++++++ 7 files changed, 220 insertions(+), 34 deletions(-) create mode 100644 src/lib/tabStripScroll.test.ts create mode 100644 src/lib/tabStripScroll.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index fd35766974..22c7227624 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,7 +23,10 @@ between releases do not reach them. Remote-PC runtime bundles are versioned sepa everything) on a looser line; bold is a semibold; inline code sits a little under its line and code blocks are a step larger with more room; the message box is typed at the same size with a mouse and stays 16px on touch; the empty chat's question is a larger line. Tool rows, times - and other small labels keep their size. + and other small labels keep their size. A typeface that arrives late redraws the tabs' names + at another width, and the tab strip then brings the open tab back into view, unless you have + scrolled the strip yourself to look at other tabs: it stays where you left it until you open + a tab. ([#473](https://github.com/devswha/herdr-web-ui/pull/473)) - In the chat's message box the agent's mark, the model, the reasoning level and the context ring sit together in one quiet pill, and the model is shown by its name where the id is a diff --git a/docs/development.md b/docs/development.md index 5c001768d2..d94d6d7a9d 100644 --- a/docs/development.md +++ b/docs/development.md @@ -47,7 +47,7 @@ bun scripts/chat-greeting-demo-regression.ts # an empty chat's greeting: centre bun scripts/composer-fit-demo-regression.ts # the input card's model label: whole or stepped out beside Queue, the context number and an upload bun scripts/held-rows-demo-regression.ts # held messages: the fold under an approval card, its button, a row's error bun scripts/prompt-dock-demo-regression.ts # the prompt card docked over the input card: its place, its height on a short phone, the grip, a typed pick -bun scripts/font-swap-demo-regression.ts # the app's faces arriving late on a slow link: a reader at the end of a chat stays there +bun scripts/font-swap-demo-regression.ts # the app's faces arriving late on a slow link: a reader at the end of a chat stays there, a tab strip the user scrolled stays put ``` `FILE_VIEWER_CASE=landscape-notch` selects a viewer case; `FILE_VIEWER_CSS=/path/to/before.css` compares another stylesheet. These checks use Chromium mobile emulation and synthetic safe-area/keyboard geometry; they cannot verify actual iOS Safari keyboard dismissal or notch insets. The existing `bun scripts/file-viewer-regression.ts` separately checks history with an owned herdr pane. The original `scripts/mobile-viewport-regression.ts` exports `checkMobileViewport` for the real-app `bun run test:ui` suite; it also checks the command palette and xterm focus transitions. The demo runners build the real client into a temporary directory, inject the committed fictional-session transport and serve it only on loopback; they do not use a live herdr session or download website media. They exercise real-app viewport and alert geometry, but not live herdr connectivity. diff --git a/scripts/font-swap-demo-regression.ts b/scripts/font-swap-demo-regression.ts index a7b6652736..e98e557cdc 100644 --- a/scripts/font-swap-demo-regression.ts +++ b/scripts/font-swap-demo-regression.ts @@ -15,10 +15,10 @@ import panes from "../site/demo/fixtures/panes.json"; const repo = join(import.meta.dir, ".."); const app = mkdtempSync(join(tmpdir(), "herdr-font-swap-demo-")); -interface View { top: number; gap: number; height: number } +interface View { top: number; gap: number; height: number; window: number } const viewOf = (page: Page): Promise => page.evaluate(() => { const node = document.querySelector(".chat-view")!; - return { top: node.scrollTop, gap: Math.round(node.scrollHeight - node.clientHeight - node.scrollTop), height: node.scrollHeight }; + return { top: node.scrollTop, gap: Math.round(node.scrollHeight - node.clientHeight - node.scrollTop), height: node.scrollHeight, window: node.clientHeight }; }); const frames = (page: Page): Promise => page.evaluate(() => new Promise((done) => requestAnimationFrame(() => requestAnimationFrame(() => done())))); const loadedFaces = (page: Page, family: string): Promise => page.evaluate((name) => [...document.fonts].filter((face) => face.family.replace(/["']/g, "") === name && face.status === "loaded").length, family); @@ -86,16 +86,19 @@ try { }; { - const { page, errors, held, release, close } = await open(null); + // a short window: a conversation that fits its window has no end to be kept at + const { page, errors, held, release, close } = await open(null, 480); try { assert.ok(held() > 0, "the first drawing asked for a face"); assert.equal(await loadedFaces(page, "Pretendard Variable"), 0, "the conversation is first drawn before the face comes"); assert.equal(await loadedFaces(page, "JetBrains Mono Web"), 0, "and before the code face comes"); const before = await viewOf(page); - assert.ok(before.height > 0 && before.gap <= 1, `an opened conversation is at its end (gap ${before.gap})`); + assert.ok(before.height > before.window + 48, `the conversation is longer than its window (${before.height} in ${before.window})`); + assert.ok(before.gap <= 1, `an opened conversation is at its end (gap ${before.gap})`); release(); await appFaces(page); const after = await viewOf(page); + assert.notEqual(after.height, before.height, `the faces redrew the conversation at another height (${before.height} -> ${after.height})`); assert.ok(after.gap <= 1, `a reader at the end is still there after the faces came (gap ${after.gap}, height ${before.height} -> ${after.height})`); assert.deepEqual(errors, []); console.log(`PASS a reader at the end stays at the end when the faces swap in (height ${before.height} -> ${after.height})`); @@ -104,10 +107,11 @@ try { { // one file never comes: the set's "loadingdone" does not fire, and the face that did come still counts - const { page, errors, release, close } = await open(/JetBrainsMono/); + const { page, errors, release, close } = await open(/JetBrainsMono/, 480); try { assert.equal(await loadedFaces(page, "Pretendard Variable"), 0, "the conversation is first drawn before the face comes"); const before = await viewOf(page); + assert.ok(before.height > before.window + 48, `the conversation is longer than its window (${before.height} in ${before.window})`); assert.ok(before.gap <= 1, `an opened conversation is at its end (gap ${before.gap})`); release(); await page.waitForFunction(() => [...document.fonts].some((face) => face.family.replace(/["']/g, "") === "Pretendard Variable" && face.status === "loaded"), null, { timeout: 10_000 }); @@ -115,6 +119,7 @@ try { assert.equal(await page.evaluate(() => document.fonts.status), "loading", "another face is still on its way"); assert.equal(await loadedFaces(page, "JetBrains Mono Web"), 0); const after = await viewOf(page); + assert.notEqual(after.height, before.height, `the face redrew the conversation at another height (${before.height} -> ${after.height})`); assert.ok(after.gap <= 1, `a face that came is followed while another is still on its way (gap ${after.gap}, height ${before.height} -> ${after.height})`); assert.deepEqual(errors, []); console.log(`PASS a face that arrived is followed while another file is still in flight (height ${before.height} -> ${after.height})`); @@ -139,6 +144,63 @@ try { console.log("PASS a reader who scrolled up is not pulled to the end when the faces swap in"); } finally { await close(); } } + + // the tab strip: a workspace given more tabs than a phone's strip shows. The reader scrolls + // to the last one and opens it, all in the first drawing; `scrolled` is what they do next. + for (const scrolled of [false, true]) { + const { page, errors, release, close } = await open(null); + try { + const workspace = panes.web.split(":")[0]!; + for (const label of ["second", "a long tab name for review", "deploy", "docs"]) { + const made = await page.evaluate(async (body) => (await fetch("/api/tab/create", { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify(body) })).status, { workspace_id: workspace, label }); + assert.equal(made, 200, `the demo makes the tab "${label}"`); + } + const strip = page.locator(".tab-strip"); + const stripOf = (): Promise<{ left: number; max: number; open: boolean }> => strip.evaluate((node) => { + const view = node.getBoundingClientRect(); + const item = node.querySelector(".tab-strip-item.is-active")!.getBoundingClientRect(); + const end = node.querySelector(".tab-strip-add")!.getBoundingClientRect().left; + return { left: node.scrollLeft, max: node.scrollWidth - node.clientWidth, open: item.left >= view.left - 1 && item.right <= end + 1 }; + }); + const tabs = strip.locator('[role="tab"]'); + await tabs.nth(4).waitFor(); + // a scroll the strip did not make, as each one below: the app sees only the scroll event + await strip.evaluate((node) => { node.scrollLeft = node.scrollWidth; }); + await frames(page); + await tabs.nth(4).tap(); + await strip.locator('[role="tab"][aria-selected="true"]', { hasText: "docs" }).waitFor(); + await frames(page); + const opened = await stripOf(); + assert.ok(opened.open && opened.left > 48, `the last tab is open and in view, at the strip's far end (${JSON.stringify(opened)})`); + assert.equal(await loadedFaces(page, "Pretendard Variable"), 0, "and is drawn before the face comes"); + + if (scrolled) { + await strip.evaluate((node) => { node.scrollLeft = 0; }); + await frames(page); + const looking = await stripOf(); + assert.ok(looking.left === 0 && !looking.open, `the open tab is out of view, where the reader left the strip (${JSON.stringify(looking)})`); + } + release(); + await appFaces(page); + const after = await stripOf(); + assert.notEqual(after.max, opened.max, `the faces redrew the tabs' names at another width (${opened.max} -> ${after.max})`); + if (scrolled) { + assert.equal(after.left, 0, "a strip the reader scrolled is left there when the faces come"); + assert.equal(after.open, false); + await tabs.nth(0).tap(); + await strip.locator('[role="tab"][aria-selected="true"]').first().waitFor(); + await page.waitForFunction(() => document.querySelector('.tab-strip [role="tab"]')?.getAttribute("aria-selected") === "true"); + await frames(page); + assert.ok((await stripOf()).open, "and a tab opened after that is in view"); + } else { + assert.ok(after.open, `the open tab is in view again, the reader's scroll before they opened it forgotten (${JSON.stringify(after)})`); + } + assert.deepEqual(errors, []); + console.log(scrolled + ? "PASS a tab strip the reader scrolled away from the open tab stays put when the faces swap in" + : `PASS a tab strip follows its open tab when the faces swap in, also after a scroll that ended in opening it (scrollLeft ${opened.left} -> ${after.left})`); + } finally { await close(); } + } } finally { await browser.close(); } } finally { server.stop(); } } finally { rmSync(app, { recursive: true, force: true }); } diff --git a/scripts/mobile-tabs-regression.ts b/scripts/mobile-tabs-regression.ts index 15aa3095c0..642e160239 100644 --- a/scripts/mobile-tabs-regression.ts +++ b/scripts/mobile-tabs-regression.ts @@ -134,26 +134,49 @@ export async function checkMobileTabs(browser: Browser, origin: string): Promise // every tab's name is redrawn with them, wider or narrower: the strip that brought the open // tab into view in the fallback brings it into view again. The font files are held here until // the fallback drawing is seen; no service worker, so that nothing answers them from a cache. - const slow = await browser.newContext({ viewport: { width: 390, height: 844 }, isMobile: true, hasTouch: true, locale: "en-US", serviceWorkers: "block" }); - try { - await slow.addInitScript(() => localStorage.setItem("herdr-web-ui:settings", JSON.stringify({ language: "en" }))); - const late = await slow.newPage(); - const lateErrors: string[] = []; - late.on("pageerror", (error) => lateErrors.push(error.message)); - let release = (): void => undefined; - const released = new Promise((done) => { release = done; }); - await late.route("**/*.woff2", async (route) => { await released; await route.continue().catch(() => undefined); }); + // A strip the user scrolled themselves, to look at other tabs, is left where they put it by a + // face that comes after: the scroll below is not the strip's own, which is all the app can + // know of a wheel or a drag. Opening a tab ends that. + for (const scrolled of [false, true]) { + const slow = await browser.newContext({ viewport: { width: 390, height: 844 }, isMobile: true, hasTouch: true, locale: "en-US", serviceWorkers: "block" }); try { - await late.goto(`${origin}/?pane=${encodeURIComponent(last)}`); - await late.locator(".conn-live").waitFor(); - await late.locator('.tab-strip [role="tab"][aria-selected="true"]', { hasText: "docs" }).waitFor(); - assert.equal(await late.evaluate(() => [...document.fonts].filter((face) => face.family.replace(/["']/g, "") === "Pretendard Variable" && face.status === "loaded").length), 0, "the strip is first drawn before its face comes"); - await activeTabInView(late, "the last tab, before the faces came"); - } finally { release(); } - await appFaces(late); - await activeTabInView(late, "the last tab, after the faces came"); - assert.deepEqual(lateErrors, []); - } finally { await slow.close(); } + await slow.addInitScript(() => localStorage.setItem("herdr-web-ui:settings", JSON.stringify({ language: "en" }))); + const late = await slow.newPage(); + const lateErrors: string[] = []; + late.on("pageerror", (error) => lateErrors.push(error.message)); + let release = (): void => undefined; + const released = new Promise((done) => { release = done; }); + await late.route("**/*.woff2", async (route) => { await released; await route.continue().catch(() => undefined); }); + const strip = late.locator(".tab-strip"); + const openInView = (): Promise => strip.evaluate((node) => { + const item = node.querySelector(".tab-strip-item.is-active")!.getBoundingClientRect(); + return item.left >= node.getBoundingClientRect().left - 1 && item.right <= node.querySelector(".tab-strip-add")!.getBoundingClientRect().left + 1; + }); + try { + await late.goto(`${origin}/?pane=${encodeURIComponent(last)}`); + await late.locator(".conn-live").waitFor(); + await late.locator('.tab-strip [role="tab"][aria-selected="true"]', { hasText: "docs" }).waitFor(); + assert.equal(await late.evaluate(() => [...document.fonts].filter((face) => face.family.replace(/["']/g, "") === "Pretendard Variable" && face.status === "loaded").length), 0, "the strip is first drawn before its face comes"); + await activeTabInView(late, "the last tab, before the faces came"); + if (scrolled) { + await strip.evaluate((node) => { node.scrollLeft = 0; }); + await late.evaluate(() => new Promise((done) => requestAnimationFrame(() => requestAnimationFrame(() => done())))); + assert.equal(await openInView(), false, "the user scrolled the strip away from the open tab"); + } + } finally { release(); } + await appFaces(late); + if (scrolled) { + assert.equal(await strip.evaluate((node) => node.scrollLeft), 0, "a strip the user scrolled is left there when the faces come"); + assert.equal(await openInView(), false); + await late.getByRole("tab", { name: "Tab 1", exact: true }).tap(); + await late.locator('.tab-strip [role="tab"][aria-selected="true"]', { hasText: "Tab 1" }).waitFor(); + await activeTabInView(late, "a tab opened after the user's scroll"); + } else { + await activeTabInView(late, "the last tab, after the faces came"); + } + assert.deepEqual(lateErrors, []); + } finally { await slow.close(); } + } console.log("PASS a phone keeps the pane's title in the header and the open tab and the + in the strip, in both lenses and with the keyboard up"); } finally { await context.close(); diff --git a/src/components/TabStrip.tsx b/src/components/TabStrip.tsx index 105a04fa01..d273ec4e12 100644 --- a/src/components/TabStrip.tsx +++ b/src/components/TabStrip.tsx @@ -22,6 +22,7 @@ import { useFacesArrived } from "../lib/fontFaces.ts"; import { focusWorkspaceListToggle } from "../lib/focus.ts"; import { useT } from "../lib/i18n.ts"; import { customTabLabel, tabLabel } from "../lib/tabName.ts"; +import { STRIP_AT_REST, stripPlaced, stripScrolled, stripSelected, type StripScroll } from "../lib/tabStripScroll.ts"; import { rosterPanes } from "../lib/dagPane.ts"; import { useMachineApi, useMachineId } from "../lib/machineContext.tsx"; import { knownStatus } from "../lib/status.ts"; @@ -109,20 +110,39 @@ export function TabStrip({ snapshot, workspace, selectedPane, onSelectPane, onNe // the open tab is in view: a pane opened from the sidebar, the palette or an alert can be on a // tab scrolled out of a phone's strip. Only the strip scrolls, never the page around it. - // A face that arrives after that (lib/fontFaces.ts) redraws every name wider or narrower with - // no tab added or opened, so the open tab is brought into view again for each. const shown = panes.length >= 2; - const faces = useFacesArrived(); - useLayoutEffect(() => { + const scroll = useRef(STRIP_AT_REST); + const bringOpenTab = (): void => { const row = strip.current; - const open = row?.querySelector(".tab-strip-item.is-active"); - if (!row || !open) return; + if (!row) { scroll.current = STRIP_AT_REST; return; } + const open = row.querySelector(".tab-strip-item.is-active"); + if (!open) return; const view = row.getBoundingClientRect(); const item = open.getBoundingClientRect(); const end = row.querySelector(".tab-strip-add")?.getBoundingClientRect().left ?? view.right; + const before = row.scrollLeft; if (item.left < view.left) row.scrollLeft -= view.left - item.left; else if (item.right > end) row.scrollLeft += item.right - end; - }, [selectedPane.tab_id, tabs.length, shown, faces]); + // where the browser really left it; a strip that did not have to move may hold a scroll of + // the user's whose event is still to come, and that one is left for the event to tell + if (row.scrollLeft !== before || scroll.current.at === null) scroll.current = stripPlaced(scroll.current, row.scrollLeft); + }; + useLayoutEffect(() => { + scroll.current = stripSelected(scroll.current); + }, [selectedPane.tab_id]); + useLayoutEffect(bringOpenTab, [selectedPane.tab_id, tabs.length, shown]); + // A face that arrives after that (lib/fontFaces.ts) redraws every name wider or narrower with + // no tab added or opened, so the open tab is brought into view again for each: unless the user + // has scrolled the strip themselves since a tab was last opened (lib/tabStripScroll.ts), and is + // looking at other tabs. A chunk can come long after the page, with the first Korean on it. + const faces = useFacesArrived(); + useLayoutEffect(() => { + if (!scroll.current.moved) bringOpenTab(); + }, [faces]); + const onScroll = (): void => { + const row = strip.current; + if (row) scroll.current = stripScrolled(scroll.current, row.scrollLeft, row.scrollWidth - row.clientWidth); + }; if (!shown) return null; @@ -235,7 +255,7 @@ export function TabStrip({ snapshot, workspace, selectedPane, onSelectPane, onNe return ( <> -
+
{tabs.map((tab) => { const active = tab.tab_id === selectedPane.tab_id; const own = panesOf(tab); diff --git a/src/lib/tabStripScroll.test.ts b/src/lib/tabStripScroll.test.ts new file mode 100644 index 0000000000..ab21489ee4 --- /dev/null +++ b/src/lib/tabStripScroll.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, test } from "bun:test"; +import { STRIP_AT_REST, stripPlaced, stripScrolled, stripSelected } from "./tabStripScroll.ts"; + +describe("whose scroll moved the tab strip", () => { + const placed = stripPlaced(STRIP_AT_REST, 240); + + test("the scroll event of the strip's own placing is not the user's", () => { + expect(stripScrolled(placed, 240, 600)).toBe(placed); + }); + + test("a scroll that ends anywhere else is the user's, to either side", () => { + expect(stripScrolled(placed, 0, 600)).toEqual({ at: 0, moved: true }); + expect(stripScrolled(placed, 600, 600).moved).toBe(true); + expect(stripScrolled(placed, 239.5, 600).moved).toBe(true); + }); + + test("a strip that never placed itself was scrolled by the user", () => { + expect(stripScrolled(STRIP_AT_REST, 12, 600).moved).toBe(true); + }); + + test("a row that got shorter and was pulled back to its new end is not the user's, and the strip is known to be there", () => { + const pulled = stripScrolled(placed, 200, 200); + expect(pulled).toEqual({ at: 200, moved: false }); + expect(stripScrolled(pulled, 200, 200).moved).toBe(false); + expect(stripScrolled(pulled, 120, 200).moved).toBe(true); + }); + + test("the user's scroll stays theirs through the strip's later events, until another tab is opened", () => { + let strip = stripScrolled(placed, 0, 600); + strip = stripScrolled(strip, 240, 600); + expect(strip.moved).toBe(true); + strip = stripPlaced(strip, 300); + expect(strip.moved).toBe(true); + strip = stripPlaced(stripSelected(strip), 480); + expect(strip).toEqual({ at: 480, moved: false }); + expect(stripScrolled(strip, 480, 600).moved).toBe(false); + }); + + test("a tab opened where the strip already showed it: the user's next scroll is still seen", () => { + // scrolled to the end, a tab in view there opened (nothing to place), then scrolled back + const opened = stripSelected(stripScrolled(stripPlaced(STRIP_AT_REST, 0), 600, 600)); + expect(opened).toEqual({ at: 600, moved: false }); + expect(stripScrolled(opened, 0, 600).moved).toBe(true); + }); +}); diff --git a/src/lib/tabStripScroll.ts b/src/lib/tabStripScroll.ts new file mode 100644 index 0000000000..4571fcc95d --- /dev/null +++ b/src/lib/tabStripScroll.ts @@ -0,0 +1,33 @@ +/** Where the tab strip is known to be, and whether the user has scrolled it themselves. */ +export interface StripScroll { + /** The `scrollLeft` the strip was last put at or found at; null before either. */ + readonly at: number | null; + /** The user scrolled the strip themselves, and no other tab has been opened since. */ + readonly moved: boolean; +} + +export const STRIP_AT_REST: StripScroll = { at: null, moved: false }; + +/** + * The strip after a `scroll` event that found it at `left`, of `max` it can scroll. No input is + * listened for: a wheel, a drag, its momentum, a key and a focus that scrolls all end in this one + * event, and so does the strip's own placing. They are told apart by where the strip is: its own + * scroll finds it exactly where it was put (`stripPlaced` has the value the browser kept), and a + * shorter row (a face that drew the names narrower, a tab renamed or closed) that the browser + * pulled back is at its new end, short of where it was. Anywhere else, the user took it there. + */ +export function stripScrolled(was: StripScroll, left: number, max: number): StripScroll { + if (left === was.at) return was; + if (was.at !== null && left < was.at && left >= max - 1) return { at: left, moved: was.moved }; + return { at: left, moved: true }; +} + +/** The strip once it has put itself at `left`, as read back from it. */ +export function stripPlaced(was: StripScroll, left: number): StripScroll { + return { at: left, moved: was.moved }; +} + +/** A tab was opened: the strip follows the open tab again, whatever the user did before. */ +export function stripSelected(was: StripScroll): StripScroll { + return was.moved ? { at: was.at, moved: false } : was; +} From d33f26679e367dc54c6faa6061af708e904f953b Mon Sep 17 00:00:00 2001 From: Hako Date: Tue, 6 Oct 2026 01:08:01 +0900 Subject: [PATCH 2/2] docs(changelog): link #479 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 22c7227624..20c0a1ecc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ between releases do not reach them. Remote-PC runtime bundles are versioned sepa at another width, and the tab strip then brings the open tab back into view, unless you have scrolled the strip yourself to look at other tabs: it stays where you left it until you open a tab. - ([#473](https://github.com/devswha/herdr-web-ui/pull/473)) + ([#473](https://github.com/devswha/herdr-web-ui/pull/473), [#479](https://github.com/devswha/herdr-web-ui/pull/479)) - In the chat's message box the agent's mark, the model, the reasoning level and the context ring sit together in one quiet pill, and the model is shown by its name where the id is a regular one: `claude-opus-5-5` reads **Opus 5.5**, `claude-sonnet-5` **Sonnet 5**, `gpt-5.6`