Chart Builder: open a snippet in the builder to edit it in place

This commit is contained in:
2026-06-20 12:36:37 +03:00
parent efb5a9bbe0
commit be8cfc402d
16 changed files with 1046 additions and 14 deletions
@@ -58,6 +58,19 @@
gap: var(--space-3);
}
/* Edit-in-place banner (spec §06 → Open in builder): names the snippet the builder
is editing, so the primary "Save changes" action reads unambiguously. */
.editingBanner {
margin: 0;
font-size: 12px;
color: var(--text-secondary);
}
.editingBanner strong {
color: var(--text-primary);
font-weight: 600;
}
/* ── Intent front door (spec §06 → Intent) ───────────────────────────────
A persistent "what do you want to show?" strip under the dataset picker. A
quiet accent-soft wash marks it as the guided on-ramp without competing with
+15 -4
View File
@@ -1466,6 +1466,12 @@ export function ChartBuilderModal() {
const setStack = useChartBuilderStore((s) => s.setStack);
const applyWarningFix = useChartBuilderStore((s) => s.applyWarningFix);
const runCreate = useChartBuilderStore((s) => s.createSnippet);
const runSave = useChartBuilderStore((s) => s.saveEdits);
// Edit-in-place (spec §06 → Open in builder): when a snippet was opened in the
// builder, the primary action *saves back* to it instead of creating a new snippet.
const editingSnippetId = useChartBuilderStore((s) => s.editingSnippetId);
const editingSnippetName = useChartBuilderStore((s) => s.editingSnippetName);
const editing = editingSnippetId !== null;
// Validity + guidance + which chart-level controls apply are derived from the
// stable `config` reference via useMemo, NOT a store selector that would build a
@@ -1526,6 +1532,11 @@ export function ChartBuilderModal() {
<div className="visually-hidden" role="status" aria-live="polite">
{fixAnnouncement}
</div>
{editing && (
<p className={styles.editingBanner}>
Editing <strong>{editingSnippetName}</strong>
</p>
)}
<DatasetPicker datasetId={datasetId} />
<IntentStrip />
@@ -1610,7 +1621,7 @@ export function ChartBuilderModal() {
{!valid && (
<p id="cb-create-hint" className={styles.createHint}>
Map at least one channel to a column to create a snippet.
Map at least one channel to a column to {editing ? 'save changes' : 'create a snippet'}.
</p>
)}
<div className={styles.actions}>
@@ -1622,12 +1633,12 @@ export function ChartBuilderModal() {
size="lg"
disabled={!valid}
aria-describedby={!valid ? 'cb-create-hint' : undefined}
// The create is the user's confirmation — close with no discard prompt.
// The commit is the user's confirmation — close with no discard prompt.
onClick={() => {
if (runCreate()) void closeModal(true);
if (editing ? runSave() : runCreate()) void closeModal(true);
}}
>
Create Snippet
{editing ? 'Save changes' : 'Create Snippet'}
</Button>
</div>
</div>
+36 -2
View File
@@ -13,7 +13,8 @@
* inline near the editor (spec §03E), mirroring the preview via PreviewStore.
*/
import { useEffect, useRef, type RefObject } from 'react';
import { useEffect, useMemo, useRef, type RefObject } from 'react';
import { useShallow } from 'zustand/react/shallow';
// `edcore.main` is the full standalone editor — every feature contribution
// (folding, suggest widget, word operations like Cmd+Backspace, find, bracket
// colorization, multi-cursor, …) — but WITHOUT the `monaco-editor` barrel's
@@ -24,7 +25,8 @@ import 'monaco-editor/esm/vs/language/json/monaco.contribution';
import '../infrastructure/monaco-env'; // side-effect: wire workers before create
import { configureVegaLiteJson } from '../infrastructure/monaco-schema';
import { configureJsonFormatter, installFormatOnPaste } from '../infrastructure/monaco-format';
import { openModal } from '../modals/ModalCoordinator';
import { parseChartSpecText } from '@core/chart-builder';
import { openChartBuilderForEdit, openModal } from '../modals/ModalCoordinator';
import {
installSpecConfigActions,
runExtractConfig,
@@ -33,6 +35,7 @@ import {
} from '../services/spec-config-actions';
import { useAppStore } from '../stores/AppStore';
import { confirm } from '../stores/ConfirmStore';
import { useDatasetStore } from '../stores/DatasetStore';
import { hasInlineData } from '../stores/ExtractStore';
import { publishActiveSnippet } from '../services/snippet-actions';
import { notify } from '../stores/NotificationStore';
@@ -193,6 +196,26 @@ function EditorToolbar({
(s) => s.activeSnippetId !== null && hasInlineData(s.draftText),
);
// Offer "Open in builder" only when the active snippet's published spec is
// losslessly representable in the builder dialect AND its referenced dataset
// exists — the gate the builder's openForEdit enforces (spec §06 → Open in
// builder). Content-gated like Extract, so it is **hidden** when inapplicable
// (the toolbar's convention for content-gated actions — vs Revert/Config, which
// disable because they are merely state-gated; arch 10 §5). `datasetNames` is a
// shallow-stable string[] so the memo doesn't churn (MEMORY → stable selectors).
const activeSpec = useSnippetStore((s) => selectActiveSnippet(s)?.spec ?? null);
const datasetNames = useDatasetStore(useShallow((s) => s.datasets.map((d) => d.name)));
const canOpenInBuilder = useMemo(() => {
if (activeSpec === null) return false;
const config = parseChartSpecText(activeSpec);
return config !== null && datasetNames.includes(config.datasetName);
}, [activeSpec, datasetNames]);
const handleOpenInBuilder = () => {
const snippet = selectActiveSnippet(useSnippetStore.getState());
if (snippet) openChartBuilderForEdit(snippet);
};
// Publish + its success toast live in one place (services/snippet-actions) so
// the button and the Cmd/Ctrl+S shortcut (EventRouter) behave identically.
const handlePublish = publishActiveSnippet;
@@ -239,6 +262,17 @@ function EditorToolbar({
to two lines and Revert/Publish stay on one row. The label rides in
`title` (and the accessible name) when only the icon shows. Publish — the
one primary action — keeps its label at every width. */}
{canOpenInBuilder && (
<Button
className={styles.collapsible}
onClick={handleOpenInBuilder}
title="Open this chart in the visual builder to edit it"
aria-label="Open in builder"
>
<Icon name="chart" className={styles.actionIcon} />
<span className={styles.actionLabel}>Open in builder</span>
</Button>
)}
{canExtract && (
<Button
className={styles.collapsible}
+25
View File
@@ -9,6 +9,8 @@
*/
import { useAppStore } from '../stores/AppStore';
import { useChartBuilderStore } from '../stores/ChartBuilderStore';
import type { Snippet } from '@core/snippet';
import type { ModalName } from './types';
import { getModalConfig } from './modal-registry';
import { clearModalFromUrl, syncModalToUrl } from './UrlStateSync';
@@ -43,6 +45,29 @@ export function openModal(name: ModalName, arg?: string): void {
syncModalToUrl(name, arg);
}
/**
* Open the Chart Builder to edit an existing snippet in place (spec §06 → Open in
* builder). Hydrates the builder from the snippet (strict spec→config parse + its
* referenced dataset) *before* showing the modal, deliberately skipping the
* registry's default `init` — which would re-derive a fresh config and clobber the
* hydration. A no-op when the snippet isn't losslessly builder-representable or its
* dataset is gone; the caller gates the entry point on the same two conditions, so
* this guard only ever fires on a race. Mirrors `openModal`'s bookkeeping minus init
* (the builder has no `getState`, so there is no editable snapshot to capture).
*/
export function openChartBuilderForEdit(snippet: Snippet): void {
// An edit session is builder-local and intentionally not URL-serializable: the
// hydrated builder serializes to `#datasets/dataset-N/build` (the same hash as a
// *create* flow on that dataset), so a reload / Back / shared link re-runs `init`
// and reopens a fresh create flow. The published spec is untouched until Save
// changes, so nothing is lost — only the transient edit context, matching the
// builder's already-non-persistent configuration (spec §06 → Open in builder).
if (!useChartBuilderStore.getState().openForEdit(snippet)) return;
useAppStore.getState().setActiveModal('chartBuilder');
stateSnapshot = snapshotOf('chartBuilder');
syncModalToUrl('chartBuilder');
}
/**
* Re-baseline the change snapshot to the modal's current state. Call after a
* commit (the form was saved) or when a form view opens, so subsequent
+67
View File
@@ -464,3 +464,70 @@ describe('switchDataset (the builder dataset picker)', () => {
expect(cb().config).toBe(before);
});
});
describe('openForEdit / saveEdits (edit-in-place, spec §06 → Open in builder)', () => {
/** Seed a dataset, build a snippet from it, and return that linked snippet. */
function buildSnippetFrom(name: string, data: unknown) {
const id = seedDataset(name, data);
cb().init(id);
cb().createSnippet(T); // creates a linked, builder-dialect snippet; resets the builder
return { id, snippet: useSnippetStore.getState().snippets[0] };
}
test('hydrates the builder from a builder-made snippet and records the edit target', () => {
const { id, snippet } = buildSnippetFrom('Sales', [
{ region: 'E', sales: 5 },
{ region: 'W', sales: 9 },
]);
expect(cb().openForEdit(snippet)).toBe(true);
expect(cb().editingSnippetId).toBe(snippet.id);
expect(cb().editingSnippetName).toBe(snippet.name);
expect(cb().datasetId).toBe(id);
expect(cb().config.datasetName).toBe('Sales');
// A loaded chart is built-on work: a dataset switch must rebase, not re-derive,
// so config must not be reference-equal to initialConfig (see switchDataset).
expect(cb().config).not.toBe(cb().initialConfig);
});
test('saveEdits republishes into the same snippet and clears edit state', () => {
const { snippet } = buildSnippetFrom('Sales', [
{ region: 'E', sales: 5 },
{ region: 'W', sales: 9 },
]);
const originalSpec = snippet.spec;
cb().openForEdit(snippet);
cb().setMark('point');
expect(cb().saveEdits(new Date('2026-06-02T00:00:00Z'))).toBe(true);
const after = useSnippetStore.getState().snippets.find((s) => s.id === snippet.id)!;
expect(after.spec).not.toBe(originalSpec);
expect(after.spec).toContain('"point"');
expect(after.spec).toBe(after.draftSpec); // republished — no pending draft
expect(useSnippetStore.getState().activeSnippetId).toBe(snippet.id);
// The builder reset out of edit mode.
expect(cb().editingSnippetId).toBeNull();
expect(cb().datasetId).toBeNull();
});
test('openForEdit refuses a spec the builder cannot represent (inline data)', () => {
seedDataset('Sales', [{ region: 'E', sales: 5 }]);
// The default sample template is an inline-data bar chart — no `{ data: { name } }`,
// so it is not builder-representable.
useSnippetStore.getState().createSnippet();
const inline = useSnippetStore.getState().snippets[0];
expect(cb().openForEdit(inline)).toBe(false);
expect(cb().editingSnippetId).toBeNull();
});
test('openForEdit refuses when the referenced dataset is gone', () => {
const { snippet } = buildSnippetFrom('Sales', [{ region: 'E', sales: 5 }]);
useDatasetStore.getState().reset(); // the dataset no longer exists
expect(cb().openForEdit(snippet)).toBe(false);
});
test('saveEdits is a no-op outside an edit session', () => {
expect(cb().saveEdits(T)).toBe(false);
});
});
+81
View File
@@ -29,6 +29,7 @@ import {
isBuilderConfigValid,
isChannelTypeAllowed,
isColumnAllowedOnChannel,
parseChartSpecText,
pruneEncodings,
rebaseBuilderConfig,
supportsBin,
@@ -53,6 +54,7 @@ import {
type TimeUnit,
} from '@core/chart-builder';
import type { ColumnType } from '@core/type-inference';
import type { Snippet } from '@core/snippet';
import { useDatasetStore } from './DatasetStore';
import { useSnippetStore } from './SnippetStore';
@@ -99,12 +101,38 @@ export interface ChartBuilderState {
* first empty channel that accepts it).
*/
activeChannel: ChannelName | null;
/**
* The snippet being **edited in place** (spec §06 → Open in builder), or null in
* the ordinary create flow. When set, the modal's primary action saves back to
* this snippet (`saveEdits`) instead of creating a new one, and its header names
* the snippet (`editingSnippetName`).
*/
editingSnippetId: string | null;
/** The edited snippet's name, for the modal header (snapshot at open). */
editingSnippetName: string | null;
/**
* Load a dataset and pre-populate a smart default config (spec §06 → Opening).
* `null` picks the most recently modified dataset (the un-targeted entry points).
* Clears any edit-in-place state — a fresh open is a create flow.
*/
init: (datasetId: number | null) => void;
/**
* Hydrate the builder from an existing snippet for **edit-in-place** (spec §06 →
* Open in builder). Parses the snippet's published spec back to a config (strict —
* `parseChartSpecText`), loads the referenced dataset's columns, and records the
* snippet so `saveEdits` writes back to it. Returns false (a no-op) when the spec
* isn't losslessly representable or its dataset is gone — the caller gates the
* entry point on the same two conditions, so this is a defensive guard.
*/
openForEdit: (snippet: Snippet) => boolean;
/**
* Save the working config back to the edited snippet (the **Save changes** action,
* spec §06 → Open in builder): republishes the built spec into both the snippet's
* versions via `SnippetStore.replaceSnippetSpec`, then resets. Returns false when
* not in edit mode or the config is invalid. `now` injectable.
*/
saveEdits: (now?: Date) => boolean;
/**
* Re-point the open builder at another dataset (the header picker, spec §06).
* An untouched default config re-derives fresh smart defaults for the new
@@ -252,6 +280,8 @@ export const useChartBuilderStore = create<ChartBuilderState>((set, get) => ({
config: EMPTY_CONFIG,
initialConfig: EMPTY_CONFIG,
activeChannel: null,
editingSnippetId: null,
editingSnippetName: null,
init: (datasetId) => {
const all = useDatasetStore.getState().datasets;
@@ -272,6 +302,8 @@ export const useChartBuilderStore = create<ChartBuilderState>((set, get) => ({
config: EMPTY_CONFIG,
initialConfig: EMPTY_CONFIG,
activeChannel: null,
editingSnippetId: null,
editingSnippetName: null,
});
return;
}
@@ -288,9 +320,56 @@ export const useChartBuilderStore = create<ChartBuilderState>((set, get) => ({
config,
initialConfig: config,
activeChannel: null,
editingSnippetId: null,
editingSnippetName: null,
});
},
openForEdit: (snippet) => {
const config = parseChartSpecText(snippet.spec);
if (!config) return false; // not losslessly representable (caller gates on this)
const dataset = useDatasetStore.getState().datasets.find((d) => d.name === config.datasetName);
if (!dataset) return false; // the referenced dataset is gone (caller gates too)
const columns: BuilderColumns = {
columns: dataset.columns,
columnTypes: dataset.columnTypes,
columnStats: dataset.columnStats,
};
// Re-stamp the parsed transform ids through this store's own sequence so a later
// addFilter/addCalculate can't collide with one (parse numbers them by position).
const hydrated: BuilderConfig = { ...config };
if (config.calculates) {
hydrated.calculates = config.calculates.map((c) => ({ ...c, id: nextTransformId('c') }));
}
if (config.filters) {
hydrated.filters = config.filters.map((f) => ({ ...f, id: nextTransformId('f') }));
}
set({
datasetId: dataset.id,
columns,
rowCount: dataset.rowCount,
config: hydrated,
// A loaded chart has no re-derivable "opening default"; keep initialConfig
// distinct from config so a dataset switch rebases (preserving the loaded
// chart), never re-derives fresh defaults (see switchDataset).
initialConfig: EMPTY_CONFIG,
activeChannel: null,
editingSnippetId: snippet.id,
editingSnippetName: snippet.name,
});
return true;
},
saveEdits: (now) => {
const { config, editingSnippetId } = get();
if (editingSnippetId === null) return false; // not an edit session
if (!isBuilderConfigValid(config)) return false; // guarded by a disabled action too
const specText = buildSnippetSpecText(config);
const ok = useSnippetStore.getState().replaceSnippetSpec(editingSnippetId, specText, now);
if (ok) get().reset();
return ok;
},
switchDataset: (datasetId) => {
const s = get();
if (datasetId === s.datasetId) return;
@@ -576,6 +655,8 @@ export const useChartBuilderStore = create<ChartBuilderState>((set, get) => ({
config: EMPTY_CONFIG,
initialConfig: EMPTY_CONFIG,
activeChannel: null,
editingSnippetId: null,
editingSnippetName: null,
}),
}));
+46
View File
@@ -639,3 +639,49 @@ describe('library view state (spec §02 → Search / Sort)', () => {
expect(store().activeSnippetId).toBe('a');
});
});
describe('replaceSnippetSpec (Chart Builder edit-in-place save)', () => {
const newSpec = JSON.stringify(
{
$schema: 'https://vega.github.io/schema/vega-lite/v6.json',
data: { name: 'D' },
mark: { type: 'point', tooltip: true },
encoding: { x: { field: 'a', type: 'quantitative' } },
},
null,
2,
);
test('republishes both versions, activates the snippet, and bumps modified', () => {
const a = createSnippet({
id: 'a',
spec: '{"mark":"bar"}',
now: new Date('2026-01-01T00:00:00Z'),
});
const b = createSnippet({ id: 'b', now: new Date('2026-02-01T00:00:00Z') });
store().hydrate([a, b]); // active = b (newest)
const ok = store().replaceSnippetSpec('a', newSpec, new Date('2026-03-01T00:00:00Z'));
expect(ok).toBe(true);
const after = store().snippets.find((s) => s.id === 'a')!;
expect(after.spec).toBe(newSpec);
expect(after.draftSpec).toBe(newSpec); // republished — no pending draft
expect(after.modified).toBe('2026-03-01T00:00:00.000Z');
expect(after.datasetRefs).toEqual(['D']); // recomputed from the new spec
expect(store().activeSnippetId).toBe('a'); // the edited snippet becomes active
expect(store().draftText).toBe(newSpec);
expect(store().editorView).toBe('draft');
});
test('keeps a user-chosen name; returns false for an unknown id', () => {
const a = createSnippet({ id: 'a', now: new Date('2026-01-01T00:00:00Z') });
store().hydrate([a]);
store().renameSnippet('a', 'My chart'); // nameSource -> 'user'
store().replaceSnippetSpec('a', newSpec);
expect(store().snippets.find((s) => s.id === 'a')!.name).toBe('My chart');
expect(store().replaceSnippetSpec('missing', newSpec)).toBe(false);
});
});
+45
View File
@@ -118,6 +118,17 @@ export interface SnippetState {
* by-name reference. No-op when no snippet is active. `now` injectable.
*/
replaceActiveDraft: (text: string, now?: Date) => void;
/**
* Replace a snippet's spec wholesale and make it active — the Chart Builder's
* **Save changes** (edit-in-place, spec §06 → Open in builder). Unlike the draft
* rewrite above, this *republishes*: both `spec` and `draftSpec` become the built
* text, so the edited snippet has no pending draft (the builder is the source).
* An auto-named snippet re-derives its name from the new content (like `publish`);
* a user-renamed one keeps its name. Flushes the outgoing active buffer first,
* recomputes `datasetRefs`, and reloads the editor on the now-active snippet.
* No-op (returns false) for an unknown id. `now` injectable.
*/
replaceSnippetSpec: (id: string, specText: string, now?: Date) => boolean;
/**
* Persist the editor buffer into the active snippet's **draft**, if it parses
* as JSON. Returns whether it committed (a half-typed, unparseable buffer is
@@ -338,6 +349,40 @@ export const useSnippetStore = create<SnippetState>((set, get) => ({
}));
},
replaceSnippetSpec: (id, specText, now) => {
// Flush the outgoing active snippet's valid in-progress edits before we switch
// the active snippet (mirrors createSnippet / selectSnippet).
get().commitDraft(now);
const target = get().snippets.find((s) => s.id === id);
if (!target) return false;
const modified = (now ?? new Date()).toISOString();
const datasetRefs = recomputeDatasetRefs(specText);
set((s) => ({
snippets: s.snippets.map((x) =>
x.id === id
? {
...x,
// Auto names track content (as publish does); a user-chosen name stays.
...(isAutoNamed(x)
? { name: deriveSnippetName(specText) ?? x.name, nameSource: 'auto' as const }
: {}),
// Republish: the built spec becomes both versions, so there is no
// pending draft to reconcile after a builder save.
spec: specText,
draftSpec: specText,
datasetRefs,
modified,
}
: x,
),
activeSnippetId: id,
draftText: specText,
editorView: 'draft',
bufferEpoch: s.bufferEpoch + 1,
}));
return true;
},
commitDraft: (now) => {
const { activeSnippetId, draftText, snippets } = get();
if (!activeSnippetId) return false;