From c52bc5ec95fd4fef27736a903b4beb00333eacde Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sun, 26 Jul 2026 03:05:30 -0500 Subject: [PATCH] fix(desktop): scope measured-height vars to each chat surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The composer and the out-of-flow status stack published their measured heights onto document.documentElement, but both components mount once per chat surface. Session tiles render a full ChatView beside the workspace pane, so N surfaces raced for one value: a background tab with a tall status stack inflated the foreground thread's bottom clearance and pushed its jump-to-bottom button into mid-screen. Whichever surface unmounted last also cleared the var for everyone still showing. Publish onto the surface's own root instead, and re-declare the clearance calc there — `:root` substitutes the root measurements once, so scoping only the inputs would leave every thread reading the same value. --- .../composer/hooks/use-composer-metrics.ts | 25 ++++--- .../app/chat/composer/status-stack/index.tsx | 11 +-- apps/desktop/src/app/chat/index.tsx | 1 + .../desktop/src/app/chat/surface-vars.test.ts | 68 +++++++++++++++++++ apps/desktop/src/app/chat/surface-vars.ts | 42 ++++++++++++ apps/desktop/src/styles.css | 14 +++- 6 files changed, 146 insertions(+), 15 deletions(-) create mode 100644 apps/desktop/src/app/chat/surface-vars.test.ts create mode 100644 apps/desktop/src/app/chat/surface-vars.ts diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-metrics.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-metrics.ts index 318802bbab80..228758dd9056 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-metrics.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-metrics.ts @@ -1,6 +1,12 @@ import { useAuiState } from '@assistant-ui/react' import { type RefObject, useCallback, useEffect, useRef, useState } from 'react' +import { + clearSurfaceVar, + COMPOSER_HEIGHT_VAR, + COMPOSER_SURFACE_HEIGHT_VAR, + setSurfaceVar +} from '@/app/chat/surface-vars' import { useMediaQuery } from '@/hooks/use-media-query' import { useResizeObserver } from '@/hooks/use-resize-observer' import { $composerPoppedOut } from '@/store/composer-popout' @@ -89,18 +95,16 @@ export function useComposerMetrics({ composerRef, composerSurfaceRef, editorRef, // (Read globals here so the callback stays stable; mirror the popoutAllowed // gate since secondary windows are forced docked.) if ($composerPoppedOut.get() && !isSecondaryWindow()) { - const root = document.documentElement lastBucketedHeightRef.current = 0 lastBucketedSurfaceHeightRef.current = 0 - root.style.setProperty('--composer-measured-height', '0px') - root.style.setProperty('--composer-surface-measured-height', '0px') + setSurfaceVar(composer, COMPOSER_HEIGHT_VAR, '0px') + setSurfaceVar(composer, COMPOSER_SURFACE_HEIGHT_VAR, '0px') return } const { height, width } = composer.getBoundingClientRect() const surfaceHeight = composerSurfaceRef.current?.getBoundingClientRect().height - const root = document.documentElement if (width > 0) { const nextTight = width < COMPOSER_STACK_BREAKPOINT_PX @@ -135,7 +139,7 @@ export function useComposerMetrics({ composerRef, composerSurfaceRef, editorRef, if (bucket !== lastBucketedHeightRef.current) { lastBucketedHeightRef.current = bucket - root.style.setProperty('--composer-measured-height', `${bucket}px`) + setSurfaceVar(composer, COMPOSER_HEIGHT_VAR, `${bucket}px`) } } @@ -144,7 +148,7 @@ export function useComposerMetrics({ composerRef, composerSurfaceRef, editorRef, if (bucket !== lastBucketedSurfaceHeightRef.current) { lastBucketedSurfaceHeightRef.current = bucket - root.style.setProperty('--composer-surface-measured-height', `${bucket}px`) + setSurfaceVar(composer, COMPOSER_SURFACE_HEIGHT_VAR, `${bucket}px`) } } }, [composerRef, composerSurfaceRef, editorRef]) @@ -160,12 +164,13 @@ export function useComposerMetrics({ composerRef, composerSurfaceRef, editorRef, }, [poppedOut, syncComposerMetrics]) useEffect(() => { + const composer = composerRef.current + return () => { - const root = document.documentElement - root.style.removeProperty('--composer-measured-height') - root.style.removeProperty('--composer-surface-measured-height') + clearSurfaceVar(composer, COMPOSER_HEIGHT_VAR) + clearSurfaceVar(composer, COMPOSER_SURFACE_HEIGHT_VAR) } - }, []) + }, [composerRef]) // Pill compacts on real width (tile/pane), OR when stacked for any reason // (viewport-narrow / wrapped) so the controls row never over-runs. diff --git a/apps/desktop/src/app/chat/composer/status-stack/index.tsx b/apps/desktop/src/app/chat/composer/status-stack/index.tsx index 739080be8b85..8c779bbde558 100644 --- a/apps/desktop/src/app/chat/composer/status-stack/index.tsx +++ b/apps/desktop/src/app/chat/composer/status-stack/index.tsx @@ -3,6 +3,7 @@ import { type ReactNode, useEffect, useLayoutEffect, useMemo, useRef } from 'rea import { useNavigate } from 'react-router-dom' import { blurComposerInput } from '@/app/chat/composer/focus' +import { clearSurfaceVar, setSurfaceVar, STATUS_STACK_VAR } from '@/app/chat/surface-vars' import { AGENTS_ROUTE } from '@/app/routes' import { BillingBanner } from '@/components/billing-banner' import { composerDockCard } from '@/components/chat/composer-dock' @@ -197,12 +198,14 @@ export function ComposerStatusStack({ queue, sessionId }: ComposerStatusStackPro // height never sees it. Publish our own measured height — bucketed like the // composer's, to avoid style invalidation churn — so the thread's // last-message clearance can add it and the stack never hides messages. + // Scoped to THIS chat surface: tiles render their own stack beside the + // workspace pane, and a shared global would let the taller one dictate every + // thread's padding (see surface-vars.ts). useLayoutEffect(() => { - const root = document.documentElement const el = stackRef.current if (!visible || !el) { - root.style.removeProperty('--status-stack-measured-height') + clearSurfaceVar(stackRef.current, STATUS_STACK_VAR) return } @@ -214,7 +217,7 @@ export function ComposerStatusStack({ queue, sessionId }: ComposerStatusStackPro if (bucket !== last) { last = bucket - root.style.setProperty('--status-stack-measured-height', `${bucket}px`) + setSurfaceVar(el, STATUS_STACK_VAR, `${bucket}px`) } } @@ -224,7 +227,7 @@ export function ComposerStatusStack({ queue, sessionId }: ComposerStatusStackPro return () => { observer.disconnect() - root.style.removeProperty('--status-stack-measured-height') + clearSurfaceVar(el, STATUS_STACK_VAR) } }, [visible]) diff --git a/apps/desktop/src/app/chat/index.tsx b/apps/desktop/src/app/chat/index.tsx index 7274a9be4b88..5fdf5a0487b1 100644 --- a/apps/desktop/src/app/chat/index.tsx +++ b/apps/desktop/src/app/chat/index.tsx @@ -470,6 +470,7 @@ export function ChatView({ 'relative isolate flex h-full min-w-0 flex-col overflow-hidden bg-(--ui-chat-surface-background)', className )} + data-chat-surface="" data-composer-target={composerScope.target} data-session-anchor={sessionAnchor} > diff --git a/apps/desktop/src/app/chat/surface-vars.test.ts b/apps/desktop/src/app/chat/surface-vars.test.ts new file mode 100644 index 000000000000..41b7c8ad3f3a --- /dev/null +++ b/apps/desktop/src/app/chat/surface-vars.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from 'vitest' + +import { chatSurfaceRoot, clearSurfaceVar, setSurfaceVar, STATUS_STACK_VAR } from './surface-vars' + +/** + * Measured-height vars must be scoped per chat surface. Session tiles render a + * full ChatView — thread, composer, and status stack — beside the workspace + * pane, so a single global slot let a background tab's tall status stack + * inflate the foreground thread's bottom clearance and shove its + * jump-to-bottom button into mid-screen. + */ +function surface(): { root: HTMLElement; stack: HTMLElement } { + const root = document.createElement('div') + root.setAttribute('data-chat-surface', '') + + const stack = document.createElement('div') + root.append(stack) + document.body.append(root) + + return { root, stack } +} + +describe('per-surface measured-height vars', () => { + it('publishes onto the owning chat surface, not the document root', () => { + const { root, stack } = surface() + + setSurfaceVar(stack, STATUS_STACK_VAR, '96px') + + expect(root.style.getPropertyValue(STATUS_STACK_VAR)).toBe('96px') + expect(document.documentElement.style.getPropertyValue(STATUS_STACK_VAR)).toBe('') + }) + + it('keeps two visible surfaces independent', () => { + const background = surface() + const foreground = surface() + + setSurfaceVar(background.stack, STATUS_STACK_VAR, '160px') + setSurfaceVar(foreground.stack, STATUS_STACK_VAR, '0px') + + expect(background.root.style.getPropertyValue(STATUS_STACK_VAR)).toBe('160px') + expect(foreground.root.style.getPropertyValue(STATUS_STACK_VAR)).toBe('0px') + }) + + it('clearing one surface leaves the other intact', () => { + const a = surface() + const b = surface() + + setSurfaceVar(a.stack, STATUS_STACK_VAR, '48px') + setSurfaceVar(b.stack, STATUS_STACK_VAR, '80px') + clearSurfaceVar(a.stack, STATUS_STACK_VAR) + + expect(a.root.style.getPropertyValue(STATUS_STACK_VAR)).toBe('') + expect(b.root.style.getPropertyValue(STATUS_STACK_VAR)).toBe('80px') + }) + + it('falls back to the document root outside a chat surface (popped-out composer)', () => { + const orphan = document.createElement('div') + document.body.append(orphan) + + expect(chatSurfaceRoot(orphan)).toBeNull() + + setSurfaceVar(orphan, STATUS_STACK_VAR, '24px') + expect(document.documentElement.style.getPropertyValue(STATUS_STACK_VAR)).toBe('24px') + + clearSurfaceVar(orphan, STATUS_STACK_VAR) + expect(document.documentElement.style.getPropertyValue(STATUS_STACK_VAR)).toBe('') + }) +}) diff --git a/apps/desktop/src/app/chat/surface-vars.ts b/apps/desktop/src/app/chat/surface-vars.ts new file mode 100644 index 000000000000..dfe9df967971 --- /dev/null +++ b/apps/desktop/src/app/chat/surface-vars.ts @@ -0,0 +1,42 @@ +/** + * Measured-height CSS vars, scoped to one chat surface. + * + * The composer and the out-of-flow status stack both publish their measured + * height so the thread can reserve bottom clearance for them. Those vars used + * to be written to `document.documentElement` — a single global slot — while + * the components writing them mount once per chat surface. Session tiles + * render a full ChatView (thread + composer + status stack) beside the + * workspace pane, so N surfaces raced for one value: a background tab with a + * tall stack inflated the foreground thread's padding and pushed its + * jump-to-bottom button into mid-screen. + * + * Publishing onto the surface's own root element instead makes each ChatView + * read its own measurement through normal CSS inheritance. Remeasuring on + * session switch cannot fix this — several surfaces are visible at once, so + * there is no single correct value to remeasure to. + */ + +/** Nearest enclosing chat surface, or `null` when the node is detached. */ +export function chatSurfaceRoot(el: Element | null): HTMLElement | null { + return el?.closest('[data-chat-surface]') ?? null +} + +export const COMPOSER_HEIGHT_VAR = '--composer-measured-height' +export const COMPOSER_SURFACE_HEIGHT_VAR = '--composer-surface-measured-height' +export const STATUS_STACK_VAR = '--status-stack-measured-height' + +/** + * Set a measured-height var on the surface owning `el`. Falls back to the + * document root so a composer rendered outside a chat surface (the popped-out + * window) keeps behaving exactly as before. + */ +export function setSurfaceVar(el: Element | null, name: string, value: string): void { + const target = chatSurfaceRoot(el) ?? document.documentElement + target.style.setProperty(name, value) +} + +/** Clear a measured-height var from the surface owning `el`. */ +export function clearSurfaceVar(el: Element | null, name: string): void { + const target = chatSurfaceRoot(el) ?? document.documentElement + target.style.removeProperty(name) +} diff --git a/apps/desktop/src/styles.css b/apps/desktop/src/styles.css index 2bd50d7e0ebb..0b5ed5c70cbf 100644 --- a/apps/desktop/src/styles.css +++ b/apps/desktop/src/styles.css @@ -347,7 +347,9 @@ --radius-scalar: 0.6; /* Space under last message vs overlay composer — driven by the measured composer height (see composer/index.tsx) - plus the out-of-flow status stack's measured height (see status-stack/index.tsx) when one is showing. */ + plus the out-of-flow status stack's measured height (see status-stack/index.tsx) when one is showing. + Both inputs are republished per chat surface (see surface-vars.ts), and the calc is re-declared there so + each surface resolves against its OWN measurements — a tile's tall stack must not pad another thread. */ --status-stack-measured-height: 0px; --thread-last-message-clearance: calc(var(--composer-measured-height) + var(--status-stack-measured-height) + 2rem); @@ -1467,6 +1469,16 @@ text-* variant utilities. */ .btn-arc { height: 0.875rem; } +/* Re-declare the clearance calc on every chat surface. `:root` computes it + once against the ROOT measurements, so scoping only the inputs would leave + every thread reading the same substituted value. Redeclaring here makes each + surface resolve the calc against its own composer + status-stack heights, + falling back to the root defaults until this surface publishes its first + measurement. See surface-vars.ts. */ +[data-chat-surface] { + --thread-last-message-clearance: calc(var(--composer-measured-height) + var(--status-stack-measured-height) + 2rem); +} + /* Long adjacent tool-call run collapsed into a fixed, auto-scrolling window. ToolGroupSlot pins the newest call to the bottom (unless the user scrolls up), so a back-to-back run stays compact instead of pushing the reply off