diff --git a/changelog.d/2431-wire-hit-targets.fixed.md b/changelog.d/2431-wire-hit-targets.fixed.md new file mode 100644 index 000000000..a623c5376 --- /dev/null +++ b/changelog.d/2431-wire-hit-targets.fixed.md @@ -0,0 +1 @@ +A workflow's read-only run flowchart now names each wire for a reader and gives every route a 10px hit target of its own: pointing at one wire marks it and dims the rest, and the plane still writes nothing ([#2431](https://github.com/ThinkFlowLab/FlyBot/issues/2431)). diff --git a/docs/subsystems/workflow.md b/docs/subsystems/workflow.md index 740b0537d..8c3d848bb 100644 --- a/docs/subsystems/workflow.md +++ b/docs/subsystems/workflow.md @@ -66,7 +66,7 @@ Everything in `plugins/workflow/`: the graph rules, the run store, the matcher, 9. The browser draws text only. Names and messages are React text children. The unit renderer asserts a hostile name is not markup. 10. `enabled: false` registers no endpoint, tool or listener and writes nothing. 11. A sidebar click opens the stage or a dialog and leaves the Workflows section in place. The stage and the dialogs portal onto `document.body`. When that container is missing they stay in the section tree; the click does not throw. -12. A node's prompt and the graph are one record. Settings → Workflow graphs (canvas-first with per-node inspector), the flowchart drawer (collapsed by default, inspector on select), and `define_workflow` all write `workflow/graph`. Read-only, the drawer's Flow tab lays the graph out from its steps instead (`flowLayout`, #1652): one row per layer, a parallel layer inside its dashed frame, node boxes sized to the measured panel width, and wires routed orthogonally by `flowWires` from the drawn, measured boxes — one bus per fork or join, skip and loop edges in the right gutter — so stored editor coordinates never misplace a node or leave a wire hanging. In Edit, the drawer paints that record fitted to its fixed 400px panel: `fitPlane` scales node centres (never the node boxes) into the panel width, wires and parallel frames use the fitted positions, and a pointer delta is divided by the fit scale before it is stored, so an edit still commits the same world coordinates. Because only the centres scale, a parallel layer's 168px cards overlap once it holds three or more branches in a 364px drawer, and the later card takes clicks meant for the branch's out-port; `editPlane` therefore keeps the fitted plane only while a layer's cards stay apart, and otherwise lays the graph out in `flowLayout`'s rows, column widths and wrap with the drawn coordinates as the panel's own, so there a drag is a plain pointer delta (#1677). A drag or a palette drop in that laid-out drawer plane changes structure, not layout: it commits no coordinates, and a stored `x`/`y` is only ever written from the Settings canvas, whose record is where layout lives (#1686). The plane says so in one line, `Layout lives in Settings` (zh: 布局在 Settings 画布改), when a drag there is refused. The Settings canvas paints the same record scaled to its own width (invariant 2), not as a second scroll container. A run reads that record; it does not define it. After a live run finishes, the stage shows the last finished live run (`lane.last` with `live === true`) until a new live run starts. The drawer's Flow tab and Settings' graph paint one node state: the last run's `nodeStatus`, and Idle when the workflow has no run at all (#1753). +12. A node's prompt and the graph are one record. Settings → Workflow graphs (canvas-first with per-node inspector), the flowchart drawer (collapsed by default, inspector on select), and `define_workflow` all write `workflow/graph`. Read-only, the drawer's Flow tab lays the graph out from its steps instead (`flowLayout`, #1652): one row per layer, a parallel layer inside its dashed frame, node boxes sized to the measured panel width, and wires routed orthogonally by `flowWires` from the drawn, measured boxes — one bus per fork or join, skip and loop edges in the right gutter — so stored editor coordinates never misplace a node or leave a wire hanging. That read-only plane's wire layer is a named `group` (`Connections`, zh 连线) rather than `aria-hidden`: every route is a `role="img"` with a ``, a 10px transparent hit path of its own (`pointer-events: stroke`), and a name built from both ends, so a fork or join bus and its drops read as the group they carry (`Coder → Reviewer + Tester`, `Reviewer + Tester → Deployer`); pointing at one wire marks it `data-active` and dims the rest `data-dim`, and the pointer writes nothing, so the plane stays read-only (#2431). In Edit, the drawer paints that record fitted to its fixed 400px panel: `fitPlane` scales node centres (never the node boxes) into the panel width, wires and parallel frames use the fitted positions, and a pointer delta is divided by the fit scale before it is stored, so an edit still commits the same world coordinates. Because only the centres scale, a parallel layer's 168px cards overlap once it holds three or more branches in a 364px drawer, and the later card takes clicks meant for the branch's out-port; `editPlane` therefore keeps the fitted plane only while a layer's cards stay apart, and otherwise lays the graph out in `flowLayout`'s rows, column widths and wrap with the drawn coordinates as the panel's own, so there a drag is a plain pointer delta (#1677). A drag or a palette drop in that laid-out drawer plane changes structure, not layout: it commits no coordinates, and a stored `x`/`y` is only ever written from the Settings canvas, whose record is where layout lives (#1686). The plane says so in one line, `Layout lives in Settings` (zh: 布局在 Settings 画布改), when a drag there is refused. The Settings canvas paints the same record scaled to its own width (invariant 2), not as a second scroll container. A run reads that record; it does not define it. After a live run finishes, the stage shows the last finished live run (`lane.last` with `live === true`) until a new live run starts. The drawer's Flow tab and Settings' graph paint one node state: the last run's `nodeStatus`, and Idle when the workflow has no run at all (#1753). 13. The stage chat follows a new line only while the reader is at the bottom. A line the stage filters out (a workflow node's own reply) changes no pixel, so it moves the chat nowhere; a reader who scrolled up keeps their place until they return to the bottom. ## One task identity (#1696) diff --git a/plugins/workflow/client.js b/plugins/workflow/client.js index 9b8a4f5f3..4052c9406 100644 --- a/plugins/workflow/client.js +++ b/plugins/workflow/client.js @@ -21,6 +21,7 @@ window.__ModuleLoader__.load({ const ZH = { Workflows: 'Workflows', + Connections: '连线', New: '新建', Project: '项目', Unassigned: '未归属', @@ -969,24 +970,28 @@ window.__ModuleLoader__.load({ const target = targets[0] const x = source.x if ((source.line ?? 0) >= fromLast && (target.line ?? 0) <= 0) { - wires.push({ key, kind: 'step', d: `M ${x} ${source.top + source.h} L ${x} ${target.top}`, arrow: true }) + wires.push({ key, kind: 'step', from: [source.id], to: [target.id], d: `M ${x} ${source.top + source.h} L ${x} ${target.top}`, arrow: true }) return } const marks = sourceMarks(source) const rise = targetMarks(target) if (marks.at(-1)[0] !== rise[0][0]) marks.push([rise[0][0], busY]) marks.push(...rise.slice(1)) - wires.push({ key, kind: 'step', d: pathOf(marks), arrow: true }) + wires.push({ key, kind: 'step', from: [source.id], to: [target.id], d: pathOf(marks), arrow: true }) return } const xs = sources.concat(targets).map(box => box.x) let minX = Math.min(...xs) const maxX = Math.max(...xs) if (sources.some(box => (box.line ?? 0) < fromLast)) minX = Math.min(minX, downX) + const targetIds = targets.map(box => box.id) const parts = sources.map(box => pathOf(sourceMarks(box))) if (maxX - minX >= 0.5) parts.push(`M ${minX} ${busY} L ${maxX} ${busY}`) - wires.push({ key: `${key}|bus`, kind: sources.length > 1 ? 'join' : 'fork', d: parts.join(' '), arrow: false }) - for (const box of targets) wires.push({ key: `${key}|${box.id}`, kind: 'drop', d: pathOf(targetMarks(box)), arrow: true }) + wires.push({ key: `${key}|bus`, kind: sources.length > 1 ? 'join' : 'fork', from: sources.map(box => box.id), to: targetIds, d: parts.join(' '), arrow: false }) + for (const box of targets) { + const feeders = group.edges.filter(edge => edge.to === box.id).map(edge => edge.from) + wires.push({ key: `${key}|${box.id}`, kind: 'drop', from: feeders.length ? feeders : sources.map(source => source.id), to: [box.id], d: pathOf(targetMarks(box)), arrow: true }) + } }) } skips.forEach(({ edge, from, to }, k) => { @@ -1012,6 +1017,8 @@ window.__ModuleLoader__.load({ wires.push({ key: `${edge.from}>${edge.to}|${edge.loop ? 'loop' : 'skip'}`, kind: edge.loop || to.row <= from.row ? 'loop' : 'skip', + from: [edge.from], + to: [edge.to], d: pathOf(marks), arrow: true, }) @@ -1331,16 +1338,42 @@ window.__ModuleLoader__.load({ function FlowView({ workflow, layout, status, roster }) { const nodes = workflow.nodes ?? [] + // P-395/#2422: the wire the pointer is on. It is pointer state only — a + // Read-only plane commits nothing — and it is what lets one stroke of two + // that share a port be pointed at while the rest recede. + const [pointedWire, setPointedWire] = useState(null) + const nameOf = id => nodes.find(node => node.id === id)?.name ?? id + // A join wire stands for every edge in its bus, so it is named by both ends. + const nameWire = wire => `${(wire.from ?? []).map(nameOf).join(' + ')} → ${(wire.to ?? []).map(nameOf).join(' + ')}` + // Each route is a named image with a 10px transparent hit path of its own: + // two wires that land on one port are one stroke to the eye, so pointing at + // one has to be a hit test on that route, not on the drawn line. + const wirePaths = layout.wires.map(wire => { + const active = pointedWire === wire.key + const dim = pointedWire !== null && !active + const marks = { 'data-key': wire.key, 'data-active': active ? '' : undefined, 'data-dim': dim ? '' : undefined } + const label = nameWire(wire) + return [ + h('path', { + key: wire.key, d: wire.d, 'data-wire': wire.kind, + className: wire.kind === 'loop' ? 'wf-wire-loop' : undefined, + markerEnd: wire.arrow ? 'url(#wf-flow-arrow)' : undefined, + ...marks, + }), + h('path', { + key: `${wire.key}:hit`, className: 'wf-wire-hit', d: wire.d, ...marks, + 'data-hit': '1', role: 'img', 'aria-label': label, + onPointerEnter: () => setPointedWire(wire.key), + onPointerLeave: () => setPointedWire(current => (current === wire.key ? null : current)), + }, h('title', null, label)), + ] + }) return h('div', { className: 'wf-plane wf-flow-view', 'data-plane': '1', style: { height: layout.height, width: layout.width } }, - h('svg', { className: 'wf-wires', 'aria-hidden': true, ...wireSvgProps(layout.height, layout.width) }, + h('svg', { className: 'wf-wires', role: 'group', 'aria-label': tr('Connections'), ...wireSvgProps(layout.height, layout.width) }, h('defs', null, h('marker', { id: 'wf-flow-arrow', viewBox: '0 0 10 10', refX: '9', refY: '5', markerWidth: '6', markerHeight: '6', orient: 'auto-start-reverse' }, h('path', { d: 'M0 1 9 5 0 9z', className: 'wf-arrow' }))), - layout.wires.map(wire => h('path', { - key: wire.key, d: wire.d, 'data-wire': wire.kind, - className: wire.kind === 'loop' ? 'wf-wire-loop' : undefined, - markerEnd: wire.arrow ? 'url(#wf-flow-arrow)' : undefined, - }))), + wirePaths), layout.frames.map(frame => h('div', { key: frame.key, className: 'wf-par wf-abs', style: { left: frame.left, top: frame.top, width: frame.width, height: frame.height }, @@ -2514,6 +2547,9 @@ window.__ModuleLoader__.load({ .wf-flow-view .wf-plabel{width:max-content;box-sizing:border-box;white-space:normal} .wf-flow-view .wf-wires path{stroke-linejoin:round} .wf-flow-view .wf-wires path.wf-wire-loop{stroke-dasharray:4 3} +.wf-flow-view .wf-wires .wf-wire-hit{pointer-events:stroke;stroke:transparent;stroke-width:10;fill:none} +.wf-flow-view .wf-wires path[data-active]{stroke:var(--bt-accent,#4d6bfe);stroke-width:2.4} +.wf-flow-view .wf-wires path[data-dim]{opacity:.16} .wf-flow-view .wf-arrow{fill:var(--bt-ink-3,#9a9a9a);stroke:none} .wf-palette{display:flex;flex-direction:column;align-items:stretch;gap:10px;width:min(100%,360px);margin:0 auto 4px;padding:10px 12px;border:1px dashed var(--bt-line,rgba(20,20,20,.14));border-radius:12px;background:var(--bt-card,#fff)} .wf-palette-hint{margin:0;font-size:11.5px;color:var(--bt-ink-2,rgba(20,20,20,.6));text-align:center;line-height:1.4} diff --git a/plugins/workflow/test/client.test.js b/plugins/workflow/test/client.test.js index 415ad6904..306a0be3f 100644 --- a/plugins/workflow/test/client.test.js +++ b/plugins/workflow/test/client.test.js @@ -317,6 +317,73 @@ async function openDrawer(graph, roster = { bots: [] }) { return { view, calls, nodeAt, canvas } } +// #2142 gave the Settings canvas's wires a hit target and a name; the drawer's own +// Read-only plane kept `aria-hidden` strokes with nothing to point at (#2422). +async function openReadOnlyFlow(graph, roster = { bots: [] }) { + const calls = [] + const call = async (name) => { + calls.push({ name }) + if (name === 'workflow/state') return { workflows: [graph], templates: [] } + return { ok: true, workflows: [graph] } + } + const view = mount(client.Section, { roster, call, actions: {} }) + await settle() + findAll(view.tree, node => node.type === 'button' && textOf(node).includes(graph.name))[0].props.onClick() + button(view.tree, 'Flow').props.onClick() + return { view, calls } +} +const wireLayer = view => findAll(view.tree, node => String(node.props?.className ?? '').split(' ').includes('wf-wires'))[0] +const wireHits = view => findAll(wireLayer(view), node => node.type === 'path' && node.props?.['data-hit'] === '1') +const wireStrokes = view => findAll(wireLayer(view), node => node.type === 'path' && node.props?.['data-wire'] !== undefined) +const wireNamed = (view, name) => wireHits(view).find(node => accessibleName(node) === name) + +test('the read-only run flowchart names every wire and gives each one a hit target of its own (#2422)', async () => { + const { view } = await openReadOnlyFlow(delivery()) + const layer = wireLayer(view) + assert.ok(layer, 'the read-only run flowchart draws no wire layer') + assert.notEqual(layer.props?.['aria-hidden'], true, 'the wire layer is hidden from a reader') + assert.equal(layer.props?.role, 'group', 'the wire layer is not a named group') + assert.equal(layer.props?.['aria-label'], 'Connections', 'the wire layer carries no name') + const hits = wireHits(view) + const strokes = wireStrokes(view) + assert.ok(strokes.length >= 6, `only ${strokes.length} wires are drawn`) + assert.equal(hits.length, strokes.length, 'a drawn wire carries no hit path of its own') + for (const hit of hits) { + const name = accessibleName(hit) + assert.match(name, /→/, `a wire is named ${JSON.stringify(name)}`) + assert.equal(hit.props?.role, 'img', 'a hit path is not an image a reader can reach') + assert.ok(findAll(hit, node => node.type === 'title').length > 0, `${name} carries no <title>`) + } + const names = hits.map(accessibleName) + assert.ok(names.includes('Reviewer + Tester → Deployer'), `${names.join(', ')} never names the wire into Deployer`) + assert.ok(names.includes('Coder → Reviewer + Tester'), `${names.join(', ')} never names the wire out of Coder`) + view.unmount() +}) + +test('pointing at one wire in the read-only flowchart picks it out and lets the others recede (#2422)', async () => { + const { view } = await openReadOnlyFlow(delivery()) + wireNamed(view, 'Reviewer + Tester → Deployer').props.onPointerEnter() + const active = wireHits(view).filter(node => node.props?.['data-active'] === '') + assert.equal(active.length, 1, `${active.length} wires answer the pointer`) + assert.equal(accessibleName(active[0]), 'Reviewer + Tester → Deployer') + assert.equal(wireHits(view).filter(node => node.props?.['data-dim'] === '').length, wireHits(view).length - 1, 'the other wires did not recede') + const painted = wireStrokes(view).filter(node => node.props?.['data-active'] === '') + assert.equal(painted.length, 1, 'the pointed wire is not painted as the pointed one') + wireNamed(view, 'Reviewer + Tester → Deployer').props.onPointerLeave() + assert.equal(wireHits(view).filter(node => node.props?.['data-active'] === '' || node.props?.['data-dim'] === '').length, 0, 'the pointer left the layer pointed') + view.unmount() +}) + +test('a read-only run flowchart commits nothing when a wire is pointed at (#2422)', async () => { + const { view, calls } = await openReadOnlyFlow(delivery()) + const hit = wireHits(view)[0] + hit.props.onPointerEnter() + wireHits(view)[0].props.onClick?.({ stopPropagation() {} }) + wireHits(view)[0].props.onPointerLeave() + assert.deepEqual(calls.filter(item => item.name !== 'workflow/state'), [], 'pointing at a wire wrote to the workflow') + view.unmount() +}) + test('dragging the rightmost node moves it on screen inside the frame the drag started with (#1575)', async () => { const nodes = [ { id: 'left', name: 'Left', role: '', x: 0, y: 100 },