diff --git a/e2e/tests/header.spec.js b/e2e/tests/header.spec.js index 6ea2d60a..68a8a6b7 100644 --- a/e2e/tests/header.spec.js +++ b/e2e/tests/header.spec.js @@ -163,22 +163,56 @@ test('HDR-5: the "Your diagrams" breadcrumb returns to the hub', async ({ }); // ────────────────────────────────────────────────────────────────────────────── -// HDR-2: title edit Escape cancels / reverts. +// HDR-2: title edit Escape cancels / reverts (#826). // -// NOT IMPLEMENTED (verified): the header title is a controlled bound to -// setTitle on every keystroke (AppHeader.tsx → TextInput; editorStore.setTitle). There -// is no draft snapshot and no Escape/onKeyDown handler, so Escape cannot revert — a -// live probe typing "TempEscapeTitle" then Escape leaves the field unchanged. Asserting -// a revert would be asserting behavior the product does not have. Marked fixme until a -// revert-on-Escape affordance is added (a draft buffer committed on Enter/blur and -// discarded on Escape). -test.fixme( - 'HDR-2: pressing Escape reverts an in-progress title edit', - async () => { - // Intentionally empty: the revert behavior does not exist yet (see comment above). - // Implement once the title field gains a draft buffer + Escape→revert handler. - }, -); +// The title field now edits a local DRAFT buffer (AppHeader.tsx): keystrokes update the +// draft only; Enter/blur COMMIT it via onTitleChange (= editorStore.setTitle); Escape +// DISCARDS the draft back to the last committed value WITHOUT committing. So: +// - type a new title, press Escape ⇒ the field reverts to the prior committed value; +// - then type again and press Enter ⇒ the field commits the new value. +// Both halves are asserted so this is a real revert-AND-commit check, not a half-test. +// ────────────────────────────────────────────────────────────────────────────── +test('HDR-2: pressing Escape reverts an in-progress title edit; Enter commits', async ({ + page, +}) => { + await gotoFresh(page); + + const title = titleInput(page); + await expect(title).toBeVisible(); + // Starter title is "Untitled" — the committed baseline we expect Escape to restore. + await expect(title).toHaveValue('Untitled'); + + // Type a throwaway title (draft only — not yet committed). + await setTitle(page, 'TempEscapeTitle'); + await expect(title).toHaveValue('TempEscapeTitle'); + + // Escape reverts the draft to the last committed value — no commit happened. + await page.keyboard.press('Escape'); + await expect(title).toHaveValue('Untitled'); + + // And Enter still commits a real edit: type a new value, Enter, value persists. + const COMMITTED = 'EnterCommitsTitle'; + await setTitle(page, COMMITTED); + await page.keyboard.press('Enter'); + await expect(title).toHaveValue(COMMITTED); + + // Prove Enter committed into the actual item (not just the field): save + open the hub + // and read the persisted card title — the COMMITTED value rode through, the reverted + // TempEscapeTitle did not. + await page.locator('[data-testid="header-menu"]').click(); + await page.locator('[data-testid="header-save"]').click(); + const noticeCancel = page.locator('[data-testid="confirm-cancel"]'); + await expect(noticeCancel).toBeVisible(); + await noticeCancel.click(); + await expect(noticeCancel).toBeHidden(); + + await gotoHome(page); + await expect(page.locator('[data-testid="home-grid"]')).toBeVisible({ + timeout: 10_000, + }); + await expect(page.getByText(COMMITTED, { exact: true })).toBeVisible(); + await expect(page.getByText('TempEscapeTitle', { exact: true })).toHaveCount(0); +}); // ────────────────────────────────────────────────────────────────────────────── // HDR-3: an unsaved marker appears on edit and clears after Save. diff --git a/e2e/tests/preview.spec.js b/e2e/tests/preview.spec.js index 09674118..55b0ab4a 100644 --- a/e2e/tests/preview.spec.js +++ b/e2e/tests/preview.spec.js @@ -256,81 +256,72 @@ test('PRV-4: console eval runs JS in the preview iframe and echoes the result', }); // ────────────────────────────────────────────────────────────────────────────── -// PRV-6: With auto-preview OFF, editing the DSL does NOT re-render until a manual -// refresh (toggling auto-preview back ON re-fires the render — the only -// manual re-render trigger in the no-rail UI). +// PRV-6: With auto-preview OFF, editing the DSL does NOT re-render until the manual +// Refresh control fires (#824). // ────────────────────────────────────────────────────────────────────────────── -// PRODUCT BUG (fixme until wired): this test is a FALSE-GREEN. `settings.autoPreview` -// is never passed to PreviewFrame — the two call sites in AppRoot.tsx (~918 and ~1350) -// omit `autoPreview={settings.autoPreview}`, so PreviewFrame falls back to its default -// `autoPreview=true` (PreviewFrame.tsx:47) and ALWAYS re-renders on the 500ms debounce. -// The Settings toggle is therefore a dead control for rendering: it flips the stored -// setting but never gates the preview. The old assertions happened to pass only because -// they fired inside the debounce window (before the always-on re-render landed) — they -// would pass identically with auto-preview ON, so they prove nothing. -// To enable this test: -// 1. Wire the prop: pass `autoPreview={settings.autoPreview}` to PreviewFrame at -// AppRoot.tsx ~918 and ~1350 so the toggle actually gates rendering. -// 2. Make the staleness assertion debounce-proof: after editing with auto-preview -// OFF, wait > PREVIEW_DEBOUNCE (the 500ms PreviewFrame debounce) BEFORE asserting -// the preview is NOT re-rendered, so a real always-on render would be caught. -test.fixme( - 'PRV-6: with auto-preview OFF, editing does not re-render until auto-preview is re-enabled', - async ({ page }) => { - // The render path depends on the iframe's srcdoc evaluating @zenuml/core; assert - // it against the local app (it works the same against any host the app serves, - // but the render timing is most stable locally — keep parity with smoke/dsl specs - // which run on both, so no skip needed here). - - // Step 1 — establish a known baseline render with a distinctive participant. - const BASELINE = 'PrvBaseline\nUser\nUser->PrvBaseline: before'; - await typeDsl(page, BASELINE); - await expect(mountLocator(page)).toContainText('PrvBaseline', { - timeout: 15_000, - }); - - // Step 2 — turn Auto-preview OFF via Settings (the Radix Switch toggles to - // unchecked). This stops PreviewFrame's debounced re-render effect. - await openViaHeaderMenu(page, 'header-settings'); - await expect(page.locator('[data-testid="settings-modal"]')).toBeVisible(); - const autoPreviewSwitch = page.locator( - '[data-testid="setting-autoPreview"]', - ); - await expect(autoPreviewSwitch).toHaveAttribute('aria-checked', 'true'); - await autoPreviewSwitch.click(); - await expect(autoPreviewSwitch).toHaveAttribute('aria-checked', 'false'); - await page.keyboard.press('Escape'); - await expect(page.locator('[data-testid="settings-modal"]')).toBeHidden(); - - // Step 3 — edit the DSL to a NEW distinctive participant. With auto-preview off, - // the preview must stay STALE: still showing the baseline, never the new token. - const EDITED = 'PrvUpdated\nUser\nUser->PrvUpdated: after'; - await typeDsl(page, EDITED); - await expect(editorLocator(page)).toContainText('PrvUpdated'); - - // Give the debounce window time to fire IF it were going to (it must not). Then - // assert the preview is unchanged: baseline still rendered, new token absent. - await expect(mountLocator(page)).toContainText('PrvBaseline'); - await expect(mountLocator(page)).not.toContainText('PrvUpdated'); - - // Step 4 — manual refresh: re-enable Auto-preview. The PreviewFrame effect's dep - // array includes `autoPreview`, so flipping it back ON re-posts the render and - // the preview catches up to the edited DSL. - await openViaHeaderMenu(page, 'header-settings'); - await expect(page.locator('[data-testid="settings-modal"]')).toBeVisible(); - await autoPreviewSwitch.click(); - await expect(autoPreviewSwitch).toHaveAttribute('aria-checked', 'true'); - await page.keyboard.press('Escape'); - await expect(page.locator('[data-testid="settings-modal"]')).toBeHidden(); - - await expect(mountLocator(page)).toContainText('PrvUpdated', { - timeout: 15_000, - }); - await expect(mountLocator(page)).not.toContainText('PrvBaseline', { - timeout: 15_000, - }); - }, +// WIRED (#824): `autoPreview={settings.autoPreview}` is now passed to PreviewFrame +// (AppRoot.tsx), so the Settings toggle gates the debounced re-render. With it OFF, a +// RendererHeader "Refresh" control (data-testid="renderer-refresh") appears and calls +// PreviewFrame's imperative render() to re-render on demand. This test is DEBOUNCE-PROOF: +// after editing with auto-preview OFF it waits LONGER than PREVIEW_DEBOUNCE (+ margin) +// BEFORE asserting the preview is stale, so a regression to always-on rendering is caught +// (the old false-green fired inside the debounce window and proved nothing). +// +// Gated off the staging gate (PW_BASE_URL): it depends on local render timing, like the +// other render-timing specs (PRV-4 / production-build). +test.skip( + !!process.env.PW_BASE_URL, + 'PRV-6 relies on local render timing (debounce window); not run on the staging gate', ); +test('PRV-6: with auto-preview OFF, editing does not re-render until manual Refresh', async ({ + page, +}) => { + // Step 1 — establish a known baseline render with a distinctive participant. + const BASELINE = 'PrvBaseline\nUser\nUser->PrvBaseline: before'; + await typeDsl(page, BASELINE); + await expect(mountLocator(page)).toContainText('PrvBaseline', { + timeout: 15_000, + }); + + // Step 2 — turn Auto-preview OFF via Settings (the Radix Switch toggles to + // unchecked). This stops PreviewFrame's debounced re-render effect and reveals the + // RendererHeader Refresh control. + await openViaHeaderMenu(page, 'header-settings'); + await expect(page.locator('[data-testid="settings-modal"]')).toBeVisible(); + const autoPreviewSwitch = page.locator('[data-testid="setting-autoPreview"]'); + await expect(autoPreviewSwitch).toHaveAttribute('aria-checked', 'true'); + await autoPreviewSwitch.click(); + await expect(autoPreviewSwitch).toHaveAttribute('aria-checked', 'false'); + await page.keyboard.press('Escape'); + await expect(page.locator('[data-testid="settings-modal"]')).toBeHidden(); + + // The manual Refresh control is now present (auto-preview OFF ⇒ AppRoot wires onRefresh). + const refresh = page.locator('[data-testid="renderer-refresh"]'); + await expect(refresh).toBeVisible(); + + // Step 3 — edit the DSL to a NEW distinctive participant. With auto-preview off, + // the preview must stay STALE: still showing the baseline, never the new token. + const EDITED = 'PrvUpdated\nUser\nUser->PrvUpdated: after'; + await typeDsl(page, EDITED); + await expect(editorLocator(page)).toContainText('PrvUpdated'); + + // DEBOUNCE-PROOF: wait WELL PAST the 500ms PreviewFrame debounce (+ generous margin) + // so an always-on re-render would have landed by now if the toggle were dead. Then + // assert the preview is unchanged: baseline still rendered, new token absent. + await page.waitForTimeout(2000); // > PREVIEW_DEBOUNCE (500ms) + margin + await expect(mountLocator(page)).toContainText('PrvBaseline'); + await expect(mountLocator(page)).not.toContainText('PrvUpdated'); + + // Step 4 — click the manual Refresh control. PreviewFrame.render() re-posts a render + // of the CURRENT (edited) code, so the preview catches up to the edited DSL. + await refresh.click(); + await expect(mountLocator(page)).toContainText('PrvUpdated', { + timeout: 15_000, + }); + await expect(mountLocator(page)).not.toContainText('PrvBaseline', { + timeout: 15_000, + }); +}); // ────────────────────────────────────────────────────────────────────────────── // PRV-8: The renderer header shows a 100% zoom indicator. diff --git a/e2e/tests/settings.spec.js b/e2e/tests/settings.spec.js index 00ab6ef5..5259fbb9 100644 --- a/e2e/tests/settings.spec.js +++ b/e2e/tests/settings.spec.js @@ -206,26 +206,64 @@ test('SET-3: font-family change applies live to .cm-content', async ({ }); // ────────────────────────────────────────────────────────────────────────────── -// SET-4: indent-unit change → auto-indent width. DEFERRED. +// SET-4: indent-size change → auto-indent width (#825). // -// The Indent-size Select renders but is NOT wired to the editor: AppRoot threads -// only theme/fontSize/fontFamily/keymap into (AppRoot.tsx ~1268), and -// NOTHING in web/src sets CM6's `indentUnit` facet from settings.indentSize. The DSL -// block body indents a FIXED 2 spaces via delimitedIndent (zenumlLanguage.ts). So -// selecting indent=4 produces no observable change in the editor — there is no honest -// assertion to make today. Marked fixme rather than faked. +// settings.indentSize/indentWith are now threaded into (AppRoot.tsx), which +// sets CM6's `indentUnit` facet from them. The DSL grammar's delimitedIndent reads that +// facet (delimitedStrategy = baseIndent + context.unit), so changing indent size produces +// a REAL, observable change in auto-indent width: pressing Enter inside an `A.run() { … }` +// block indents the new body line by exactly one indent unit. +// +// We compute the observable effect from the DOC TEXT, not a CSS/pixel proxy: after Enter +// the cursor sits at the body's indent, so inserting a sentinel char and reading its +// column (leading-whitespace length on that line) yields the applied indent width. At the +// DEFAULT indent=2 the body indents 2; after switching to indent=4 it indents 4. // ────────────────────────────────────────────────────────────────────────────── -test.fixme( - 'SET-4: indent-unit change (4) changes auto-indent width in a block', - async ({ page }) => { - // Wiring gap: settings.indentSize is not connected to the CM6 indentUnit facet, - // so the editor always indents blocks by 2 spaces regardless of this setting. - // Implement once AppRoot passes indentSize/indentWith into and the - // editor sets indentUnit from it; then: set indent=4, type `A.run() {`, Enter, - // assert the new line is indented 4 spaces. - await gotoFresh(page); - }, -); +test('SET-4: indent-size change (2 → 4) changes auto-indent width in a DSL block', async ({ + page, +}) => { + await gotoFresh(page); + const content = dslContent(page); + await expect(content).toBeVisible(); + + // Reads the leading-space count of the line the cursor lands on after Enter-in-block. + // Type the opener, Enter (auto-indents the new line one unit), then a sentinel; read + // back the doc and measure the sentinel line's indent. Returns the indent width. + async function blockIndentWidth() { + await content.click(); + await page.keyboard.press(selectAll); + await page.keyboard.press('Delete'); + // `A.run() {` is a StatementBraceBlock opener; closeBrackets auto-inserts the `}`. + await content.pressSequentially('A.run() {'); + await page.keyboard.press('Enter'); // auto-indent the body line by one unit + await content.pressSequentially('X'); // sentinel marks the indented column + // Read the indent width straight from the rendered CM6 lines. CM6 renders each + // document line as a `.cm-line` whose textContent includes the leading whitespace, + // so the sentinel line's leading-space count IS the applied auto-indent width. + const indent = await page.evaluate(() => { + const line = Array.from( + document.querySelectorAll('[data-testid="dsl-editor"] .cm-line'), + ) + .map((n) => n.textContent || '') + .find((l) => l.includes('X')); + const m = line ? line.match(/^(\s*)X/) : null; + return m ? m[1].length : -1; + }); + return indent; + } + + // Default indent = 2 → the block body indents 2 columns. + expect(await blockIndentWidth()).toBe(2); + + // Switch indent size to 4 (live compartment reconfigure — no reload). + await openSettings(page); + await chooseSetting(page, 'setting-indentSize', '4'); + await page.keyboard.press('Escape'); + await expect(page.locator('[data-testid="settings-modal"]')).toBeHidden(); + + // Now the same Enter-in-block indents the body 4 columns — the setting took effect. + expect(await blockIndentWidth()).toBe(4); +}); // ────────────────────────────────────────────────────────────────────────────── // SET-5: a behavior toggle takes observable effect. diff --git a/web/src/app/AppRoot.tsx b/web/src/app/AppRoot.tsx index 8a9443ec..6053709e 100644 --- a/web/src/app/AppRoot.tsx +++ b/web/src/app/AppRoot.tsx @@ -920,6 +920,7 @@ export function AppRoot() { code={item.js} css={item.cssMode === 'css' ? item.css : transpiledCss} stickyOffset={stickyOffset} + autoPreview={settings.autoPreview} /> ) : ( @@ -1265,7 +1266,7 @@ export function AppRoot() { DSL -
+
{/* CSS pane → collapsible strip. Re-keyed per page so the collapsed default re-derives from the new page's CSS emptiness (matches the @@ -1306,7 +1307,7 @@ export function AppRoot() { } > - + ) @@ -1330,6 +1331,9 @@ export function AppRoot() { ) : ( previewRef.current?.render()} pageTabs={ { await waitFor(() => expect(screen.getByTestId('header-title')).toHaveFocus()); }); - it('editing header-title calls onTitleChange', async () => { + // #826: the title field now edits a local DRAFT and commits on Enter/blur (not on + // every keystroke), so the user can revert with Escape. Editing then pressing Enter + // commits the typed title via onTitleChange. + it('editing header-title and pressing Enter commits onTitleChange', async () => { const onTitleChange = vi.fn(); render(); const input = screen.getByTestId('header-title'); await userEvent.clear(input); await userEvent.type(input, 'New Title'); - expect(onTitleChange).toHaveBeenCalled(); + await userEvent.keyboard('{Enter}'); + expect(onTitleChange).toHaveBeenCalledWith('New Title'); + }); + + // #826: blur also commits (commit-on-exit), so clicking away from a renamed field + // saves it. + it('editing header-title and blurring commits onTitleChange', async () => { + const onTitleChange = vi.fn(); + render(); + const input = screen.getByTestId('header-title'); + await userEvent.clear(input); + await userEvent.type(input, 'Blurred Title'); + input.blur(); + expect(onTitleChange).toHaveBeenCalledWith('Blurred Title'); + }); + + // #826: Escape reverts the in-progress edit WITHOUT committing — the field returns to + // the last committed value and onTitleChange is never fired. + it('pressing Escape reverts the title edit and does not commit', async () => { + const onTitleChange = vi.fn(); + render(); + const input = screen.getByTestId('header-title') as HTMLInputElement; + expect(input.value).toBe('My Diagram'); + await userEvent.clear(input); + await userEvent.type(input, 'TempEscapeTitle'); + expect(input.value).toBe('TempEscapeTitle'); + await userEvent.keyboard('{Escape}'); + // Reverted to the committed value; never committed. + expect(input.value).toBe('My Diagram'); + expect(onTitleChange).not.toHaveBeenCalled(); }); // ---- Save-state indicator (replaces the Save button) ---- diff --git a/web/src/components/header/AppHeader.tsx b/web/src/components/header/AppHeader.tsx index 333e37ec..3eeb467f 100644 --- a/web/src/components/header/AppHeader.tsx +++ b/web/src/components/header/AppHeader.tsx @@ -1,4 +1,4 @@ -import { useRef, useState, type ReactNode } from 'react'; +import { useEffect, useRef, useState, type ReactNode } from 'react'; import { Button, TextInput, @@ -177,6 +177,50 @@ export function AppHeader({ el.select(); }; + // #826: title edit with a DRAFT buffer so Escape can revert without committing. + // The field edits a local `draft`; the committed title (`title` prop, owned by the + // editor store) only changes on Enter/blur. Escape discards the draft back to the + // last committed value. While the user is NOT editing, the field mirrors the prop — + // so external title changes (rename via the doc menu, switching diagrams) still flow + // through. We track editing via focus, and a ref holds the value to revert TO on + // Escape (captured at focus time) so a mid-edit prop change can't desync the revert. + const [draft, setDraft] = useState(title); + const editingTitle = useRef(false); + const committedTitleRef = useRef(title); + committedTitleRef.current = title; + // When not actively editing, keep the field in sync with the committed prop. During + // an edit we leave `draft` alone so keystrokes aren't clobbered by a re-render. + useEffect(() => { + if (!editingTitle.current) setDraft(title); + }, [title]); + + const commitTitle = (value: string) => { + // Only fire onTitleChange when the value actually differs from what's committed, + // so a no-op blur/Enter doesn't mark the diagram dirty. + if (value !== committedTitleRef.current) { + // Optimistically advance the committed ref so a follow-up blur re-syncs to the + // just-committed value (the `title` prop updates async via the store, one turn + // later). Without this, blur's re-sync would read the STALE prop and briefly + // revert the field before the prop catches up. + committedTitleRef.current = value; + onTitleChange(value); + } + }; + const handleTitleKeyDown = (e: React.KeyboardEvent) => { + if (e.key === 'Enter') { + e.preventDefault(); + commitTitle(draft); + // Keep editing flag set through the synchronous commit; blur (below) clears it. + e.currentTarget.blur(); + } else if (e.key === 'Escape') { + e.preventDefault(); + // Revert: discard the draft, restore the last committed value, do NOT commit. + setDraft(committedTitleRef.current); + editingTitle.current = false; + e.currentTarget.blur(); + } + }; + return ( <>
onTitleChange(e.target.value)} + onFocus={() => { + editingTitle.current = true; + }} + onChange={(e) => setDraft(e.target.value)} + onKeyDown={handleTitleKeyDown} + onBlur={() => { + // Commit on blur (commit-on-exit), then leave edit mode. Escape's blur + // already reverted the draft, so this commit is a no-op there (the draft + // equals the committed value → commitTitle skips onTitleChange). + if (editingTitle.current) { + editingTitle.current = false; + commitTitle(draft); + } + // Re-sync to the authoritative committed value once editing ends. + setDraft(committedTitleRef.current); + }} /> diff --git a/web/src/components/preview/RendererHeader.test.tsx b/web/src/components/preview/RendererHeader.test.tsx index 7899123b..4a1270c6 100644 --- a/web/src/components/preview/RendererHeader.test.tsx +++ b/web/src/components/preview/RendererHeader.test.tsx @@ -47,6 +47,22 @@ describe('RendererHeader', () => { expect(screen.queryByTestId('renderer-fit')).not.toBeInTheDocument(); }); + // #824: the manual Refresh control follows the same "render only when wired" rule as + // Fit, so the default UI (auto-preview ON ⇒ no onRefresh) shows no extra control. + it('does not render the Refresh control when onRefresh is omitted', () => { + setup(); + expect(screen.queryByTestId('renderer-refresh')).not.toBeInTheDocument(); + }); + + it('renders the Refresh control and calls onRefresh when a handler is provided', () => { + const onRefresh = vi.fn(); + setup({ onRefresh }); + const refresh = screen.getByTestId('renderer-refresh'); + expect(refresh).toHaveAttribute('aria-label', 'Refresh preview'); + fireEvent.click(refresh); + expect(onRefresh).toHaveBeenCalledTimes(1); + }); + it('renders the Fit control and calls onFit when a handler is provided', () => { const onFit = vi.fn(); setup({ onFit }); diff --git a/web/src/components/preview/RendererHeader.tsx b/web/src/components/preview/RendererHeader.tsx index bc7e489c..3d6c79e4 100644 --- a/web/src/components/preview/RendererHeader.tsx +++ b/web/src/components/preview/RendererHeader.tsx @@ -22,6 +22,13 @@ export interface RendererHeaderProps { onPresent(): void; /** Optional fit-to-screen handler. Fit button only renders when provided. */ onFit?(): void; + /** + * #824: optional manual re-render handler. The Refresh button renders ONLY when this + * is supplied — AppRoot passes it solely when auto-preview is OFF, so the default UI + * (auto-preview ON, the 99% case) is unchanged and shows no extra control. Mirrors the + * onFit "render only when wired" pattern so there is never a dead button. + */ + onRefresh?(): void; /** Zoom percentage display, e.g. "100%". Presentational; defaults to "100%". */ zoomLabel?: string; } @@ -30,6 +37,7 @@ export function RendererHeader({ pageTabs, onPresent, onFit, + onRefresh, zoomLabel = '100%', }: RendererHeaderProps) { return ( @@ -44,6 +52,27 @@ export function RendererHeader({ {zoomLabel} + {/* #824: manual Refresh — only present when auto-preview is OFF (AppRoot gates + it by passing onRefresh conditionally). Lets the user re-render on demand so + "auto-preview off" stays usable. Matches the Fit/Present control styling. */} + {onRefresh && ( + + onRefresh()} + > + {/* circular-refresh icon */} + + + + + + + )} + {onFit && ( { await waitFor(() => expect(onZoneChange).toHaveBeenCalledWith('head')); }); + // #825: indentSize/indentWith wiring. The editor sets CM6's `indentUnit` facet from + // these props; the DSL grammar's delimitedIndent reads that facet, so a non-default + // indent size produces an OBSERVABLE block-indent width change. We assert via the + // public getIndentation API (same harness as indentInvariant.test.ts) — the body line + // of `A.b() { … }` is indented exactly one indent unit past the opener. + function dslBlockBodyIndent(view: EditorView): number | null { + // doc is `A.b() {\n body\n}` — line 2 is the block body. getIndentation returns the + // GRAMMAR-computed indent (baseIndent + one unit), independent of the literal text. + return getIndentation(view.state, view.state.doc.line(2).from); + } + + it('indentSize=4 makes the DSL block body indent 4 columns (indentUnit wired)', async () => { + const ref = createRef(); + render( + {}} + testId="dsl-editor" indentWith="spaces" indentSize={4} />, + ); + await waitFor(() => expect(ref.current?.view).toBeTruthy()); + const view = ref.current!.view!; + expect(getIndentUnit(view.state)).toBe(4); // facet reflects the setting + expect(dslBlockBodyIndent(view)).toBe(4); // observable: body indents one 4-space unit + }); + + it('indentSize=2 (default) keeps the DSL block body at 2 columns', async () => { + const ref = createRef(); + render( + {}} + testId="dsl-editor" indentWith="spaces" indentSize={2} />, + ); + await waitFor(() => expect(ref.current?.view).toBeTruthy()); + const view = ref.current!.view!; + expect(getIndentUnit(view.state)).toBe(2); + expect(dslBlockBodyIndent(view)).toBe(2); + }); + + it('indentWith=tabs sets a tab indent unit (one tabSize-wide level)', async () => { + const ref = createRef(); + render( + {}} + testId="dsl-editor" indentWith="tabs" indentSize={4} />, + ); + await waitFor(() => expect(ref.current?.view).toBeTruthy()); + const view = ref.current!.view!; + // A tab unit reports its width as tabSize columns (getIndentUnit: tabSize * length). + expect(getIndentUnit(view.state)).toBe(view.state.tabSize); + expect(dslBlockBodyIndent(view)).toBe(view.state.tabSize); + }); + + it('indentSize change reconfigures the live editor (2 → 8) without remount', async () => { + const ref = createRef(); + const { rerender } = render( + {}} + testId="dsl-editor" indentWith="spaces" indentSize={2} />, + ); + await waitFor(() => expect(ref.current?.view).toBeTruthy()); + const view = ref.current!.view!; + expect(dslBlockBodyIndent(view)).toBe(2); + // Same view instance after the prop change → the compartment reconfigured live. + rerender( + {}} + testId="dsl-editor" indentWith="spaces" indentSize={8} />, + ); + await waitFor(() => expect(getIndentUnit(ref.current!.view!.state)).toBe(8)); + expect(ref.current!.view).toBe(view); // not remounted + expect(dslBlockBodyIndent(ref.current!.view!)).toBe(8); + }); + it('Mod-Shift-f does NOT format/mutate a read-only (ACSS) editor (FIX 6a)', async () => { const onChange = vi.fn(); const ref = createRef(); diff --git a/web/src/editor/CodeEditor.tsx b/web/src/editor/CodeEditor.tsx index 5b279209..be23965c 100644 --- a/web/src/editor/CodeEditor.tsx +++ b/web/src/editor/CodeEditor.tsx @@ -4,6 +4,7 @@ import { defaultKeymap } from '@codemirror/commands'; import { Compartment, Prec, type Extension } from '@codemirror/state'; import { linter, lintGutter, forceLinting, type Diagnostic } from '@codemirror/lint'; import { autocompletion, completionKeymap } from '@codemirror/autocomplete'; +import { indentUnit } from '@codemirror/language'; import { abbreviationTracker } from '@emmetio/codemirror6-plugin'; import { vim } from '@replit/codemirror-vim'; import { forwardRef, useEffect, useMemo, useRef } from 'react'; @@ -30,6 +31,13 @@ export interface CodeEditorProps { fontFamily?: string; // default 'FiraCode' (DEFAULT_SETTINGS.editorFont) fontSize?: number; // default 16 (DEFAULT_SETTINGS.fontSize) keymap?: 'sublime' | 'vim'; // default 'sublime' (CM6 default keymap, no vim) + // #825: indent unit wiring. `indentWith` selects spaces vs a tab; `indentSize` is the + // number of spaces per level (ignored when indentWith === 'tabs'). Together they set + // CM6's `indentUnit` facet, which drives auto-indent width (delimitedIndent in the + // DSL grammar uses this unit) and the indent/dedent commands. Defaults match + // DEFAULT_SETTINGS (2 spaces) so callers that omit them keep today's behavior. + indentWith?: 'spaces' | 'tabs'; // default 'spaces' + indentSize?: number; // default 2 (spaces per indent level) diagnostics?: { lineNumber: number; message: string }[]; // REQ-ED-7: inline error markers // DSL only: fires when the cursor's parse zone (head vs block) changes. Feeds the // context-sensitive Hint Bar. Ignored for CSS. @@ -59,6 +67,17 @@ function fontTheme(family: string, size: number): Extension { }); } +// #825: resolve the indent setting to the string CM6's `indentUnit` facet expects: +// a run of `n` spaces, or a single tab. The facet's combine() validates that the +// string is whitespace and homogeneous, so a tab is always exactly '\t' (one level = +// one tabSize column) and spaces are ' '.repeat(n). n is clamped to >= 1 so a stray 0 +// can never produce the empty-string facet value (which would throw). +function indentUnitString(indentWith: 'spaces' | 'tabs', indentSize: number): string { + if (indentWith === 'tabs') return '\t'; + const n = Math.max(1, Math.floor(indentSize) || 1); + return ' '.repeat(n); +} + // Stable default so editors without diagnostics don't reconfigure the lint // compartment on every render (a fresh [] would change identity each time). const NO_DIAGNOSTICS: { lineNumber: number; message: string }[] = []; @@ -108,6 +127,8 @@ export const CodeEditor = forwardRef(functi fontFamily = 'FiraCode', fontSize = 16, keymap: keymapMode = 'sublime', + indentWith = 'spaces', + indentSize = 2, diagnostics = NO_DIAGNOSTICS, onZoneChange, }, @@ -116,6 +137,7 @@ export const CodeEditor = forwardRef(functi const themeCompartment = useRef(new Compartment()); const fontCompartment = useRef(new Compartment()); const keymapCompartment = useRef(new Compartment()); + const indentCompartment = useRef(new Compartment()); const lintCompartment = useRef(new Compartment()); const viewRef = useRef(null); // Keep latest onChange without rebuilding extensions (used by the CSS format command). @@ -221,6 +243,9 @@ export const CodeEditor = forwardRef(functi themeCompartment.current.of(resolveTheme(themeId)), fontCompartment.current.of(fontTheme(fontFamily, fontSize)), keymapCompartment.current.of(keymapMode === 'vim' ? vim() : []), + // #825: indent unit (spaces×n or tab). Live-swapped via the compartment below so a + // Settings change reflects without remounting the editor. + indentCompartment.current.of(indentUnit.of(indentUnitString(indentWith, indentSize))), // Our bindings come BEFORE defaultKeymap so they win on conflicts. keymap.of([...bindings, ...defaultKeymap]), ...languageExtensions, @@ -246,6 +271,15 @@ export const CodeEditor = forwardRef(functi }); }, [fontFamily, fontSize]); + // #825: re-apply the indent unit live when the Settings indentWith/indentSize change. + useEffect(() => { + viewRef.current?.dispatch({ + effects: indentCompartment.current.reconfigure( + indentUnit.of(indentUnitString(indentWith, indentSize)), + ), + }); + }, [indentWith, indentSize]); + useEffect(() => { viewRef.current?.dispatch({ effects: lintCompartment.current.reconfigure(diagnosticsLinter(diagnostics)), diff --git a/web/src/preview/PreviewFrame.test.tsx b/web/src/preview/PreviewFrame.test.tsx index ba663df6..44d0745a 100644 --- a/web/src/preview/PreviewFrame.test.tsx +++ b/web/src/preview/PreviewFrame.test.tsx @@ -99,6 +99,82 @@ describe('PreviewFrame', () => { await expect(p).resolves.toBe('data:image/png;base64,X'); }); + // #824: auto-preview gating + the manual render() handle. These are the cheap, + // debounce-precise mirror of e2e PRV-6 (with auto-preview OFF, edits don't re-render + // until a manual refresh). Fake timers let us advance PAST the 500ms debounce and + // prove the always-on render did NOT fire — the false-green the old e2e missed. + it('with autoPreview OFF, a code change does NOT re-render even after the debounce', () => { + vi.useFakeTimers(); + try { + const { container, rerender } = render( + , + ); + const iframe = container.querySelector('iframe') as HTMLIFrameElement; + const post = vi.fn(); + Object.defineProperty(iframe, 'contentWindow', { value: { postMessage: post }, configurable: true }); + act(() => { window.dispatchEvent(new MessageEvent('message', { source: iframe.contentWindow, data: { type: 'ready' } })); }); + post.mockClear(); // ignore the ready-triggered render + + rerender(); // edit + act(() => { vi.advanceTimersByTime(2000); }); // well past PREVIEW_DEBOUNCE (500ms) + + // Auto-preview is OFF → no render posted for the edit. + expect(post).not.toHaveBeenCalledWith( + expect.objectContaining({ type: 'render', code: 'C.d' }), '*', + ); + } finally { + vi.useRealTimers(); + } + }); + + it('with autoPreview OFF, the imperative render() forces a re-render of the current code', () => { + const ref = createRef(); + const { container, rerender } = render( + , + ); + const iframe = container.querySelector('iframe') as HTMLIFrameElement; + const post = vi.fn(); + Object.defineProperty(iframe, 'contentWindow', { value: { postMessage: post }, configurable: true }); + act(() => { window.dispatchEvent(new MessageEvent('message', { source: iframe.contentWindow, data: { type: 'ready' } })); }); + + rerender(); // edit (no auto render) + post.mockClear(); + + // Manual refresh → renders the LATEST code on demand. + act(() => { ref.current!.render(); }); + expect(post).toHaveBeenCalledWith(expect.objectContaining({ type: 'render', code: 'C.d' }), '*'); + }); + + it('with autoPreview ON (default), a code change re-renders after the debounce', () => { + vi.useFakeTimers(); + try { + const { container, rerender } = render(); + const iframe = container.querySelector('iframe') as HTMLIFrameElement; + const post = vi.fn(); + Object.defineProperty(iframe, 'contentWindow', { value: { postMessage: post }, configurable: true }); + act(() => { window.dispatchEvent(new MessageEvent('message', { source: iframe.contentWindow, data: { type: 'ready' } })); }); + post.mockClear(); + + rerender(); // edit + act(() => { vi.advanceTimersByTime(600); }); // past the 500ms debounce + + expect(post).toHaveBeenCalledWith(expect.objectContaining({ type: 'render', code: 'C.d' }), '*'); + } finally { + vi.useRealTimers(); + } + }); + + it('render() is a no-op before the iframe reports ready', () => { + const ref = createRef(); + const { container } = render(); + const iframe = container.querySelector('iframe') as HTMLIFrameElement; + const post = vi.fn(); + Object.defineProperty(iframe, 'contentWindow', { value: { postMessage: post }, configurable: true }); + // No 'ready' dispatched yet. + act(() => { ref.current!.render(); }); + expect(post).not.toHaveBeenCalled(); + }); + it('routes error messages to onError', () => { const onError = vi.fn(); const { container } = render(); diff --git a/web/src/preview/PreviewFrame.tsx b/web/src/preview/PreviewFrame.tsx index b175e569..5cd65b16 100644 --- a/web/src/preview/PreviewFrame.tsx +++ b/web/src/preview/PreviewFrame.tsx @@ -7,6 +7,10 @@ import { detectFromEnv } from '../app/runtimeMode'; export interface PreviewHandle { getPng(): Promise; evalConsole(expr: string): Promise<{ ok: boolean; value: string }>; + // #824: force a one-shot re-render of the CURRENT code, bypassing the autoPreview + // gate. Powers the manual "Refresh" control shown when auto-preview is OFF, so the + // preview stays usable without the debounced auto re-render. No-op until `ready`. + render(): void; } export interface PreviewFrameProps { @@ -234,6 +238,13 @@ export const PreviewFrame = forwardRef(functio setTimeout(() => { if (evalWaiters.current.delete(id)) resolve({ ok: false, value: 'timeout' }); }, 5000); }); }, + render() { + // Only meaningful once the iframe has reported `ready` (its bootstrap registered + // the render listener). Before that, the `ready` handler renders the current code + // anyway, so dropping an early manual render is correct, not a missed update. + if (!readyRef.current) return; + postRender(codeRef.current); + }, })); const iframeEl = (className: string, style?: CSSProperties) => (