Skip to content

Commit e56b288

Browse files
authored
fix(workflow): draw a highlighted edge over the ordinary ones (#6642)
* fix(workflow): draw a highlighted edge over the ordinary ones An edge's z came from the nesting depth of the container it belongs to, and a highlighted edge kept that depth like any other. A line one level deeper therefore sat above it and painted straight through the highlight, cutting it in half wherever the two crossed. Give a highlighted edge — selected, or connected to the selected card — the top tier of the edge band instead. Depth only ever ordered edges against each other, and once the user has picked one out, being drawn whole matters more than which container it came from. The tier stays inside the band, below the cards, deliberately: highlighted edges were elevated over the cards once before and drew across the chrome of their own endpoints. A line belongs behind cards, knobs and the action-bar swell whether or not it is highlighted, so ordinary edges give up the top of the band rather than the band being widened into the cards. * fix(workflow): elevate the connection preview edge with the rest It renders highlighted — its data carries `isConnectedToSelection` — but it was the one call site left taking a depth tier, so the line being drawn could be crossed by an ordinary edge in a deeper container. Highlighted now means elevated with no exception. Also drop the export on the highlighted tier: nothing outside the module reads it, and the band's tiers are an implementation detail of `getEdgeZIndex`. * fix(workflow): give the edge highlight one definition The z-index elevation I added checked canvas selection only, while the edge darkens for panel focus too — a block open in the editor lights its edges, and those stayed depth-tiered, so an ordinary edge could still cut through the highlight. The bug I set out to fix, on the path I had not covered. The condition already existed in two places and the second one carries a comment saying it must mirror the first exactly, because a knob checking fewer conditions than the line leaves a dark line running into a light knob. Adding the z would have made a third copy, and the finding here is what the third copy gets you. One predicate now, in `edge-highlight`, used by the line, the knobs, and the z. The canvas subscribes to the panel store rather than reading `getState()`, since the z has to be recomputed when the open block changes.
1 parent c58a642 commit e56b288

6 files changed

Lines changed: 216 additions & 26 deletions

File tree

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ import {
8484
useIsBlockInActiveExecutionHandoff,
8585
} from '@/app/workspace/[workspaceId]/w/[workflowId]/hooks'
8686
import { useBlockDimensions } from '@/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-block-dimensions'
87+
import {
88+
isEdgeConnectedToEditor,
89+
isEdgeHighlighted,
90+
} from '@/app/workspace/[workspaceId]/w/[workflowId]/utils/edge-highlight'
8791
import { hasBlockAccent } from '@/blocks/accent'
8892
import { useCustomBlockOverlayVersion } from '@/blocks/custom/client-overlay'
8993
import { getBlock } from '@/blocks/registry'
@@ -716,19 +720,21 @@ export const WorkflowBlock = memo(function WorkflowBlock({
716720
const keys: string[] = []
717721
for (const edge of state.edges) {
718722
if (edge.source !== id && edge.target !== id) continue
719-
/*
720-
* Must mirror workflow-edge's shouldHighlightEdge exactly: the edge
721-
* darkens when an endpoint is canvas-selected OR open in the editor
722-
* panel. If the knob checks fewer conditions than the line, a dark
723-
* line runs into a light knob.
724-
*/
725-
const isHighlighted =
726-
state.nodeInternals.get(edge.source)?.selected ||
727-
state.nodeInternals.get(edge.target)?.selected ||
728-
(edge.data as { isConnectedToSelection?: boolean } | undefined)
729-
?.isConnectedToSelection ||
730-
(editorOpenBlockId !== null &&
731-
(edge.source === editorOpenBlockId || edge.target === editorOpenBlockId))
723+
/* Same predicate the line itself uses — a knob checking fewer
724+
conditions than the edge leaves a dark line running into a light
725+
knob. */
726+
const isHighlighted = isEdgeHighlighted({
727+
isEndpointSelected:
728+
state.nodeInternals.get(edge.source)?.selected ||
729+
state.nodeInternals.get(edge.target)?.selected ||
730+
(edge.data as { isConnectedToSelection?: boolean } | undefined)
731+
?.isConnectedToSelection,
732+
isConnectedToEditor: isEdgeConnectedToEditor(
733+
editorOpenBlockId,
734+
edge.source,
735+
edge.target
736+
),
737+
})
732738
if (!isHighlighted) continue
733739
if (edge.source === id) keys.push(edge.sourceHandle || 'source')
734740
if (edge.target === id) keys.push(edge.targetHandle || 'target')

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-edge/workflow-edge.tsx

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ import { memo, useCallback, useMemo } from 'react'
22
import { type EdgeDiffStatus, WorkflowEdgeView } from '@sim/workflow-renderer'
33
import { type EdgeProps, useStore } from 'reactflow'
44
import { useShallow } from 'zustand/react/shallow'
5+
import {
6+
isEdgeConnectedToEditor,
7+
isEdgeHighlighted,
8+
} from '@/app/workspace/[workspaceId]/w/[workflowId]/utils/edge-highlight'
59
import {
610
useIsBlockActive,
711
useIsCurrentWorkflowExecuting,
@@ -55,11 +59,15 @@ const WorkflowEdgeComponent = (props: WorkflowEdgeProps) => {
5559
isEndpointSelected ||
5660
(data as { isConnectedToSelection?: boolean } | undefined)?.isConnectedToSelection
5761
)
58-
const isConnectedToEditor =
59-
activeTab === 'editor' &&
60-
currentBlockId !== null &&
61-
(currentBlockId === source || currentBlockId === target)
62-
const shouldHighlightEdge = isConnectedToSelection || isConnectedToEditor
62+
const isConnectedToEditor = isEdgeConnectedToEditor(
63+
activeTab === 'editor' ? currentBlockId : null,
64+
source,
65+
target
66+
)
67+
const shouldHighlightEdge = isEdgeHighlighted({
68+
isEndpointSelected: isConnectedToSelection,
69+
isConnectedToEditor,
70+
})
6371

6472
const previewExecutionStatus = (
6573
data as { executionStatus?: 'success' | 'error' | 'not-executed' } | undefined
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/**
2+
* Whether an edge is drawn highlighted.
3+
*
4+
* Three places need this answer and each reaches it from a different source —
5+
* the edge itself from the React Flow store, a card from the same store while
6+
* deciding which of its knobs to darken, and the canvas while assigning the
7+
* edge's z so a highlighted line is not crossed by an ordinary one. They have
8+
* to agree: a knob checking fewer conditions than the line leaves a dark line
9+
* running into a light knob, and a z checking fewer leaves the highlight cut in
10+
* half by whatever crosses it.
11+
*
12+
* They agreed by being copied, which is the arrangement that produced both of
13+
* those bugs. This is the one definition.
14+
*/
15+
export function isEdgeHighlighted(state: {
16+
/** Either endpoint is selected on the canvas. */
17+
isEndpointSelected?: boolean
18+
/** Either endpoint is the block open in the editor panel. */
19+
isConnectedToEditor?: boolean
20+
/** The edge itself is selected. */
21+
isEdgeSelected?: boolean
22+
}): boolean {
23+
return Boolean(state.isEndpointSelected || state.isConnectedToEditor || state.isEdgeSelected)
24+
}
25+
26+
/**
27+
* Whether an edge touches the block currently open in the editor panel.
28+
*
29+
* `null` while the panel is on another tab, so a block left open behind the
30+
* console does not keep its edges lit.
31+
*/
32+
export function isEdgeConnectedToEditor(
33+
editorOpenBlockId: string | null,
34+
source: string,
35+
target: string
36+
): boolean {
37+
return (
38+
editorOpenBlockId !== null && (source === editorOpenBlockId || target === editorOpenBlockId)
39+
)
40+
}

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/workflow.tsx

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,10 @@ import {
9393
shouldHighlightContainerDropTarget,
9494
validateTriggerPaste,
9595
} from '@/app/workspace/[workspaceId]/w/[workflowId]/utils'
96+
import {
97+
isEdgeConnectedToEditor,
98+
isEdgeHighlighted,
99+
} from '@/app/workspace/[workspaceId]/w/[workflowId]/utils/edge-highlight'
96100
import {
97101
defaultEdgeOptions,
98102
edgeTypes,
@@ -127,7 +131,7 @@ import {
127131
} from '@/stores/execution'
128132
import { useSearchModalStore } from '@/stores/modals/search/store'
129133
import type { PendingConnect } from '@/stores/modals/search/types'
130-
import { usePanelEditorStore } from '@/stores/panel'
134+
import { usePanelEditorStore, usePanelStore } from '@/stores/panel'
131135
import { useUndoRedoStore } from '@/stores/undo-redo'
132136
import { useVariablesModalStore } from '@/stores/variables/modal'
133137
import { useWorkflowDiffStore } from '@/stores/workflow-diff/store'
@@ -4461,6 +4465,11 @@ const WorkflowContent = React.memo(
44614465
}, [closeConnectionBlockSelector, displayNodes, lastInteractedNodeId, pendingConnect])
44624466

44634467
/** Transforms edges to include selection state and delete handlers. Memoized to prevent re-renders. */
4468+
/* Subscribed rather than read from `getState()`: the edge z below depends on
4469+
which block is open, so the memo has to re-run when that changes. */
4470+
const editorOpenBlockId = usePanelEditorStore((state) => state.currentBlockId)
4471+
const panelActiveTab = usePanelStore((state) => state.activeTab)
4472+
44644473
const edgesWithSelection = useMemo(() => {
44654474
const nodeMap = new Map(displayNodes.map((n) => [n.id, n]))
44664475
/* Indexed once: this memo re-runs on every drag frame, and scanning the
@@ -4478,21 +4487,38 @@ const WorkflowContent = React.memo(
44784487
// pointer events, so the edge has to be above it to stay clickable) and
44794488
// still below that container's own children.
44804489
//
4490+
// A highlighted edge takes the top of that band instead, so no ordinary
4491+
// edge can cross over the one the user has picked out. Depth only ever
4492+
// ordered lines against each other, and an unselected edge one level
4493+
// deeper was painting straight through the highlight.
4494+
//
44814495
// Edges are NEVER elevated above cards — not even when an endpoint is
44824496
// selected. A line always passes behind cards, knobs, and the action
44834497
// bar swell; elevating highlighted edges drew them across their own
4484-
// endpoint's chrome.
4498+
// endpoint's chrome. The highlighted tier stays inside the band for
4499+
// exactly that reason.
44854500
const containerNode = parentLoopId ? nodeMap.get(parentLoopId) : null
4486-
const baseZIndex = getEdgeZIndex(containerNode ? (containerNode.zIndex ?? 0) : undefined)
44874501
const isConnectedToSelection =
44884502
selectedNodeIdSet.has(edge.source) || selectedNodeIdSet.has(edge.target)
4503+
const isSelected = selectedEdges.has(edgeContextId)
4504+
const baseZIndex = getEdgeZIndex(containerNode ? (containerNode.zIndex ?? 0) : undefined, {
4505+
isHighlighted: isEdgeHighlighted({
4506+
isEndpointSelected: isConnectedToSelection,
4507+
isConnectedToEditor: isEdgeConnectedToEditor(
4508+
panelActiveTab === 'editor' ? editorOpenBlockId : null,
4509+
edge.source,
4510+
edge.target
4511+
),
4512+
isEdgeSelected: isSelected,
4513+
}),
4514+
})
44894515

44904516
return {
44914517
...edge,
44924518
zIndex: baseZIndex,
44934519
data: {
44944520
...edge.data,
4495-
isSelected: selectedEdges.has(edgeContextId),
4521+
isSelected,
44964522
isConnectedToSelection,
44974523
isInsideLoop: Boolean(parentLoopId),
44984524
parentLoopId,
@@ -4501,7 +4527,15 @@ const WorkflowContent = React.memo(
45014527
},
45024528
}
45034529
})
4504-
}, [edgesForDisplay, displayNodes, selectedNodeIds, selectedEdges, handleEdgeDelete])
4530+
}, [
4531+
edgesForDisplay,
4532+
displayNodes,
4533+
selectedNodeIds,
4534+
selectedEdges,
4535+
handleEdgeDelete,
4536+
editorOpenBlockId,
4537+
panelActiveTab,
4538+
])
45054539

45064540
const edgesForRender = useMemo(() => {
45074541
if (!pendingConnect) return edgesWithSelection
@@ -4520,7 +4554,12 @@ const WorkflowContent = React.memo(
45204554
target: CONNECTION_BLOCK_SELECTOR_NODE_ID,
45214555
targetHandle: 'target',
45224556
type: 'workflowEdge',
4523-
zIndex: getEdgeZIndex(sourceParentNode ? (sourceParentNode.zIndex ?? 0) : undefined),
4557+
/* Rendered highlighted (`isConnectedToSelection` below), so it is
4558+
elevated like any other highlighted edge — the preview line is the
4559+
one the user is currently drawing. */
4560+
zIndex: getEdgeZIndex(sourceParentNode ? (sourceParentNode.zIndex ?? 0) : undefined, {
4561+
isHighlighted: true,
4562+
}),
45244563
focusable: false,
45254564
deletable: false,
45264565
reconnectable: false,
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import {
6+
BLOCK_Z_BASE,
7+
CONTAINER_CHILD_Z_BASE,
8+
EDGE_Z_BASE,
9+
EDGE_Z_MAX,
10+
getBlockZIndex,
11+
getEdgeZIndex,
12+
} from './canvas-layers'
13+
14+
/**
15+
* Nesting depths an edge is tiered by. Stops short of the band's ceiling: past
16+
* it every edge saturates at the same tier, which is checked on its own below.
17+
*/
18+
const DEPTHS = [undefined, 0, 1, 2, 5]
19+
20+
describe('getEdgeZIndex', () => {
21+
it('puts a highlighted edge over every ordinary one, however deeply nested', () => {
22+
/* The reported bug: a highlighted edge kept its own container's depth, so an
23+
ordinary edge one level deeper painted over it and cut the highlight. */
24+
const highlighted = getEdgeZIndex(undefined, { isHighlighted: true })
25+
26+
for (const depth of DEPTHS) {
27+
expect(getEdgeZIndex(depth)).toBeLessThan(highlighted)
28+
}
29+
})
30+
31+
it('keeps a highlighted edge below the cards', () => {
32+
/* Elevating highlighted edges over the cards drew them across the chrome of
33+
their own endpoints, so the highlighted tier stays inside the edge band. */
34+
const highlighted = getEdgeZIndex(undefined, { isHighlighted: true })
35+
36+
expect(highlighted).toBeLessThan(BLOCK_Z_BASE)
37+
expect(highlighted).toBeLessThan(getBlockZIndex(BLOCK_Z_BASE))
38+
expect(highlighted).toBeLessThan(CONTAINER_CHILD_Z_BASE)
39+
})
40+
41+
it('leaves the in-flight connection line above everything in the band', () => {
42+
expect(getEdgeZIndex(undefined, { isHighlighted: true })).toBeLessThan(EDGE_Z_MAX)
43+
for (const depth of DEPTHS) {
44+
expect(getEdgeZIndex(depth)).toBeLessThan(EDGE_Z_MAX)
45+
}
46+
})
47+
48+
it('still orders ordinary edges by the depth they are nested at', () => {
49+
expect(getEdgeZIndex(undefined)).toBe(EDGE_Z_BASE)
50+
expect(getEdgeZIndex(0)).toBeGreaterThan(getEdgeZIndex(undefined))
51+
expect(getEdgeZIndex(1)).toBeGreaterThan(getEdgeZIndex(0))
52+
})
53+
54+
it('keeps every edge clear of the container bodies it crosses', () => {
55+
/* Containers are numbered from 0 by nesting depth; an edge sharing a body's
56+
z loses the equal-z tiebreak to DOM order and is drawn behind it. */
57+
for (const depth of DEPTHS) {
58+
expect(getEdgeZIndex(depth)).toBeGreaterThan(depth ?? 0)
59+
expect(getEdgeZIndex(depth, { isHighlighted: true })).toBeGreaterThan(depth ?? 0)
60+
}
61+
})
62+
63+
it('saturates rather than growing past the band', () => {
64+
/* The band is fixed, so beyond its ceiling every edge shares the deepest
65+
tier and no longer clears a container nested that far — true before this
66+
change too, at a ceiling of `EDGE_Z_MAX` rather than one below the
67+
highlighted tier. Nothing in the editor nests anywhere near it. */
68+
expect(getEdgeZIndex(40)).toBe(getEdgeZIndex(8))
69+
expect(getEdgeZIndex(8)).toBeLessThan(getEdgeZIndex(undefined, { isHighlighted: true }))
70+
})
71+
})

packages/workflow-renderer/src/canvas-layers.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,23 @@
2121
* once already and took the preview's edges behind its containers with it.
2222
*/
2323
export const EDGE_Z_BASE = 10
24+
/**
25+
* Deepest nesting tier an ordinary edge reaches, leaving the top of the band to
26+
* the two edges that have to be seen whole.
27+
*/
28+
const EDGE_Z_DEPTH_MAX = 18
29+
/**
30+
* A highlighted edge — selected, or connected to the selected card. Above every
31+
* ordinary edge whatever it is nested in, because the highlight is what the
32+
* user is looking at and a line crossing it from a deeper container was cutting
33+
* it in half.
34+
*
35+
* Still inside the edge band, deliberately. Highlighted edges used to be
36+
* elevated over the cards as well, which drew them across the chrome of their
37+
* own endpoints; a line belongs behind cards, knobs and the action-bar swell
38+
* whether or not it is highlighted.
39+
*/
40+
const EDGE_Z_HIGHLIGHTED = 19
2441
export const EDGE_Z_MAX = 20
2542
export const BLOCK_Z_BASE = 21
2643
export const CONTAINER_CHILD_Z_BASE = 1000
@@ -41,10 +58,19 @@ export function getBlockZIndex(
4158
* it belongs to, so an edge always clears the container body it crosses while
4259
* staying under that container's own children.
4360
*
61+
* A highlighted edge leaves that ordering and takes {@link EDGE_Z_HIGHLIGHTED}
62+
* instead. Depth is only a tiebreak between lines nobody is looking at; once one
63+
* is highlighted, being drawn whole matters more than which container it came
64+
* from — an unselected edge one level deeper used to paint straight over it.
65+
*
4466
* `containerZIndex` is the parent container's own z (its nesting depth), or
4567
* undefined for an edge at the top level.
4668
*/
47-
export function getEdgeZIndex(containerZIndex: number | undefined): number {
69+
export function getEdgeZIndex(
70+
containerZIndex: number | undefined,
71+
state: { isHighlighted?: boolean } = {}
72+
): number {
73+
if (state.isHighlighted) return EDGE_Z_HIGHLIGHTED
4874
const depth = containerZIndex === undefined ? 0 : containerZIndex + 1
49-
return Math.min(EDGE_Z_BASE + depth, EDGE_Z_MAX)
75+
return Math.min(EDGE_Z_BASE + depth, EDGE_Z_DEPTH_MAX)
5076
}

0 commit comments

Comments
 (0)