diff --git a/.changeset/11778-flow-add-node-paths.md b/.changeset/11778-flow-add-node-paths.md new file mode 100644 index 0000000000..e625d042b0 --- /dev/null +++ b/.changeset/11778-flow-add-node-paths.md @@ -0,0 +1,12 @@ +--- +'@object-ui/app-shell': patch +--- + +The flow designer's three ways to add a node agree: each opens the add-node palette, the picked node goes into the flow's path and is laid out, and the node inspector's Node Type list is the palette's list (objectui#11778). + +- **The "+" on an edge and a node's "Add connected node" ask for the type.** Both used to add a "Create record" node at once. They now open the same grouped, searchable palette as the toolbar's Add node, with its Recently used group, and add the node you pick. +- **The new node goes into the path.** The "+" on an edge splits that edge, as before. Add connected node and the toolbar's Add node put the node after a node: the node whose "+" you pressed, or for the toolbar the selected node, else Start. When that node has one way on, the new node goes between it and the next node. When it has none, the new node follows it. A decision, an approval, or a node that already leads to two or more nodes gets the new node as a new branch, as before. With End selected, the toolbar puts the new node before End. The toolbar used to start a second branch from the selected node, or add a node with no connection at all when nothing was selected. +- **The new node is laid out.** A node inserted on an edge was pinned halfway between the edge's two ends, on top of the card above it. It now has no stored position, so the canvas lays it out on its own row and moves the nodes below it down. Nodes you have dragged keep their positions. +- **The Node Type list is the palette's.** It offers every node type the palette offers, under the palette's names, so a node can now be changed into a Notify node. It no longer shows raw type names such as `http_request`, `connector_action` or `try_catch`. It no longer offers a type the palette does not add, such as Start, or `http_request`, the older spelling of the palette's HTTP request (`http`). A node whose stored type the palette does not list, such as Start, still shows that type under its name. + +Nothing is added to the package entry: no export, prop, type member or language-pack key. diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.connectorInput.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.connectorInput.test.tsx index 835fd49ad7..1453c9daca 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.connectorInput.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.connectorInput.test.tsx @@ -294,7 +294,9 @@ describe('#4305 — pins that must NOT move', () => { mockRegistry([{ key: 'chat.postMessage', label: 'Post Message', inputSchema: SLACK_POST_MESSAGE }]); renderInspector(makeDraft()); - await waitFor(() => expect(screen.getByText('Connector')).toBeTruthy()); + // The field's label: since objectui#11778 the Node Type select names this + // node's type "Connector" too (the palette's name), not `connector_action`. + await waitFor(() => expect(screen.getByText('Connector', { selector: 'label' })).toBeTruthy()); expect(screen.getByText('Action')).toBeTruthy(); expect(screen.getByDisplayValue('slack')).toBeTruthy(); expect(screen.getByDisplayValue('chat.postMessage')).toBeTruthy(); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx index 2ba1551de2..429e399658 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.declaredDefault.test.tsx @@ -79,7 +79,7 @@ vi.mock('../previews/useObjectFields', () => ({ })); import { FlowNodeInspector } from './FlowNodeInspector'; -import { fieldsForNodeType, FLOW_NODE_TYPE_OPTIONS } from './flow-node-config'; +import { fieldsForNodeType, FLOW_NODE_CONFIG_TYPES } from './flow-node-config'; import type { MetadataSelection } from '../preview-registry'; // The runtime's own answer for an omitted `escalation` key. Imported so the // #6620 rows below compare the RENDERED control against the installed contract @@ -989,19 +989,21 @@ describe('the declaration surface this card names', () => { * here; that is a result, not a method. Whoever lands next re-reads the * number the same way rather than trusting this sentence. * - * Swept over the picker's node types plus the four that carry config but are - * not offered in the picker (ADR-0031 import/export-only, and the legacy - * aliases). The sweep is asserted to cover the picker so a new node type - * cannot slip a declaration past it. + * Swept over every group the table holds (`FLOW_NODE_CONFIG_TYPES`, the + * table's own keys), so a new node type cannot slip a declaration past it. + * Until objectui#11778 this walked the inspector's hand-kept type list plus + * the types that carry config but that list never offered (ADR-0031 + * import/export-only, the legacy aliases, and `notify`); the table's keys + * hold every one of them, which the first assertion re-reads. */ const OFF_PICKER_TYPES = ['boundary_event', 'parallel_gateway', 'join_gateway', 'legacy_action', 'notify']; it('exactly ten fields declare a defaultValue, and these are they', () => { - const swept = [...FLOW_NODE_TYPE_OPTIONS, ...OFF_PICKER_TYPES]; + const swept = FLOW_NODE_CONFIG_TYPES; expect( - FLOW_NODE_TYPE_OPTIONS.every((t) => swept.includes(t)), - 'the sweep covers every node type the picker offers', - ).toBe(true); + swept, + 'the sweep walks the whole table, the types the retired hand list never offered included', + ).toEqual(expect.arrayContaining(OFF_PICKER_TYPES)); const declaring = new Set(); for (const type of swept) { @@ -1040,7 +1042,7 @@ describe('the declaration surface this card names', () => { * whose growth should be visible in review. */ it('every select-kind declaring field renders its declared default', () => { - const swept = [...FLOW_NODE_TYPE_OPTIONS, ...OFF_PICKER_TYPES]; + const swept = FLOW_NODE_CONFIG_TYPES; const cases: Array<{ id: string; type: string; label: string; expected: string }> = []; for (const type of swept) { for (const field of fieldsForNodeType(type)) { diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.endOutputVariable-9335.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.endOutputVariable-9335.test.tsx index 807871f5bf..a947200ae1 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.endOutputVariable-9335.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.endOutputVariable-9335.test.tsx @@ -69,7 +69,7 @@ vi.mock('../previews/useObjectFields', () => ({ })); import { FlowNodeInspector } from './FlowNodeInspector'; -import { fieldsForNodeType, FLOW_NODE_TYPE_OPTIONS } from './flow-node-config'; +import { fieldsForNodeType, FLOW_NODE_CONFIG_TYPES } from './flow-node-config'; import type { MetadataSelection } from '../preview-registry'; // ⛔ The `/automation` subpath is load bearing — these read `undefined` off the // package root, and any `.parse` on them then throws. @@ -221,7 +221,7 @@ describe('the end group no longer offers the key (objectui#9335)', () => { // this table, and on the CRUD / script / subflow / http / map / legacy // groups the key IS declared by the spec and legitimate. Derived by sweep, // never restated as a count. - const types = [...FLOW_NODE_TYPE_OPTIONS, 'action']; + const types = [...FLOW_NODE_CONFIG_TYPES, 'action']; const stillOffering = types.filter((t) => fieldsForNodeType(t).some((f) => f.id === 'outputVariable')); expect( diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.loopItemVariable-9340.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.loopItemVariable-9340.test.tsx index 0ba7d0c7bb..659a9176df 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.loopItemVariable-9340.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.loopItemVariable-9340.test.tsx @@ -63,7 +63,7 @@ vi.mock('../previews/useObjectFields', () => ({ })); import { FlowNodeInspector } from './FlowNodeInspector'; -import { fieldsForNodeType, localizeFlowFields, FLOW_NODE_TYPE_OPTIONS } from './flow-node-config'; +import { fieldsForNodeType, localizeFlowFields, FLOW_NODE_CONFIG_TYPES } from './flow-node-config'; import type { MetadataSelection } from '../preview-registry'; // ⛔ The `/automation` subpath is load bearing — these read `undefined` off the // package root, and any `.parse` on them then throws. @@ -236,7 +236,7 @@ describe('the loop group hints the identifier the spec applies (objectui#9340)', // The lit control: descriptors in this SAME table do carry the slot, so the // `undefined` above is a deliberate non-declaration, not a shape that can // never hold one. - const declaring = FLOW_NODE_TYPE_OPTIONS.flatMap((t) => fieldsForNodeType(t)).filter((f) => f.defaultValue !== undefined); + const declaring = FLOW_NODE_CONFIG_TYPES.flatMap((t) => fieldsForNodeType(t)).filter((f) => f.defaultValue !== undefined); expect(declaring.length, 'other rows in this table DO declare a default').toBeGreaterThan(0); }); @@ -259,11 +259,11 @@ describe('the loop group hints the identifier the spec applies (objectui#9340)', }); it('sweeps EVERY node type — so a third group growing the key cannot drift unseen', () => { - // The structural identification, generalised: walk every type the picker - // offers, keep the ones whose group owns this key, and hold each against + // The structural identification, generalised: walk every type the table + // holds a group for, keep the ones whose group owns this key, and hold each against // the identifier ITS OWN schema applies. `map` is in here as the live lit // control — if the fenced-off twin ever drifts, this arm reddens. - const offering = FLOW_NODE_TYPE_OPTIONS.filter((t) => iteratorRowOf(t) !== undefined); + const offering = FLOW_NODE_CONFIG_TYPES.filter((t) => iteratorRowOf(t) !== undefined); expect( offering.length, diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.nodeTypeOptions-11778.test.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.nodeTypeOptions-11778.test.tsx new file mode 100644 index 0000000000..257ba2dd27 --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.nodeTypeOptions-11778.test.tsx @@ -0,0 +1,133 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11778 — the inspector's Node Type select offers the add-node + * palette's list, with the palette's display names. + * + * It was a hand list (`FLOW_NODE_TYPE_OPTIONS`) that left out `notify`, so a + * node could not be turned into the Notify the palette adds, and it showed raw + * type names (`http_request`, `connector_action`, `try_catch`). The palette + * hook runs for real here: the engine overlay answers "absent" (the palette's + * offline answer is the hardcoded `NODE_PALETTE`) except in the one case that + * serves an engine descriptor, which the select must then offer too. + */ + +import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'; +import { render, screen, cleanup, act } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +vi.mock('../previews/useObjectFields', () => ({ + useObjectFields: () => ({ fields: [], loading: false, error: null }), +})); + +import { FlowNodeInspector } from './FlowNodeInspector'; +import { NODE_PALETTE } from '../previews/flow-canvas-parts'; +import { t, type SupportedLocale } from '../i18n'; +import type { MetadataSelection } from '../preview-registry'; + +const ACTIONS_ROUTE = '/api/v1/automation/actions'; +let engineActions: unknown[] | null = null; + +beforeEach(() => { + engineActions = null; + vi.stubGlobal( + 'fetch', + vi.fn(async (input: unknown) => { + const url = String(input).split('?')[0]; + if (url === ACTIONS_ROUTE && engineActions) { + return new Response(JSON.stringify({ data: { actions: engineActions } }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + } + return new Response('not found', { status: 404 }); + }), + ); +}); + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); +}); + +type Node = { id: string; type: string; label?: string }; + +function mount(node: Node, locale: SupportedLocale = 'en-US') { + const onPatch = vi.fn(); + render( + , + ); + return { onPatch }; +} + +const typeSelect = (locale: SupportedLocale = 'en-US') => + screen.getByRole('combobox', { name: t('engine.inspector.flowNode.type', locale) }); + +async function optionNames(locale: SupportedLocale = 'en-US'): Promise { + await userEvent.click(typeSelect(locale)); + return (await screen.findAllByRole('option')).map((o) => o.textContent ?? ''); +} + +describe('the Node Type select is the palette\'s list (objectui#11778)', () => { + it('offers every palette type under its display name, in the palette\'s order — Notify included', async () => { + mount({ id: 'n1', type: 'create_record', label: 'Create record' }); + const names = await optionNames(); + // Derived from the palette's own registry, never restated. + expect(names).toEqual(NODE_PALETTE.map((p) => p.label)); + expect(names).toContain('Notify'); + }); + + it('shows no raw type name — not even for the types the hand list spelled raw', async () => { + mount({ id: 'n1', type: 'create_record', label: 'Create record' }); + const names = await optionNames(); + for (const raw of ['http_request', 'connector_action', 'try_catch', 'create_record', 'notify']) { + expect(names, raw).not.toContain(raw); + } + }); + + it('picking Notify turns the node into a notify node', async () => { + const { onPatch } = mount({ id: 'n1', type: 'create_record', label: 'Create record' }); + await optionNames(); + await userEvent.click(screen.getByRole('option', { name: 'Notify' })); + expect(onPatch).toHaveBeenCalled(); + const patch = onPatch.mock.calls.at(-1)![0] as { nodes: Node[] }; + expect(patch.nodes.find((n) => n.id === 'n1')!.type).toBe('notify'); + }); + + it('zh-CN reads the palette\'s localized names', async () => { + mount({ id: 'n1', type: 'create_record', label: '创建记录' }, 'zh-CN'); + const names = await optionNames('zh-CN'); + expect(names).toContain('通知'); + expect(names).toContain('创建记录'); + expect(names).not.toContain('Notify'); + }); + + it('a stored type the palette does not offer shows under its display name, for that node only', async () => { + mount({ id: 'start2', type: 'start', label: 'Begin' }); + expect(typeSelect().textContent, 'the trigger names the type, not the raw `start`').toBe('Start'); + const names = await optionNames(); + expect(names.filter((n) => n === 'Start')).toHaveLength(1); + cleanup(); + mount({ id: 'n1', type: 'create_record', label: 'Create record' }); + expect(await optionNames(), 'another node is not offered Start').not.toContain('Start'); + }); + + it('an engine-published node type joins the list, as it joins the palette', async () => { + engineActions = [{ type: 'map', name: 'Map items', description: 'Run a subflow per item', paradigms: ['flow'] }]; + mount({ id: 'n1', type: 'create_record', label: 'Create record' }); + // The engine answers after the first paint; let its answer land first. + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + }); + expect(await optionNames()).toContain('Map items'); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.tsx index 4bb28cf016..c176e205fa 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/FlowNodeInspector.tsx @@ -41,10 +41,8 @@ import { switchedBlockOf, isBareSwitchedOffBlock, configKeyOf, - FLOW_NODE_TYPE_OPTIONS, type FlowConfigField, } from './flow-node-config.js'; -import { translateNodeLabel } from '../i18n.js'; import { jsonSchemaToFlowFields } from './json-schema-to-fields.js'; import { applyConnectorInputForm, @@ -55,7 +53,8 @@ import { useConnectorRegistry, } from './connector-input-fields.js'; import { applyDecisionBranches, syncDecisionEdgesByOrder, withBranchTargets } from './flow-decision-edges.js'; -import { useActionConfigSchemas } from '../previews/useFlowNodePalette.js'; +import { useActionConfigSchemas, useFlowNodePalette } from '../previews/useFlowNodePalette.js'; +import { defaultNodeLabel, paletteTypeOptions } from '../previews/flow-canvas-parts.js'; import { FlowNodeConfigField } from './FlowNodeConfigField.js'; import { specRequiredColumns, specRequiresField } from './flow-required-keys.js'; import { useFlowScope } from './useFlowScope.js'; @@ -178,6 +177,9 @@ export function FlowNodeInspector({ selection, draft, onPatch, onClearSelection, // with the backend. Falls back to the hardcoded field group when no schema is // published (offline / plugin absent / older backend). const configSchemas = useActionConfigSchemas(); + // objectui#11778 — the Node Type select offers the add-node palette's types + // with its display names: the list the canvas adds from, engine-merged. + const paletteItems = useFlowNodePalette(); // A nested node anchors its scope on the container (ADR-0031 outer scope). The // container's own outputs — a loop's iteratorVariable — are excluded from the // graph walk at its id, so inject the loop group explicitly for a body node. @@ -443,9 +445,15 @@ export function FlowNodeInspector({ selection, draft, onPatch, onClearSelection, onClearSelection(); }; - const typeOptions = FLOW_NODE_TYPE_OPTIONS.includes(node.type as (typeof FLOW_NODE_TYPE_OPTIONS)[number]) - ? [...FLOW_NODE_TYPE_OPTIONS] - : [...FLOW_NODE_TYPE_OPTIONS, node.type ?? ''].filter(Boolean); + // objectui#11778 — the palette's list (it once was a hand list here that + // missed `notify` and showed raw type names). A stored type the palette does + // not offer (`start`, an alias like `http_request`, a plugin type whose engine + // is not answering) is added for THIS node only, under its display name, so + // the select still shows what the node is; no other node is offered it. + const typeOptions = paletteTypeOptions(paletteItems, locale); + if (node.type && !typeOptions.some((o) => o.value === node.type)) { + typeOptions.push({ value: node.type, label: defaultNodeLabel(node.type, locale) }); + } // A nested node has no structural editing this phase (no delete, id is // read-only — those live on the container's Advanced JSON). @@ -482,7 +490,7 @@ export function FlowNodeInspector({ selection, draft, onPatch, onClearSelection, ({ value: v, label: translateNodeLabel(v, locale, v) }))} + options={typeOptions} onCommit={(v) => patchNode({ type: v })} disabled={readOnly} /> diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.inactiveRetained.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.inactiveRetained.test.ts index c17cc83504..e5a93e6101 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.inactiveRetained.test.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.inactiveRetained.test.ts @@ -23,7 +23,7 @@ import { fieldsForNodeType, isFieldVisible, inactiveRetainedKind, - FLOW_NODE_TYPE_OPTIONS, + FLOW_NODE_CONFIG_TYPES, type FlowConfigField, } from './flow-node-config.js'; import { jsonSchemaToFlowFields } from './json-schema-to-fields.js'; @@ -131,7 +131,7 @@ describe('inactiveRetainedKind — mechanical coverage of every showWhen group', 'subflow', 'notify', 'connector_action', 'parallel', 'try_catch', 'parallel_gateway', 'join_gateway', 'boundary_event', 'legacy_action', ]; - const TYPES = [...new Set([...CANONICAL, ...FLOW_NODE_TYPE_OPTIONS, ...TYPE_ALIASES])]; + const TYPES = [...new Set([...CANONICAL, ...FLOW_NODE_CONFIG_TYPES, ...TYPE_ALIASES])]; /** Write `value` at `field.path` on a fresh node object. */ function nodeWith(type: string, entries: Array<[string[], unknown]>) { diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.spec-reconciliation.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.spec-reconciliation.test.ts index 9130a01b5a..8bd891abdd 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.spec-reconciliation.test.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.spec-reconciliation.test.ts @@ -38,7 +38,7 @@ import * as Automation from '@objectstack/spec/automation'; // The Zod wrapper-key vocabulary — one list, read by the `.mjs` CI gates that // walk the same internals (objectui#6923, ruled 2026-08-31). import { ZOD_WRAPPER_KEYS } from '@object-ui/test-support'; -import { fieldsForNodeType, FLOW_NODE_TYPE_OPTIONS, type FlowConfigField } from './flow-node-config'; +import { fieldsForNodeType, FLOW_NODE_CONFIG_TYPES, type FlowConfigField } from './flow-node-config'; // Feature-detected exports — absent on a spec that predates framework#4278. // (Truthiness alone never resolves a lazySchema proxy.) @@ -369,9 +369,10 @@ describe('declared defaults ↔ per-node-type spec schemas (#6794, #6620, object // sweep of their own: `fieldsForNodeType` resolves every alias to one of // these canonical tables, so walking the table keys walks every field. expect(NODE_TYPES.length, 'FLOW_NODE_CONFIG declares node types').toBeGreaterThan(20); - expect(NODE_TYPES, 'and the picker types are among them').toEqual( - expect.arrayContaining([...FLOW_NODE_TYPE_OPTIONS]), - ); + expect( + [...NODE_TYPES].sort(), + 'and they are exactly the table types the module exports (objectui#11778)', + ).toEqual([...FLOW_NODE_CONFIG_TYPES].sort()); expect(NODE_TYPES, 'including the off-picker tables a picker-only sweep would miss').toEqual( expect.arrayContaining(['boundary_event', 'notify', 'legacy_action', 'join_gateway']), ); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts index 964f676af4..ce82023935 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/flow-node-config.ts @@ -1619,27 +1619,14 @@ export function inactiveRetainedKind( return fields.some((f) => f.id === field.showWhen!.field) ? 'controller-off' : 'no-controller'; } -/** Node types offered in the inspector's type picker (spec `FlowNodeAction`). */ -export const FLOW_NODE_TYPE_OPTIONS = [ - 'start', - 'create_record', - 'update_record', - 'delete_record', - 'get_record', - 'decision', - 'assignment', - 'loop', - 'http_request', - 'script', - 'screen', - 'approval', - 'wait', - 'subflow', - 'map', - 'connector_action', - // ADR-0031: structured constructs replace the BPMN gateway/boundary types in - // the picker — those remain import/export-only (no engine executor). - 'parallel', - 'try_catch', - 'end', -] as const; +/** + * Every node type this table holds a hand-written field group for — its keys, + * the canonical types the aliases in `TYPE_ALIASES` resolve onto through + * {@link fieldsForNodeType}. The population a sweep over the table walks. + * + * ⛔ Not the inspector's Node Type list. That list is the add-node palette's + * (objectui#11778): it replaced the hand list `FLOW_NODE_TYPE_OPTIONS` that + * stood here, which offered `http_request` (the engine's deprecated alias of + * `http`) and missed `notify`, a type the palette adds. + */ +export const FLOW_NODE_CONFIG_TYPES: readonly string[] = Object.keys(FLOW_NODE_CONFIG); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/flow-value-envelope.test.ts b/packages/app-shell/src/views/metadata-admin/inspectors/flow-value-envelope.test.ts index 6a5275fa90..83475fb64d 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/flow-value-envelope.test.ts +++ b/packages/app-shell/src/views/metadata-admin/inspectors/flow-value-envelope.test.ts @@ -20,7 +20,7 @@ import { valueEnvelopeRefusal, writeValueEnvelope, } from './flow-value-envelope'; -import { FLOW_NODE_TYPE_OPTIONS, fieldsForNodeType } from './flow-node-config'; +import { FLOW_NODE_CONFIG_TYPES, fieldsForNodeType } from './flow-node-config'; describe('isValueEnvelopeSlot — read off the spec expression ledger (objectui#7588)', () => { it('answers true for the assignment node’s `assignments` map', () => { @@ -44,7 +44,7 @@ describe('isValueEnvelopeSlot — read off the spec expression ledger (objectui# // It went red at `@objectstack/spec` 17.5.0, which declares // `create_record` / `update_record` `fields.*` as `value`-role slots, and the // confirmation it asks for is the next case (objectui#11073). - const keyValueFields = FLOW_NODE_TYPE_OPTIONS.flatMap((type) => + const keyValueFields = FLOW_NODE_CONFIG_TYPES.flatMap((type) => fieldsForNodeType(type) .filter((f) => f.kind === 'keyValue') .map((f) => ({ type, path: f.path })), diff --git a/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.addNodePaths-11778.test.tsx b/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.addNodePaths-11778.test.tsx new file mode 100644 index 0000000000..04b7d8722f --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.addNodePaths-11778.test.tsx @@ -0,0 +1,242 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#11778 — the flow canvas's three ways to add a node agree. + * + * Before the fix, on the `start → end` draft Studio creates a new flow with: + * + * - the toolbar's Add node put the picked node beside the path (a second + * branch `start → N` with Start selected, no edge at all with nothing + * selected); + * - an edge's "+" and a node's "Add connected node" never asked for a type: + * each added a `create_record`, the edge's pinned at its endpoints' + * midpoint, on top of the Start card. + * + * Now each "+" opens the one add-node palette, the pick goes INTO the path, + * and the node is left to the layered auto-layout. Driven through the real + * canvas over a host that merges each patch into the draft and keeps the + * selection the canvas reports, as Studio does. Palette rows are pressed with + * `browserClick`, which routes the click the way a browser does after a + * pointer capture (objectui#11546): a palette opened from an edge or a card + * lives inside the canvas viewport in the React tree. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { render, screen, cleanup, fireEvent, within } from '@testing-library/react'; +import { FlowCanvas } from './FlowCanvas'; +import { NODE_H, type FlowDesignerEdge, type FlowDesignerNode } from './flow-canvas-layout'; +import { browserClick } from './__tests__/browserClick'; + +// The palette's engine overlay answers "engine absent": the canvas then offers +// the hardcoded palette, its offline answer. Installed for the whole file so no +// read flushed after a test body reaches a real socket. +vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response('not found', { status: 404 })), +); + +afterEach(() => { + cleanup(); + // The palette's "Recently used" group lives in localStorage outside a provider. + localStorage.clear(); +}); + +type Draft = { nodes: FlowDesignerNode[]; edges: FlowDesignerEdge[] }; + +/** The draft Studio's "New flow" saves (`buildFlowSkeleton`). */ +const skeleton = (): Draft => ({ + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'end' }], +}); + +function mount(initial: Draft, selected: string | null = null) { + const probe = { draft: initial, patches: 0 }; + function Host() { + const [draft, setDraft] = React.useState(initial); + const [selectedId, setSelectedId] = React.useState(selected); + React.useEffect(() => { + probe.draft = draft; + }, [draft]); + return ( + setSelectedId(n?.id ?? null)} + onPatch={(patch) => { + probe.patches += 1; + setDraft((d) => ({ ...d, ...(patch as Partial) })); + }} + /> + ); + } + const utils = render(); + return { probe, ...utils }; +} + +const pairs = (d: Draft) => d.edges.map((e) => `${e.source}->${e.target}`); +const added = (d: Draft) => d.nodes.filter((n) => n.id !== 'start' && n.id !== 'end'); + +/** Press the open palette's row for `label` the way a browser routes the click. */ +function pick(label: string) { + const option = screen.getAllByRole('option').find((o) => o.textContent?.startsWith(label)); + expect(option, `the open palette offers ${label}`).toBeDefined(); + browserClick(option!); +} + +const edgePlus = () => screen.getByRole('button', { name: 'Insert node here' }); +const toolbarAdd = () => screen.getByRole('button', { name: 'Add node' }); +function cardPlus(container: HTMLElement, id: string) { + const card = container.querySelector(`[data-node-id="${id}"]`) as HTMLElement; + return within(card).getByRole('button', { name: 'Add connected node' }); +} +const topOf = (container: HTMLElement, id: string) => + parseFloat((container.querySelector(`[data-node-id="${id}"]`) as HTMLElement).style.top); + +describe('every "+" opens the add-node palette, and nothing lands before a pick (objectui#11778)', () => { + it('the edge "+": no node yet, the grouped palette is open', () => { + const { probe } = mount(skeleton()); + fireEvent.click(edgePlus()); + expect(probe.patches, 'the "+" alone adds nothing').toBe(0); + expect(screen.getByPlaceholderText('Search nodes…')).toBeInTheDocument(); + expect(screen.getByText('Integration')).toBeInTheDocument(); + }); + + it('a node card\'s "Add connected node": no node yet, the grouped palette is open', () => { + const { probe, container } = mount(skeleton()); + fireEvent.click(cardPlus(container, 'start')); + expect(probe.patches).toBe(0); + expect(screen.getByPlaceholderText('Search nodes…')).toBeInTheDocument(); + }); +}); + +describe('the edge "+" inserts the picked type between the edge\'s endpoints (objectui#11778)', () => { + it('picking Notify on start → end gives start → notify → end, one edge in and one out', () => { + const { probe } = mount(skeleton()); + fireEvent.click(edgePlus()); + pick('Notify'); + const [node] = added(probe.draft); + expect(node.type).toBe('notify'); + expect(pairs(probe.draft)).toEqual([`start->${node.id}`, `${node.id}->end`]); + // The inserted segment keeps the edge's identity; the new one carries on. + expect(probe.draft.edges[0].id).toBe('e1'); + }); + + it('the inserted node is not pinned: the layered layout gives it its own layer, clear of Start', () => { + const { probe, container } = mount(skeleton()); + fireEvent.click(edgePlus()); + pick('Notify'); + const [node] = added(probe.draft); + expect(node.position, 'no stored position').toBeUndefined(); + const start = topOf(container, 'start'); + const mid = topOf(container, node.id); + const end = topOf(container, 'end'); + expect(mid - start, 'below the Start card, not on top of it').toBeGreaterThanOrEqual(NODE_H); + expect(end - mid, 'and End moves down below it').toBeGreaterThanOrEqual(NODE_H); + }); +}); + +describe('"Add connected node" puts the picked type after its node, in the path (objectui#11778)', () => { + it('on Start of start → end: start → N → end, no second branch from Start', () => { + const { probe, container } = mount(skeleton()); + fireEvent.click(cardPlus(container, 'start')); + pick('Notify'); + const [node] = added(probe.draft); + expect(node.type).toBe('notify'); + expect(pairs(probe.draft)).toEqual([`start->${node.id}`, `${node.id}->end`]); + expect(node.position).toBeUndefined(); + }); + + it('on a node with no way on: appends after it', () => { + const { probe, container } = mount({ + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'a', type: 'script', label: 'A' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'a' }], + }); + fireEvent.click(cardPlus(container, 'a')); + pick('Create record'); + const node = probe.draft.nodes.find((n) => !['start', 'a'].includes(n.id))!; + expect(pairs(probe.draft)).toEqual(['start->a', `a->${node.id}`]); + }); + + it('on a decision: a new branch from it, carrying its next condition — a decision has no single path on', () => { + const { probe, container } = mount({ + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { + id: 'd', + type: 'decision', + label: 'Big?', + config: { conditions: [{ label: 'Big', expression: 'amount > 100' }, { label: 'Else', expression: 'true' }] }, + }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'd' }, + { id: 'e2', source: 'd', target: 'end', label: 'Big', condition: 'amount > 100' }, + ], + }); + fireEvent.click(cardPlus(container, 'd')); + pick('Notify'); + const node = probe.draft.nodes.find((n) => n.type === 'notify')!; + expect(pairs(probe.draft)).toEqual(['start->d', 'd->end', `d->${node.id}`]); + expect(probe.draft.edges[2]).toMatchObject({ source: 'd', target: node.id, label: 'Else', isDefault: true }); + }); +}); + +describe('the toolbar\'s Add node puts the picked type after the selected node, else after Start (objectui#11778)', () => { + it('nothing selected, on start → end: start → N → end — never a node wired to nothing', () => { + const { probe } = mount(skeleton()); + fireEvent.click(toolbarAdd()); + pick('Notify'); + const [node] = added(probe.draft); + expect(pairs(probe.draft)).toEqual([`start->${node.id}`, `${node.id}->end`]); + }); + + it('Start selected: the same — not the card\'s second branch start → notify beside start → end', () => { + const { probe } = mount(skeleton(), 'start'); + fireEvent.click(toolbarAdd()); + pick('Notify'); + const [node] = added(probe.draft); + expect(pairs(probe.draft)).toEqual([`start->${node.id}`, `${node.id}->end`]); + expect(pairs(probe.draft)).not.toContain('start->end'); + }); + + it('End selected: the node goes before End — an End has no "after"', () => { + const { probe } = mount( + { + nodes: [...skeleton().nodes, { id: 'a', type: 'script', label: 'A' }], + edges: [ + { id: 'e1', source: 'start', target: 'a' }, + { id: 'e2', source: 'a', target: 'end' }, + ], + }, + 'end', + ); + fireEvent.click(toolbarAdd()); + pick('Notify'); + const node = probe.draft.nodes.find((n) => n.type === 'notify')!; + expect(pairs(probe.draft)).toEqual(['start->a', `a->${node.id}`, `${node.id}->end`]); + }); + + it('adds in a row build the path in order, because the new node is the selection the next add follows', () => { + const { probe } = mount(skeleton()); + for (const label of ['Notify', 'Create record', 'Script']) { + fireEvent.click(toolbarAdd()); + pick(label); + } + const [n1, n2, n3] = added(probe.draft); + expect([n1.type, n2.type, n3.type]).toEqual(['notify', 'create_record', 'script']); + expect(pairs(probe.draft).sort()).toEqual( + [`start->${n1.id}`, `${n1.id}->${n2.id}`, `${n2.id}->${n3.id}`, `${n3.id}->end`].sort(), + ); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.test.tsx b/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.test.tsx index 360d05a34c..364e815a31 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.test.tsx @@ -468,14 +468,23 @@ describe('FlowCanvas — geometry writes are spec-canonical `position` (#3172)', expectNoLegacyKey(nodes); }); - it('insert-on-edge → the new node is pinned via `position`', () => { + /** The open add-node palette's row for `label` (objectui#11778: every "+" opens it). */ + const pickInPalette = (label: string) => { + const option = screen.getAllByRole('option').find((o) => o.textContent?.startsWith(label)); + expect(option, `palette row ${label}`).toBeDefined(); + fireEvent.click(option!); + }; + + it('insert-on-edge → the new node is auto-laid (objectui#11778), legacy node still healed', () => { const onPatch = vi.fn(); renderCanvas(onPatch); fireEvent.click(screen.getByRole('button', { name: 'Insert node here' })); + pickInPalette('Create record'); const nodes = patchedNodes(onPatch); expect(nodes).toHaveLength(3); - const inserted = nodes[2].position as { x: number; y: number }; - expect(Number.isFinite(inserted.x) && Number.isFinite(inserted.y)).toBe(true); + // Unpinned, like an append: the layered layout gives it its own layer. + expect(nodes[2].position).toBeUndefined(); + expect(nodes.find((n) => n.id === 'b')!.position).toEqual({ x: 400, y: 120 }); expectNoLegacyKey(nodes); }); @@ -483,6 +492,7 @@ describe('FlowCanvas — geometry writes are spec-canonical `position` (#3172)', const onPatch = vi.fn(); renderCanvas(onPatch); fireEvent.click(screen.getAllByRole('button', { name: 'Add connected node' })[0]); + pickInPalette('Create record'); const nodes = patchedNodes(onPatch); expect(nodes).toHaveLength(3); // An appended node is deliberately unpinned (the layered layout slots it). diff --git a/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.tsx b/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.tsx index 936cca5040..9cd475ce51 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.tsx +++ b/packages/app-shell/src/views/metadata-admin/previews/FlowCanvas.tsx @@ -10,8 +10,11 @@ * * - drag to reposition nodes (committed to the spec's `node.position = {x,y}` * on drop — objectui#3172), - * - add nodes from a palette (toolbar or a node's bottom "+" handle), - * - insert a node on an edge ("+" at the edge midpoint splits A→B), + * - add a node from the ONE add-node palette, whichever "+" opened it + * (objectui#11778): the toolbar's Add node and a node's bottom "+" put it + * after that node in its path (`placeAfter`), the "+" at an edge's + * midpoint splits that edge A→B into A→N→B; the new node is left unpinned + * so the layered auto-layout places it, * - delete the selected node (Delete/Backspace) with full edge cleanup, * - pan (background drag) and zoom / fit-to-view. * @@ -175,7 +178,10 @@ export function FlowCanvas({ const viewportRef = React.useRef(null); const [zoom, setZoom] = React.useState(1); const [pan, setPan] = React.useState({ x: 0, y: 0 }); - const [paletteOpen, setPaletteOpen] = React.useState(false); + // objectui#11778 — which "+" has the add-node palette open: `toolbar`, + // `edge:` or `node:`; null when none. One key, so at most one + // palette is ever open and a pick closes it wherever it was opened. + const [paletteAt, setPaletteAt] = React.useState(null); // Node types offered by the add-node palette, driven by the engine's // published descriptors (`GET /api/v1/automation/actions`) merged with the // hardcoded base — so the palette reflects what the backend actually supports @@ -328,27 +334,29 @@ export function FlowCanvas({ } onPatch(patch); onSelect(newNode); - setPaletteOpen(false); + setPaletteAt(null); }, [edges, nodes, onPatch, onSelect, positionOf, locale, newNodeId], ); - /** Split edge A→B by inserting a new node N: A→N (keeps guard) + N→B. */ + /** + * Split edge A→B by inserting a new node N of the picked `type`: A→N (keeps + * guard) + N→B. objectui#11778 — N is left unpinned, like a `from`-append, so + * the layered auto-layout gives it its own layer between A and B. Pinning it + * at the endpoints' midpoint (the old behavior) dropped it half a layer below + * A, on top of the cards it sat between. + */ const insertOnEdge = React.useCallback( - (edge: FlowDesignerEdge, type = 'create_record') => { + (edge: FlowDesignerEdge, type: string) => { if (!onPatch) return; const edgeIdx = edges.findIndex((e) => e === edge); if (edgeIdx < 0) return; const id = newNodeId(); - const from = positionOf(edge.source); - const to = positionOf(edge.target); - const at = { x: (from.x + to.x) / 2, y: (from.y + to.y) / 2 }; const newNode: FlowDesignerNode = { id, type, label: defaultNodeLabel(type, locale), ...defaultNodeExtras(type), - position: { x: at.x, y: at.y }, }; // A→N inherits the original edge's branch semantics; N→B is plain. const firstSegment: FlowDesignerEdge = { ...edge, target: id }; @@ -360,10 +368,34 @@ export function FlowCanvas({ const nextEdges = spliceArray(edges, edgeIdx, firstSegment); onPatch({ nodes: appendArray(nodes, newNode), edges: appendArray(nextEdges, secondSegment) }); onSelect(newNode); + setPaletteAt(null); }, - [edges, nodes, onPatch, onSelect, positionOf, locale, newNodeId], + [edges, nodes, onPatch, onSelect, locale, newNodeId], + ); + + /** + * objectui#11778 — add a node of the picked `type` AFTER `anchorId`, in its + * path: the one rule the toolbar's Add node (anchor: the selected node, else + * Start) and a node's bottom "+" (anchor: that node) share. `placeAfter` + * decides where; this only dispatches to the insert or the append. + */ + const addAfter = React.useCallback( + (anchorId: string | null, type: string) => { + const place = placeAfter(anchorId, nodes, edges); + if (place.kind === 'split') insertOnEdge(place.edge, type); + else addNode(type, place.kind === 'from' ? { from: place.from } : undefined); + }, + [nodes, edges, insertOnEdge, addNode], ); + /** Open/close wiring for the add-node palette behind one "+" (see `paletteAt`). */ + const paletteFor = (key: string) => ({ + locale, + items: paletteItems, + open: paletteAt === key, + onOpenChange: (open: boolean) => setPaletteAt(open ? key : null), + }); + /** * ADR-0044 one-click "add revision loop": drop a signal `wait` node plus the * two edges that form a send-back-for-revision loop on an approval node — @@ -622,13 +654,7 @@ export function FlowCanvas({ {/* Toolbar */}
{editable && ( - addNode(type, { from: selectedId ?? undefined })} - > + addAfter(selectedId, type)}> + {/* objectui#11778 — the same palette as the toolbar's; + the pick splits THIS edge. */} + insertOnEdge(edge, type)}> + + )} @@ -932,7 +959,10 @@ export function FlowCanvas({ dimmed={simRunning && !runState} onPointerDown={onNodePointerDown(node.id)} onSelect={() => designMode && onSelect(node)} - onAppend={() => addNode('create_record', { from: node.id })} + onAppend={(type) => addAfter(node.id, type)} + paletteItems={paletteItems} + appendPaletteOpen={paletteAt === `node:${node.id}`} + onAppendPaletteOpenChange={(open) => setPaletteAt(open ? `node:${node.id}` : null)} onAddReviseLoop={ editable && node.type === 'approval' && !reviseLoopSources.has(node.id) ? () => addReviseLoop(node.id) @@ -964,6 +994,56 @@ export function FlowCanvas({ ); } +/** + * Node types whose out-edges ARE their outcomes: a decision's branches, an + * approval's `approve` / `reject`, a BPMN fan-out. A node added after one of + * these is a new outcome, never a step spliced into an outcome already wired. + */ +const BRANCHING_NODE_TYPES: ReadonlySet = new Set(['decision', 'approval', 'parallel_gateway']); + +/** Where `placeAfter` puts a new node. */ +type Placement = + /** Split this edge, exactly as its own "+" would. */ + | { kind: 'split'; edge: FlowDesignerEdge } + /** A new out-edge from this node (a decision carries its next branch). */ + | { kind: 'from'; from: string } + /** No edge at all: there is no path to put it in. */ + | { kind: 'loose' }; + +/** + * objectui#11778 — the one rule for "add a node after this one", so the + * toolbar's Add node and a node's bottom "+" can never disagree. The anchor is + * `anchorId` when it names a node, else the flow's Start. + * + * - a branching anchor (`BRANCHING_NODE_TYPES`), or one that already fans + * out to two or more nodes → a new branch from it: there is no single path + * after it to put the node in; + * - an anchor with exactly one way on → split that edge (anchor → N → next), + * the new node takes the old one's place in the path; + * - an anchor with no way on → append (anchor → N); + * - an End anchor has no "after": the node goes BEFORE it, splitting its one + * way in; an End reached by several paths, or none, has no single path in + * — loose, as is a flow with no Start and nothing selected. + * + * "A way on" is a forward edge (never an ADR-0044 back-edge) between two + * distinct nodes that both exist; an edge naming a missing node is not a path + * (objectui#11772 leaves those for the Problems panel to name, unrepaired). + */ +function placeAfter(anchorId: string | null, nodes: FlowDesignerNode[], edges: FlowDesignerEdge[]): Placement { + const live = new Set(nodes.map((n) => n.id)); + const anchor = nodes.find((n) => n.id === anchorId) ?? nodes.find((n) => n.type === 'start'); + if (!anchor) return { kind: 'loose' }; + const isWayOn = (e: FlowDesignerEdge) => + !isBackEdge(e) && e.source !== e.target && live.has(e.source) && live.has(e.target); + if (anchor.type === 'end') { + const waysIn = edges.filter((e) => e.target === anchor.id && isWayOn(e)); + return waysIn.length === 1 ? { kind: 'split', edge: waysIn[0] } : { kind: 'loose' }; + } + const waysOn = edges.filter((e) => e.source === anchor.id && isWayOn(e)); + if (waysOn.length === 1 && !BRANCHING_NODE_TYPES.has(anchor.type)) return { kind: 'split', edge: waysOn[0] }; + return { kind: 'from', from: anchor.id }; +} + /** * One-line config summary shown on the node card (best-effort, type-aware). * The words it adds of its own (a branch count, an approver count, `code`) diff --git a/packages/app-shell/src/views/metadata-admin/previews/FlowPreview.removeNode-11772.test.tsx b/packages/app-shell/src/views/metadata-admin/previews/FlowPreview.removeNode-11772.test.tsx index f231ce7335..290b858e24 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/FlowPreview.removeNode-11772.test.tsx +++ b/packages/app-shell/src/views/metadata-admin/previews/FlowPreview.removeNode-11772.test.tsx @@ -26,11 +26,16 @@ import { render, screen, cleanup, fireEvent, act, within } from '@testing-librar // The engine palette / config-schema reads are stubbed to their offline answer // and the trigger field catalog to an empty one, so nothing here needs a -// network client (the inspectors' own tests stub the same two hooks). -vi.mock('./useFlowNodePalette', () => ({ - useActionConfigSchemas: () => ({}), - useFlowNodePalette: () => [], -})); +// network client (the inspectors' own tests stub the same two hooks). The +// palette's offline answer is the hardcoded `NODE_PALETTE`, not an empty list: +// since objectui#11778 every "+" opens it and the node lands on a pick. +vi.mock('./useFlowNodePalette', async () => { + const { NODE_PALETTE } = await import('./flow-canvas-parts'); + return { + useActionConfigSchemas: () => ({}), + useFlowNodePalette: () => NODE_PALETTE, + }; +}); vi.mock('./useObjectFields', () => ({ useObjectFields: () => ({ fields: [], loading: false, error: null }), })); @@ -127,8 +132,20 @@ function danglingEdges(d: Draft): Edge[] { return edgesOf(d).filter((e) => !ids.has(e.source) || !ids.has(e.target)); } +/** + * The open add-node palette's row for the node type the card's reproduction + * added (`create_record`, the type the "+" inserted before objectui#11778 made + * it ask). + */ +function pickCreateRecord() { + const option = screen.getAllByRole('option').find((o) => o.textContent?.startsWith('Create record')); + expect(option, 'the palette offers Create record').toBeDefined(); + fireEvent.click(option!); +} + function insertOnTheOnlyEdge() { fireEvent.click(screen.getByRole('button', { name: t('engine.flowCanvas.insertNode', 'en-US') })); + pickCreateRecord(); } function removeSelectedNode() { @@ -140,6 +157,7 @@ function addConnectedFrom(container: HTMLElement, nodeId: string) { const card = container.querySelector(`[data-node-id="${nodeId}"]`) as HTMLElement; expect(card, `node card ${nodeId}`).not.toBeNull(); fireEvent.click(within(card).getByRole('button', { name: t('engine.flowCanvas.addConnected', 'en-US') })); + pickCreateRecord(); } describe('flow designer: Remove node takes its edges, and a fresh node inherits none (objectui#11772)', () => { @@ -177,9 +195,12 @@ describe('flow designer: Remove node takes its edges, and a fresh node inherits const { probe, container } = mount(skeleton()); addConnectedFrom(container, 'start'); expect(nodesOf(probe.draft).map((n) => n.id)).toEqual(['start', 'end', 'node_1']); + // The same two ids as before the fix. Their WIRING is objectui#11778's: the + // node goes into Start's one path (`e1` now ends at it, `edge_1` carries on + // to End) where it used to open a second branch `start → node_1` beside it. expect(edgesOf(probe.draft)).toEqual([ - { id: 'e1', source: 'start', target: 'end' }, - { id: 'edge_1', source: 'start', target: 'node_1' }, + { id: 'e1', source: 'start', target: 'node_1' }, + { id: 'edge_1', source: 'node_1', target: 'end' }, ]); }); diff --git a/packages/app-shell/src/views/metadata-admin/previews/flow-canvas-parts.tsx b/packages/app-shell/src/views/metadata-admin/previews/flow-canvas-parts.tsx index 1c5882864f..f89018c528 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/flow-canvas-parts.tsx +++ b/packages/app-shell/src/views/metadata-admin/previews/flow-canvas-parts.tsx @@ -3,7 +3,9 @@ /** * flow-canvas-parts — presentational building blocks for `FlowCanvas.tsx`: * the per-node-type icon/tone mapping, the node card, and the add-node - * palette popover. Kept dependency-free and Shadcn-native (Tailwind + lucide). + * palette popover (the one picker every add-node entry point opens, and the + * list the inspector's Node Type select offers). Kept dependency-free and + * Shadcn-native (Tailwind + lucide). */ import * as React from 'react'; @@ -445,7 +447,17 @@ export interface NodeCardProps { badge?: { level: 'error' | 'warning'; title: string }; onPointerDown?: (e: React.PointerEvent) => void; onSelect?: () => void; - onAppend?: () => void; + /** + * objectui#11778 — the bottom "+" ("Add connected node") opens the add-node + * palette, and this receives the node type the author picked there. The + * canvas decides where the node goes (after this one, in its path). + */ + onAppend?: (type: string) => void; + /** The palette's node types, as the canvas's toolbar palette offers them. */ + paletteItems?: PaletteItem[]; + /** Whether this card's "+" palette is the one open on the canvas. */ + appendPaletteOpen?: boolean; + onAppendPaletteOpenChange?: (open: boolean) => void; /** * ADR-0044: when set (approval nodes without an existing revise loop), render * the one-click "add revision loop" affordance — drops a wait node + the @@ -486,7 +498,7 @@ export interface NodeCardProps { /** * A single draggable flow node rendered at an absolute canvas coordinate. * The card body drives selection + reposition; a dedicated bottom "+" handle - * (edit mode only) appends a connected child without ambiguity. + * (edit mode only) opens the add-node palette to add a node after this one. */ export function NodeCard({ id, @@ -504,6 +516,9 @@ export function NodeCard({ onPointerDown, onSelect, onAppend, + paletteItems, + appendPaletteOpen, + onAppendPaletteOpenChange, onAddReviseLoop, regions, expanded, @@ -642,22 +657,30 @@ export function NodeCard({ )} {editable && type !== 'end' && ( - + + )} {onAddReviseLoop && (