From 07941681821fc9b3661c385304b00d3f9ae8dacf Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 20:10:30 +0000 Subject: [PATCH] test(console-ai): pin #2477 items 2+3, already shipped by #2478 but unpinned Premise check for issue #2477 items 2 and 3 found both already implemented by PR #2478 (merged 2026-07-14, ~22 min after the card was filed): - item 2: StudioChatDock DOES pass persistExpandedKey (its own DOCK_STUDIO_EXPANDED_STORAGE_KEY), so a collapse survives pillar/package switches and Studio re-entry. - item 3: the folded layout's side-by-side threshold IS xl (1280), not 2xl. Neither carried a regression pin, so reverting either shipped fix kept the whole suite green. This adds the pins and corrects the doc comments left describing pre-#2478 behaviour. No runtime behaviour change. - Extract WIDE_VIEWPORT_BREAKPOINT + useIsWideViewport into ./wideViewport (same testability motive as the existing ./centerTab extraction) and pin the 1280 boundary, the 1280-1512 laptop band, and the below-xl tab fallback. - Pin the StudioChatDock -> useChatDockState wiring: the storage helpers were tested but the call passing them was not, so deleting persistExpandedKey left chatDockReturnLocation.test.tsx fully green. - Pin cross-dock non-interference in both directions (the hazard the card names). - Fix three stale doc comments that still claimed the Studio dock was NOT persisted / that the folded layout went side-by-side at 2xl. Refs #2477 --- .changeset/studio-dock-polish-pins.md | 10 ++ packages/app-shell/src/layout/ChatDock.tsx | 5 +- .../__tests__/chatDockReturnLocation.test.tsx | 20 +++ .../views/studio-design/StudioAiCopilot.tsx | 6 +- .../studio-design/StudioDesignSurface.tsx | 35 ++--- .../studioChatDockPersistence.test.tsx | 121 ++++++++++++++++++ .../__tests__/wideViewport.test.tsx | 71 ++++++++++ .../src/views/studio-design/wideViewport.ts | 53 ++++++++ 8 files changed, 290 insertions(+), 31 deletions(-) create mode 100644 .changeset/studio-dock-polish-pins.md create mode 100644 packages/app-shell/src/views/studio-design/__tests__/studioChatDockPersistence.test.tsx create mode 100644 packages/app-shell/src/views/studio-design/__tests__/wideViewport.test.tsx create mode 100644 packages/app-shell/src/views/studio-design/wideViewport.ts diff --git a/.changeset/studio-dock-polish-pins.md b/.changeset/studio-dock-polish-pins.md new file mode 100644 index 0000000000..8431cb832b --- /dev/null +++ b/.changeset/studio-dock-polish-pins.md @@ -0,0 +1,10 @@ +--- +--- + +Internal only — no user-visible change, so no release. + +ADR-0057 / issue #2477 items 2 and 3 (Studio dock collapse persistence, folded +layout side-by-side at `xl`) were already implemented by PR #2478; this adds the +regression pins that PR shipped without, and corrects three doc comments that +still described the pre-#2478 behaviour. Tests, comments, and the extraction of +the breakpoint constant into its own module — no runtime behaviour changes. diff --git a/packages/app-shell/src/layout/ChatDock.tsx b/packages/app-shell/src/layout/ChatDock.tsx index 95e3034f25..27f034976c 100644 --- a/packages/app-shell/src/layout/ChatDock.tsx +++ b/packages/app-shell/src/layout/ChatDock.tsx @@ -64,8 +64,9 @@ export interface ChatDockOptions { /** * sessionStorage key the expanded/collapsed state round-trips through (see * {@link DOCK_EXPANDED_STORAGE_KEY}). Omitted → in-memory only, so each mount - * starts from `defaultExpanded` (the Studio dock's parity with today's - * non-persisted copilot collapse). + * starts from `defaultExpanded`. Both shipped docks DO pass a key, and they + * pass DIFFERENT ones on purpose — console and Studio must never share one + * collapse (#2477 item 2). */ persistExpandedKey?: string; /** diff --git a/packages/app-shell/src/layout/__tests__/chatDockReturnLocation.test.tsx b/packages/app-shell/src/layout/__tests__/chatDockReturnLocation.test.tsx index 8177e21ae2..e16af4471b 100644 --- a/packages/app-shell/src/layout/__tests__/chatDockReturnLocation.test.tsx +++ b/packages/app-shell/src/layout/__tests__/chatDockReturnLocation.test.tsx @@ -76,4 +76,24 @@ describe('stored-expanded round trip (issue #2477 item 2 — Studio remembers a writeStoredDockExpanded(DOCK_EXPANDED_STORAGE_KEY, false); expect(readStoredDockExpanded(DOCK_EXPANDED_STORAGE_KEY, false)).toBe(false); }); + + it('the two docks never fight: a collapse on either leaves the other alone', () => { + // The hazard the card names by name — one shared flag would make collapsing + // the Studio copilot also collapse the console rail (and vice versa). + expect(DOCK_STUDIO_EXPANDED_STORAGE_KEY).not.toBe(DOCK_EXPANDED_STORAGE_KEY); + + // Studio → console: collapsing Studio must not touch the console's flag, + // which stays unset and so keeps falling back to the console's own default. + writeStoredDockExpanded(KEY, false); + expect(window.sessionStorage.getItem(DOCK_EXPANDED_STORAGE_KEY)).toBeNull(); + expect(readStoredDockExpanded(DOCK_EXPANDED_STORAGE_KEY, false)).toBe(false); + expect(readStoredDockExpanded(DOCK_EXPANDED_STORAGE_KEY, true)).toBe(true); + + // console → Studio: the console rail expanding (e.g. armChatDockExpanded + // before navigating back) must not re-expand a Studio dock the user + // deliberately collapsed. + armChatDockExpanded(); + expect(readStoredDockExpanded(DOCK_EXPANDED_STORAGE_KEY, false)).toBe(true); + expect(readStoredDockExpanded(KEY, true)).toBe(false); + }); }); diff --git a/packages/app-shell/src/views/studio-design/StudioAiCopilot.tsx b/packages/app-shell/src/views/studio-design/StudioAiCopilot.tsx index 53c5d81b33..bb7c3a6698 100644 --- a/packages/app-shell/src/views/studio-design/StudioAiCopilot.tsx +++ b/packages/app-shell/src/views/studio-design/StudioAiCopilot.tsx @@ -118,8 +118,10 @@ export interface StudioChatDockProps { * the console rail. * * Differences from the console rail, on purpose: - * - Default EXPANDED (the copilot has always been visible by default) and - * NOT persisted (parity with the old panel's in-memory collapse). + * - Default EXPANDED (the copilot has always been visible by default), but a + * collapse IS remembered — per-tab, under this surface's own key (#2477 + * item 2). The old panel's in-memory collapse re-opened on every pillar / + * package switch; only the first-visit posture is still "expanded". * - Collapsed state = the {@link ChatDockLauncher} edge button — Studio has * no FAB to double as the launcher. * - Maximize opens `/ai/build?package=…`, which resumes THIS thread (the diff --git a/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx b/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx index 938f8cf2d8..76e6bdf59b 100644 --- a/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx +++ b/packages/app-shell/src/views/studio-design/StudioDesignSurface.tsx @@ -19,6 +19,7 @@ import { useParams, useNavigate, useSearchParams, Link } from 'react-router-dom' import { useAdapter, SchemaRendererProvider } from '@object-ui/react'; import { StudioChatDock } from './StudioAiCopilot'; import { nextCenterTab, type StudioCenterTab } from './centerTab'; +import { useIsWideViewport } from './wideViewport'; import { GridFieldAuthoringProvider, cn, @@ -116,31 +117,10 @@ import { DraftChangesPanel } from '../../preview/DraftChangesPanel'; import { resolveConsoleUrl } from '../../console/organizations/resolveHomeUrl'; import { toast } from 'sonner'; -/** - * ADR-0057 P3c follow-up — is the viewport wide enough (Tailwind `xl`) to show - * the folded layout's canvas AND properties side by side beside the chat dock, - * instead of as tabs? Mirrors `useIsMobile`'s matchMedia idiom. - * - * `xl` (1280), lowered from `2xl` per issue #2477 item 3: the common laptop - * (1280–1512) was falling into tabs, which auto-hide the canvas the moment you - * select a block — breaking the WYSIWYG "edit and watch it apply" loop. At - * 1280 the canvas is narrow (~360px after the nav rail, a slimmed side-by-side - * inspector, and the ~420px dock) but LIVE, which beats hidden; at 1440+ it is - * comfortable. Below `xl` the tabs remain (there simply isn't room for three - * columns plus chat). - */ -const WIDE_VIEWPORT_BREAKPOINT = 1280; -function useIsWideViewport(): boolean { - const [isWide, setIsWide] = React.useState(undefined); - React.useEffect(() => { - const mql = window.matchMedia(`(min-width: ${WIDE_VIEWPORT_BREAKPOINT}px)`); - const onChange = () => setIsWide(window.innerWidth >= WIDE_VIEWPORT_BREAKPOINT); - mql.addEventListener('change', onChange); - setIsWide(window.innerWidth >= WIDE_VIEWPORT_BREAKPOINT); - return () => mql.removeEventListener('change', onChange); - }, []); - return !!isWide; -} +// ADR-0057 P3c follow-up (#2477 item 3) — the folded layout's side-by-side +// threshold (`xl`, not `2xl`) and its matchMedia hook live in ./wideViewport so +// the rule is testable without mounting this surface; see that module for why +// 1280 is the right line and what the canvas measures there. const PILLARS: ReadonlyArray<{ key: string; label: string; Icon: LucideIcon }> = [ { key: 'data', label: 'Data', Icon: Database }, @@ -1158,8 +1138,9 @@ function InterfacesPillar({ // to Properties; deselect → back to Canvas) while preserving a manual choice // in steady state — see nextCenterTab. Inert when `foldInspector` is off. const [centerTab, setCenterTab] = React.useState('canvas'); - // Folded layout, wide viewport (2xl+): enough room to show canvas AND - // properties side by side beside the chat dock — no tabs, no auto-switch. + // Folded layout, wide viewport (xl+, #2477 item 3 — was 2xl): enough room to + // show canvas AND properties side by side beside the chat dock — no tabs, no + // auto-switch. const isWide = useIsWideViewport(); const showFoldedTabs = foldInspector && !isWide; // The right-hand properties aside can be collapsed to a thin rail to give the diff --git a/packages/app-shell/src/views/studio-design/__tests__/studioChatDockPersistence.test.tsx b/packages/app-shell/src/views/studio-design/__tests__/studioChatDockPersistence.test.tsx new file mode 100644 index 0000000000..f51c2500ba --- /dev/null +++ b/packages/app-shell/src/views/studio-design/__tests__/studioChatDockPersistence.test.tsx @@ -0,0 +1,121 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * ADR-0057 P3c follow-up — issue #2477 item 2: the Studio right dock remembers + * a collapse instead of re-expanding on every pillar switch / package change / + * Studio re-entry, under a key SCOPED to this surface so it never fights the + * console dock's own state. + * + * The behaviour shipped in PR #2478, but only its storage HELPERS were pinned + * (`layout/__tests__/chatDockReturnLocation.test.tsx`). The wiring that makes + * them matter — StudioChatDock actually passing its own keys into + * `useChatDockState` — was untested, so deleting `persistExpandedKey` (the one + * line that fixes the card's complaint) left every test green. This pins it. + */ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { render } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; +import type { ChatDockOptions } from '../../../layout/ChatDock'; +import { + DOCK_EXPANDED_STORAGE_KEY, + DOCK_STUDIO_EXPANDED_STORAGE_KEY, + DOCK_STUDIO_WIDTH_STORAGE_KEY, + DOCK_WIDTH_STORAGE_KEY, +} from '../../../layout/chatDockState'; + +// Every option object StudioChatDock hands to useChatDockState, in order. +const dockOptions: Array = []; + +vi.mock('../../../layout/ChatDock', async (importOriginal) => { + const actual = await importOriginal>(); + return { + ...actual, + // Record the options, then defer to the REAL hook so the recorded wiring is + // the same call the shipped dock actually makes (a stub would let the two + // drift apart silently). + useChatDockState: (options?: ChatDockOptions) => { + dockOptions.push(options); + return (actual.useChatDockState as (o?: ChatDockOptions) => unknown)(options); + }, + ChatDockPanel: () => null, + ChatDockLauncher: () => null, + ChatDockMobileSheet: () => null, + }; +}); + +// The dock's body and its data seams are irrelevant here — only the state +// wiring is under test. One agent so the empty-catalog gate lets it render. +vi.mock('@object-ui/plugin-chatbot', () => ({ + useAgents: () => ({ agents: [{ name: 'build' }], isLoading: false, error: null }), +})); +// Partial — the module also feeds the metadata client factory, which imports +// `createAuthenticatedFetch` from it at load time. +vi.mock('@object-ui/auth', async (importOriginal) => ({ + ...(await importOriginal>()), + useAuth: () => ({ user: { id: 'u1' } }), +})); +vi.mock('../../../console/ai/AiChatPage', () => ({ + ChatPane: () => null, + resolveApiBase: () => '/api/v1', +})); +vi.mock('../../../hooks/useChatConversation', () => ({ + useChatConversation: () => ({ conversationId: 'c1', initialMessages: [] }), +})); + +import { StudioChatDock } from '../StudioAiCopilot'; + +beforeEach(() => { + dockOptions.length = 0; + window.sessionStorage.clear(); + window.localStorage.clear(); +}); + +/** + * Mount the dock and return the options it passed to `useChatDockState`. React + * re-renders (and StrictMode double-invokes), so assert the wiring is STABLE + * across every call rather than pinning a call count — a per-render-varying key + * would be its own bug, and this catches it. + */ +function mountStudioDock(): ChatDockOptions | undefined { + render( + + + , + ); + expect(dockOptions.length).toBeGreaterThan(0); + for (const options of dockOptions) { + expect(options).toEqual(dockOptions[0]); + } + return dockOptions[0]; +} + +describe('StudioChatDock state wiring (#2477 item 2)', () => { + it('persists its expanded state under the STUDIO key, not the console one', () => { + const options = mountStudioDock(); + // The fix itself: without a persist key the collapse is in-memory and the + // dock re-expands on every pillar switch — the card's actual complaint. + expect(options?.persistExpandedKey).toBe(DOCK_STUDIO_EXPANDED_STORAGE_KEY); + // Scoped: sharing the console's key would make the two docks one collapse. + expect(options?.persistExpandedKey).not.toBe(DOCK_EXPANDED_STORAGE_KEY); + }); + + it('keeps the copilot-visible first-visit posture (default expanded)', () => { + expect(mountStudioDock()?.defaultExpanded).toBe(true); + }); + + it('sizes itself under the STUDIO width key so the console cannot squeeze the canvas', () => { + const options = mountStudioDock(); + expect(options?.persistWidthKey).toBe(DOCK_STUDIO_WIDTH_STORAGE_KEY); + expect(options?.persistWidthKey).not.toBe(DOCK_WIDTH_STORAGE_KEY); + }); + + it('a console collapse flag does not collapse the Studio dock', () => { + // Cross-surface non-interference, the direction the card names: the console + // dock storing '0' must leave Studio on its own default (expanded), and must + // not have written anything under the Studio key. + window.sessionStorage.setItem(DOCK_EXPANDED_STORAGE_KEY, '0'); + expect(mountStudioDock()?.defaultExpanded).toBe(true); + expect(window.sessionStorage.getItem(DOCK_STUDIO_EXPANDED_STORAGE_KEY)).toBeNull(); + }); +}); diff --git a/packages/app-shell/src/views/studio-design/__tests__/wideViewport.test.tsx b/packages/app-shell/src/views/studio-design/__tests__/wideViewport.test.tsx new file mode 100644 index 0000000000..eee6614353 --- /dev/null +++ b/packages/app-shell/src/views/studio-design/__tests__/wideViewport.test.tsx @@ -0,0 +1,71 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * ADR-0057 P3c follow-up — issue #2477 item 3: the Studio folded layout goes + * side-by-side (canvas BESIDE properties) from `xl` (1280), not `2xl` (1536). + * + * This is the regression pin the original fix (PR #2478) shipped without: the + * breakpoint lived as a module-private constant inside StudioDesignSurface, so + * raising it back to 1536 — which re-hides the canvas behind the tab + * auto-switch on every 1280–1512 laptop, the exact defect the card reported — + * kept the whole suite green. `.test.tsx` for the DOM (matchMedia + innerWidth). + */ +import { describe, it, expect, afterEach } from 'vitest'; +import { renderHook } from '@testing-library/react'; +import { WIDE_VIEWPORT_BREAKPOINT, useIsWideViewport } from '../wideViewport'; + +const realInnerWidth = window.innerWidth; + +function setViewportWidth(px: number) { + Object.defineProperty(window, 'innerWidth', { + value: px, + configurable: true, + writable: true, + }); +} + +afterEach(() => { + setViewportWidth(realInnerWidth); +}); + +describe('WIDE_VIEWPORT_BREAKPOINT (#2477 item 3 — xl, not 2xl)', () => { + it('is Tailwind xl (1280) so the common laptop keeps the canvas visible', () => { + // The card's whole complaint: at 2xl the 1280–1512 band tabbed, and the tab + // auto-switch hides the canvas the moment a block is selected. + expect(WIDE_VIEWPORT_BREAKPOINT).toBe(1280); + expect(WIDE_VIEWPORT_BREAKPOINT).not.toBe(1536); + }); +}); + +describe('useIsWideViewport', () => { + it('is wide at exactly the breakpoint (inclusive boundary)', () => { + setViewportWidth(WIDE_VIEWPORT_BREAKPOINT); + const { result } = renderHook(() => useIsWideViewport()); + expect(result.current).toBe(true); + }); + + it('is wide across the common laptop band the card named (1280–1512)', () => { + for (const width of [1280, 1366, 1440, 1512]) { + setViewportWidth(width); + const { result, unmount } = renderHook(() => useIsWideViewport()); + expect(result.current, `${width}px should be side-by-side`).toBe(true); + unmount(); + } + }); + + it('is NOT wide one pixel below the breakpoint — the tabs stay below xl', () => { + setViewportWidth(WIDE_VIEWPORT_BREAKPOINT - 1); + const { result } = renderHook(() => useIsWideViewport()); + expect(result.current).toBe(false); + }); + + it('keeps tabs on the narrow viewports that genuinely lack room', () => { + for (const width of [768, 1024, 1152]) { + setViewportWidth(width); + const { result, unmount } = renderHook(() => useIsWideViewport()); + expect(result.current, `${width}px should still tab`).toBe(false); + unmount(); + } + }); +}); diff --git a/packages/app-shell/src/views/studio-design/wideViewport.ts b/packages/app-shell/src/views/studio-design/wideViewport.ts new file mode 100644 index 0000000000..b1611cc005 --- /dev/null +++ b/packages/app-shell/src/views/studio-design/wideViewport.ts @@ -0,0 +1,53 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * ADR-0057 P3c follow-up — the Studio folded layout's side-by-side threshold, + * extracted from StudioDesignSurface for the same reason {@link + * ./centerTab} was: the surface itself is far too heavy to mount just to pin a + * breakpoint, so the rule that decides "canvas beside properties" vs "canvas OR + * properties as tabs" lives here and is directly testable. + * + * The value is deliberately `xl` (1280) and NOT `2xl` (1536) — issue #2477 + * item 3. At `2xl` the common laptop (1280–1512) fell into the tab layout, + * whose auto-switch hides the canvas the moment you select a block, breaking + * the WYSIWYG "edit it and watch it apply" loop that is the whole point of the + * designer. Measured at the 1280 boundary with the chat dock expanded at its + * 420px default: nav rail 208 (`w-52`) + inspector 288 (`w-72`) + dock 420 + * leaves the canvas ~364px — narrow, but LIVE, which beats hidden; collapsing + * the dock (a preference that now sticks, item 2) hands the canvas 784px back, + * and at 1440+ it is comfortable without touching the dock. Below `xl` the tabs + * remain: there is genuinely no room for three columns plus chat. + */ +import * as React from 'react'; + +/** + * Minimum viewport width (px) for the folded layout's side-by-side canvas + + * properties. Tailwind's `xl`. Changing this back to 1536 (`2xl`) reintroduces + * the hidden-canvas regression above — the pin in `__tests__/wideViewport.test.tsx` + * exists to make that a red test rather than a silent UX loss. + */ +export const WIDE_VIEWPORT_BREAKPOINT = 1280; + +/** + * Is the viewport at least {@link WIDE_VIEWPORT_BREAKPOINT} wide? Mirrors + * `useIsMobile`'s matchMedia idiom: subscribe to the query, but read the + * authoritative width from `window.innerWidth` so the boundary is inclusive + * (exactly 1280 IS wide) and consistent across the initial read and updates. + * + * Starts `undefined` and resolves in an effect — server/first paint has no + * viewport, and coercing to `false` there keeps the narrow (tabs) layout as the + * pre-measurement default rather than flashing a three-column layout it may not + * have room for. + */ +export function useIsWideViewport(): boolean { + const [isWide, setIsWide] = React.useState(undefined); + React.useEffect(() => { + const mql = window.matchMedia(`(min-width: ${WIDE_VIEWPORT_BREAKPOINT}px)`); + const onChange = () => setIsWide(window.innerWidth >= WIDE_VIEWPORT_BREAKPOINT); + mql.addEventListener('change', onChange); + setIsWide(window.innerWidth >= WIDE_VIEWPORT_BREAKPOINT); + return () => mql.removeEventListener('change', onChange); + }, []); + return !!isWide; +}