From c616900c42e8dd3a5e39edf349c8a1b0fe0a6697 Mon Sep 17 00:00:00 2001 From: Handrey Cunha Date: Thu, 12 Nov 2026 02:12:20 -0500 Subject: [PATCH 1/4] add onChange calback Signed-off-by: Handrey Cunha --- .changeset/add-onChange-callback.md | 5 + .changeset/fix-errors-after-undo-redo.md | 5 + .../src/diagram-editor/DiagramEditor.tsx | 30 +- .../src/lib/invokeSafely.ts | 30 ++ .../store/DiagramEditorContextProvider.tsx | 76 ++- .../features/OnChangeCallback.stories.tsx | 373 +++++++++++++ .../stories/features/UndoRedo.stories.tsx | 502 ++--------------- .../features/shared/EditorToolbarLayout.tsx | 503 ++++++++++++++++++ .../diagram-editor/DiagramEditor.test.tsx | 12 +- .../tests/lib/invokeSafely.test.ts | 53 ++ ...ramEditorContextProvider.onChange.test.tsx | 472 ++++++++++++++++ .../DiagramEditorContextProvider.test.tsx | 127 +++++ 12 files changed, 1679 insertions(+), 509 deletions(-) create mode 100644 .changeset/add-onChange-callback.md create mode 100644 .changeset/fix-errors-after-undo-redo.md create mode 100644 packages/open-workflow-diagram-editor/src/lib/invokeSafely.ts create mode 100644 packages/open-workflow-diagram-editor/stories/features/OnChangeCallback.stories.tsx create mode 100644 packages/open-workflow-diagram-editor/stories/features/shared/EditorToolbarLayout.tsx create mode 100644 packages/open-workflow-diagram-editor/tests/lib/invokeSafely.test.ts create mode 100644 packages/open-workflow-diagram-editor/tests/store/DiagramEditorContextProvider.onChange.test.tsx diff --git a/.changeset/add-onChange-callback.md b/.changeset/add-onChange-callback.md new file mode 100644 index 00000000..91461335 --- /dev/null +++ b/.changeset/add-onChange-callback.md @@ -0,0 +1,5 @@ +--- +"@openworkflowspec/diagram-editor": minor +--- + +Add `setOnChangeCallback` to the editor ref API to receive the latest content after changes made inside the editor diff --git a/.changeset/fix-errors-after-undo-redo.md b/.changeset/fix-errors-after-undo-redo.md new file mode 100644 index 00000000..dcc10762 --- /dev/null +++ b/.changeset/fix-errors-after-undo-redo.md @@ -0,0 +1,5 @@ +--- +"@openworkflowspec/diagram-editor": patch +--- + +Fix validation errors staying stale after undo, redo or `setContent`: the errors now always describe the workflow currently shown diff --git a/packages/open-workflow-diagram-editor/src/diagram-editor/DiagramEditor.tsx b/packages/open-workflow-diagram-editor/src/diagram-editor/DiagramEditor.tsx index 69a41dd0..c6b5cc6d 100644 --- a/packages/open-workflow-diagram-editor/src/diagram-editor/DiagramEditor.tsx +++ b/packages/open-workflow-diagram-editor/src/diagram-editor/DiagramEditor.tsx @@ -22,8 +22,7 @@ import { I18nProvider, detectLocale, useI18n } from "@openworkflowspec/i18n"; import { dictionaries } from "../i18n/locales"; import { useDiagramEditorContext } from "../store/DiagramEditorContext"; import { ParsingErrorPage } from "./error-pages/ParsingErrorPage"; -import { ColorMode, ResolvedColorMode } from "@openworkflowspec/core"; -import { useResolvedColorMode } from "@openworkflowspec/core"; +import { ColorMode, ResolvedColorMode, useResolvedColorMode } from "@openworkflowspec/core"; import { SidebarProvider } from "@/components/ui/sidebar"; import { SidePanel } from "@/side-panel/SidePanel"; import { DiagramEditorErrorBoundary } from "./error-pages/DiagramEditorErrorBoundary"; @@ -52,6 +51,12 @@ import { EditSessionProvider } from "@/side-panel/EditSession"; * updates whenever `setContent()` loads a new string, so undo/redo always * round-trips in the format of the last successfully loaded content. * Returns `""` when no valid model has been loaded yet. + * + * **`setContent()`** — Loads a new workflow from a YAML or JSON string, as if + * the `content` prop had changed. + * + * **`setOnChangeCallback()`** — Registers a function called after changes made + * inside the editor, with the latest content. */ export type DiagramEditorRef = { /** Step back one history entry. No-op if there is nothing to undo. */ @@ -75,13 +80,22 @@ export type DiagramEditorRef = { * Silently ignored when the string cannot be parsed. */ setContent: (content: string) => void; + /** + * Register (or clear with `null` / `undefined`) a function called after every + * change made inside the editor: side-panel edits, undo and redo. It is never + * called for content that arrives through the `content` prop or + * `setContent()`, so it is safe to call `setContent()` from inside it. + * Replaces any previously registered callback. Errors thrown by the callback + * never break the editor; they are rethrown asynchronously. + */ + setOnChangeCallback: (callback: ((content: string) => void) | null | undefined) => void; }; export type DiagramEditorProps = { /** * The workflow definition to visualise, as a YAML or JSON string. - * Updating this prop (e.g. from an addon panel) re-parses the workflow and, - * in edit mode, pushes a new history entry if the model changed structurally. + * Updating this prop re-parses the workflow and, in edit mode, pushes a + * new history entry if the model changed structurally. * The serialisation format is auto-detected on first load and preserved for * the lifetime of the component — see `getContent()` on `DiagramEditorRef`. */ @@ -95,8 +109,8 @@ export type DiagramEditorProps = { */ isReadOnly: boolean; /** - * BCP 47 locale tag (e.g. `"en"`). Controls the language used for - * aria-labels and any localised text inside the editor. + * BCP 47 locale tag. Controls the language used for aria-labels and any + * localised text inside the editor. */ locale: string; /** @@ -117,7 +131,7 @@ const DiagramEditorContent = ({ }: { diagramDivRef: React.RefObject; colorMode: ResolvedColorMode; -}) => { +}): React.JSX.Element => { const { model } = useDiagramEditorContext(); return model === null ? ( @@ -140,7 +154,7 @@ const DiagramEditorBody = ({ resolvedColorMode: ResolvedColorMode; props: DiagramEditorProps; editorRef: React.ForwardedRef; -}) => { +}): React.JSX.Element => { const { t } = useI18n(); const errorBoundaryProps = { title: t("workflowError.title"), diff --git a/packages/open-workflow-diagram-editor/src/lib/invokeSafely.ts b/packages/open-workflow-diagram-editor/src/lib/invokeSafely.ts new file mode 100644 index 00000000..9483a2a3 --- /dev/null +++ b/packages/open-workflow-diagram-editor/src/lib/invokeSafely.ts @@ -0,0 +1,30 @@ +/* + * Copyright 2021-Present The Open Workflow Specification Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Runs a host-provided function so that an exception cannot break the caller. + * The error is rethrown asynchronously so the host's global error handling + * still sees it. + */ +export function invokeSafely(fn: () => void): void { + try { + fn(); + } catch (error: unknown) { + queueMicrotask((): void => { + throw error; + }); + } +} diff --git a/packages/open-workflow-diagram-editor/src/store/DiagramEditorContextProvider.tsx b/packages/open-workflow-diagram-editor/src/store/DiagramEditorContextProvider.tsx index db30702b..6c7e289e 100644 --- a/packages/open-workflow-diagram-editor/src/store/DiagramEditorContextProvider.tsx +++ b/packages/open-workflow-diagram-editor/src/store/DiagramEditorContextProvider.tsx @@ -20,11 +20,13 @@ import { ContentFormat, getTaskReferences, parseWorkflow, + SdkError, serializeWorkflow, validateWorkflow, } from "../core"; import type { Specification } from "@openworkflowspec/sdk"; import { DiagramEditorProps, DiagramEditorRef } from "../diagram-editor/DiagramEditor"; +import { invokeSafely } from "../lib/invokeSafely"; import { DiagramEditorContext, DiagramEditorContextType } from "./DiagramEditorContext"; import type * as RF from "@xyflow/react"; import { useWorkflowHistory } from "../react-flow/hooks/useWorkflowHistory"; @@ -47,7 +49,7 @@ export const DiagramEditorContextProvider = React.forwardRef< DiagramEditorRef, React.PropsWithChildren >((props, ref) => { - const onContentChange = props.onContentChange; + const onContentChange: ((content: string) => void) | undefined = props.onContentChange; // Detect the serialization format once from the initial content prop. // JSON content starts with `{` (after trimming); everything else is YAML. // useState keeps the format in sync with React's render cycle, so consumers @@ -55,7 +57,7 @@ export const DiagramEditorContextProvider = React.forwardRef< const [detectedContentFormat, setDetectedContentFormat] = React.useState( props.content.trimStart().startsWith("{") ? "json" : "yaml", ); - const contentFormat = props.language ?? detectedContentFormat; + const contentFormat: ContentFormat = props.language ?? detectedContentFormat; // Config state (non-history) const [locale, setLocale] = React.useState(props.locale); @@ -66,15 +68,15 @@ export const DiagramEditorContextProvider = React.forwardRef< setLocale(props.locale); } - const [nodes, setNodes] = React.useState([] as RF.Node[]); - const [edges, setEdges] = React.useState([] as RF.Edge[]); + const [nodes, setNodes] = React.useState([]); + const [edges, setEdges] = React.useState([]); const [selectedNodeId, setSelectedNodeId] = React.useState(null); - const [isExporting, setIsExporting] = React.useState(false); + const [isExporting, setIsExporting] = React.useState(false); // Read isReadOnly directly from props — no local state copy. // This ensures useWorkflowHistory always receives the current value without // a one-render lag from useState + useEffect synchronisation. - const isReadOnly = props.isReadOnly; + const isReadOnly: boolean = props.isReadOnly; // History — starts uninitialised (present = null). // Diagram.tsx is the sole caller of submitModel (after layout, with real viewport). @@ -91,17 +93,24 @@ export const DiagramEditorContextProvider = React.forwardRef< clearPendingViewportRestore, } = useWorkflowHistory(isReadOnly); - // errors are shared state written by both the props.content path and setContent. - // They are never part of a history snapshot — always reflect the last parse result. - const [errors, setErrors] = React.useState["errors"]>( + // Parse errors are only used while there is no model (unparseable content). + const [parseErrors, setParseErrors] = React.useState( () => parseWorkflow(props.content).errors, ); + // Derived from the model, so the errors always describe the model currently shown. + const validationErrors = React.useMemo( + () => (model ? validateWorkflow(model) : null), + [model], + ); + const errors: SdkError[] = validationErrors ?? parseErrors; + // Keep a ref to the latest selectedNodeId so the effect below can read it // synchronously without taking it as a dependency (avoids re-seeding on every click). const selectedNodeIdRef = React.useRef(selectedNodeId); const externallySeededModelRef = React.useRef(null); const reportedModelRef = React.useRef(null); + const onChangeCallbackRef = React.useRef<((content: string) => void) | null>(null); React.useEffect(() => { selectedNodeIdRef.current = selectedNodeId; @@ -112,20 +121,19 @@ export const DiagramEditorContextProvider = React.forwardRef< // In read-only mode the placeholder viewport is acceptable since fitView always runs. React.useEffect(() => { const { model: parsedModel, errors: parsedErrors } = parseWorkflow(props.content); - // This will be addressed in the editing feature branch as the implemntation is changing // oxlint-disable-next-line react/set-state-in-effect - setErrors(parsedErrors); + setParseErrors(parsedErrors); if (parsedModel === null) { // Content is unparseable — reset history to null so downstream consumers - // (e.g. DiagramEditorContent) see model === null and render the error page - // instead of displaying the last successfully-parsed (now stale) model. + // see model === null and render the error page instead of displaying + // the last successfully-parsed (now stale) model. resetHistory(); setSelectedNodeId(null); return; } - // Preserve selection across content reloads (e.g. addon-panel edits): only clear - // selectedNodeId when the previously-selected node/edge no longer exists in the - // new model. We read the ref synchronously so we can pass the same resolved value + // Preserve selection across content reloads: only clear selectedNodeId + // when the previously-selected node/edge no longer exists in the new model. + // We read the ref synchronously so we can pass the same resolved value // to both setSelectedNodeId and seedModel in one shot. const resolvedId = resolveSelectedId(parsedModel, selectedNodeIdRef.current); setSelectedNodeId(resolvedId); @@ -145,11 +153,9 @@ export const DiagramEditorContextProvider = React.forwardRef< */ const setContent = React.useCallback( (content: string) => { - const { model: newModel, errors: newErrors } = parseWorkflow(content); + const { model: newModel } = parseWorkflow(content); if (newModel === null) return; - setErrors(newErrors); - const newFormat: ContentFormat = content.trimStart().startsWith("{") ? "json" : "yaml"; setDetectedContentFormat(newFormat); @@ -163,8 +169,6 @@ export const DiagramEditorContextProvider = React.forwardRef< const commitWorkflow = React.useCallback( (workflow: Specification.Workflow) => { - // TODO: Temporary - Errors still computed here by validating full workflow, Open question on how to tackle this - a form knows about one task, but these areas are for entire workflow - setErrors(validateWorkflow(workflow)); const resolvedId = resolveSelectedId(workflow, selectedNodeIdRef.current); setSelectedNodeId(resolvedId); seedModel(workflow, { x: 0, y: 0, zoom: 1 }, resolvedId); @@ -196,13 +200,37 @@ export const DiagramEditorContextProvider = React.forwardRef< } if (model === reportedModelRef.current) return; reportedModelRef.current = model; - onContentChange?.(serializeWorkflow(model, contentFormat)); + + let content: string | null = null; + try { + content = serializeWorkflow(model, contentFormat); + } catch { + // Ignore serialisation failure + } + + if (content !== null) { + const reportedContent: string = content; + if (onContentChange) { + invokeSafely(() => onContentChange(reportedContent)); + } + const onChange: ((content: string) => void) | null = onChangeCallbackRef.current; + if (onChange) { + invokeSafely(() => onChange(reportedContent)); + } + } }, [model, contentFormat, onContentChange]); + const setOnChangeCallback = React.useCallback( + (callback: ((content: string) => void) | null | undefined) => { + onChangeCallbackRef.current = callback ?? null; + }, + [], + ); + React.useImperativeHandle( ref as React.Ref, - () => ({ undo, redo, canUndo, canRedo, getContent, setContent }), - [undo, redo, canUndo, canRedo, getContent, setContent], + () => ({ undo, redo, canUndo, canRedo, getContent, setContent, setOnChangeCallback }), + [undo, redo, canUndo, canRedo, getContent, setContent, setOnChangeCallback], ); // Memoize context value to prevent unnecessary re-renders of consumers. diff --git a/packages/open-workflow-diagram-editor/stories/features/OnChangeCallback.stories.tsx b/packages/open-workflow-diagram-editor/stories/features/OnChangeCallback.stories.tsx new file mode 100644 index 00000000..854f5a9e --- /dev/null +++ b/packages/open-workflow-diagram-editor/stories/features/OnChangeCallback.stories.tsx @@ -0,0 +1,373 @@ +/* + * Copyright 2021-Present The Open Workflow Specification Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React, { useCallback, useEffect, useRef, useState } from "react"; +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { DiagramEditor, DiagramEditorRef } from "../../src/diagram-editor/DiagramEditor"; +import { useResolvedColorMode } from "@openworkflowspec/core"; +import { authenticationReusable } from "../examples"; +import { EditorToolbarLayout } from "./shared/EditorToolbarLayout"; + +// --------------------------------------------------------------------------- +// Output panel +// --------------------------------------------------------------------------- + +type OutputEntry = { + id: number; + time: string; + content: string; +}; + +type PanelTheme = { + container: React.CSSProperties; + header: React.CSSProperties; + muted: string; + button: React.CSSProperties; +}; + +const lightPanel: PanelTheme = { + container: { background: "#f7f8fa", color: "#1f2328", borderTop: "1px solid #e5e7eb" }, + header: { borderBottom: "1px solid #e5e7eb", background: "#f1f3f5" }, + muted: "#57606a", + button: { border: "1px solid #d1d5db", background: "#ffffff", color: "#374151" }, +}; + +const darkPanel: PanelTheme = { + container: { background: "#111827", color: "#e5e7eb", borderTop: "1px solid #374151" }, + header: { borderBottom: "1px solid #374151", background: "#1f2937" }, + muted: "#9ca3af", + button: { border: "1px solid #4b5563", background: "#374151", color: "#e5e7eb" }, +}; + +const PANEL_HEIGHT: string = "240px"; + +function OutputPanel({ + entries, + theme, + onClear, +}: { + entries: OutputEntry[]; + theme: PanelTheme; + onClear: () => void; +}): React.JSX.Element { + const bodyRef = useRef(null); + + // Keep the newest entry in view. + useEffect(() => { + const body = bodyRef.current; + if (body) body.scrollTop = body.scrollHeight; + }, [entries]); + + return ( +
+
+ + onChange callback output ({entries.length} {entries.length === 1 ? "call" : "calls"}) + + +
+
+ {entries.length === 0 ? ( + + Nothing received yet. Edit a task in the side panel, or use Undo / Redo in the toolbar, + and the content passed to the callback appears here. Set Content does not call it. + + ) : ( + entries.map((entry) => ( +
+
+ [{entry.time}] onChange #{entry.id} +
+ 
{entry.content}
+
+ )) + )} +
+
+ ); +} + +// --------------------------------------------------------------------------- +// Story render function +// --------------------------------------------------------------------------- + +function OnChangeCallbackStory({ + content, + colorMode: colorModeProp, + isReadOnly, + locale, +}: { + content: string; + colorMode?: string; + isReadOnly?: boolean; + locale?: string; +}): React.JSX.Element { + const editorRef = useRef(null); + const callCounter = useRef(0); + const [entries, setEntries] = useState([]); + // Bumped on every callback so the toolbar re-reads canUndo / canRedo after in-editor edits. + const [historyVersion, setHistoryVersion] = useState(0); + const resolvedColorMode = useResolvedColorMode( + (colorModeProp as "light" | "dark" | "system") ?? "system", + ); + + const handleChange = useCallback((content: string) => { + callCounter.current += 1; + const entry: OutputEntry = { + id: callCounter.current, + time: new Date().toLocaleTimeString(), + content, + }; + setEntries((previous) => [...previous, entry]); + setHistoryVersion((version) => version + 1); + }, []); + + // Register the callback for the lifetime of the story. + useEffect(() => { + const editor: DiagramEditorRef | null = editorRef.current; + editor?.setOnChangeCallback(handleChange); + return () => editor?.setOnChangeCallback(null); + }, [handleChange]); + + return ( + +
+ +
+ setEntries([])} + /> + + ); +} + +// --------------------------------------------------------------------------- +// Storybook meta +// --------------------------------------------------------------------------- + +const meta = { + title: "Features/OnChange Callback", + component: DiagramEditor, + tags: ["autodocs"], + parameters: { + layout: "fullscreen", + docs: { + description: { + component: ` +## OnChange Callback + +The \`DiagramEditor\` ref API lets a host application register a callback with +\`setOnChangeCallback()\`. The editor calls it with the **latest content** (YAML or JSON) +every time the workflow is changed **from inside the editor**. + +### How it works + +The callback runs only when the origin of the change is the editor itself: + +| What changed the workflow | Callback called? | +|---|---| +| The user edits a task in the side panel | Yes | +| \`undo()\` / \`redo()\` (button, shortcut or API) | Yes | +| \`setContent()\` called by the host | **No** | +| The \`content\` prop is updated by the host | **No** | +| Initial load | **No** | + +Because external changes never trigger the callback, a host can safely forward the +received content somewhere and write it back without creating a loop: + +\`\`\` +host calls setContent(x) -> editor reloads -> callback is NOT called +user edits in the editor -> callback(content) -> host stores / forwards content +\`\`\` + +The callback is optional. When none is registered the editor does nothing extra. +Registering or clearing a callback never triggers a render or a call. + +### API + +\`\`\`tsx +import { useEffect, useRef } from "react"; +import { + DiagramEditor, + DiagramEditorRef, +} from "@openworkflowspec/diagram-editor"; + +const editorRef = useRef(null); + +// The ref is set after the first render, so register from an effect. +useEffect(() => { + const editor = editorRef.current; + editor?.setOnChangeCallback((content: string) => { + save(content); + }); + // Unregister: + return () => editor?.setOnChangeCallback(null); +}, []); + + +\`\`\` + +| Member | Type | Description | +|---|---|---| +| \`setOnChangeCallback(callback)\` | \`(callback: ((content: string) => void) \\| null \\| undefined) => void\` | Registers the function called after each in-editor change. \`null\` / \`undefined\` clears it. The last registration wins. | + +### Good to know + +- The callback runs **after** the change has been applied to the model, not + synchronously inside the user action. +- Undo and redo are in-editor changes, so they call the callback, including when they + revert an earlier \`setContent()\`. +- In read-only mode nothing can be edited and undo/redo are no-ops, so the callback + is not called. +- An exception thrown by your callback never breaks the editor. It is rethrown + asynchronously, so your global error handling (\`window.onerror\`, error reporting) + still sees it. The editor itself writes nothing to the console. +- The \`onContentChange\` prop remains available. It reports the same events but only + receives the content string. + +### Story toolbar and output panel + +The toolbar above the diagram (**Undo**, **Redo**, **Set Content**, **Get Content**) and +the panel under it are implemented in the story, they are **not** part of the +\`DiagramEditor\` component. The toolbar is the same as in the **Undo Redo** story and +exercises the ref API. The story registers a callback that appends everything it +receives to the panel: the call number and the full content. + +Try it: + +1. Select a task, edit it in the side panel and apply the change. A new entry appears. +2. Click **Undo**, then **Redo**. Each one adds an entry, because they are in-editor changes. +3. Click **Set Content**, edit the text and **Apply**. **No** entry appears, because the + change came from the host. Click **Undo** afterwards: that reverts the loaded content + and **does** add an entry. +4. Click **Get Content** to compare the current model with the last entry in the panel. +5. Change the \`content\` control in the Storybook controls panel. **No** entry appears. + +### Calling the API from the browser console + +This story publishes the editor ref on \`window.diagramEditorRef\`: + +1. Open the browser DevTools (**F12** or **⌘ ⌥ I**) and go to the **Console** tab. +2. If Storybook runs the story in an \`