Skip to content

sync(ladder): reimplement handle branches on ladder #750

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dcoutinho1328 wants to merge 7 commits into from
Closed

sync(ladder): reimplement handle branches on ladder #750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bd86224
sync(ladder): pull handle-branch reimplementation from web
dcoutinho1328 May 8, 2026
7619bb3
fix(ports): re-export HandleBranch / RungLadderState from ports barrel
dcoutinho1328 May 8, 2026
986b4a0
sync(ladder): pull latest handle-branch changes from web
dcoutinho1328 May 11, 2026
e1244ec
sync(ladder): pull parallel-bracket-collapse fix from web
dcoutinho1328 May 12, 2026
f290310
sync(ladder): pull conditional bracket-collapse from web
dcoutinho1328 May 12, 2026
fa04fa9
sync(ladder): pull wider nested-parallel clearance from web
dcoutinho1328 May 12, 2026
f306987
sync(ladder): pull staircase+align approach from web
dcoutinho1328 May 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/__architecture__/validate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,8 @@ const KNOWN_EXCEPTIONS: Record<string, LayerName[]> = {
'frontend/store/slices/ladder/utils/index.ts': ['components'],
// Ladder slice — needs nodesBuilder + defaultCustomNodesStyles for rung creation
'frontend/store/slices/ladder/slice.ts': ['components'],
// Ladder slice types — re-exports RungLadderState narrowed to RungNode[]
'frontend/store/slices/ladder/types.ts': ['components'],
}

// ---------------------------------------------------------------------------
Expand Down
6 changes: 3 additions & 3 deletions src/frontend/components/_atoms/graphical-editor/fbd/block.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ export const Block = <T extends object>(block: BlockProps<T>) => {
const libPou = pous.find((pou) => pou.name === libMatch.name)
if (!libPou) return

const blockVariant = node.data.variant as BlockVariant
const blockVariant = (node.data as BlockNodeData<BlockVariant>).variant
const newNodeVariables = (libPou.interface?.variables ?? []).map((variable) => {
let newType
switch (variable.type.definition) {
Expand Down Expand Up @@ -632,10 +632,10 @@ export const Block = <T extends object>(block: BlockProps<T>) => {

const newNode = { ...updatedNewNode }

const originalNodeInputs = (node.data.variant as BlockVariant).variables.filter(
const originalNodeInputs = (node.data as BlockNodeData<BlockVariant>).variant.variables.filter(
(variable) => variable.class === 'input' || variable.class === 'inOut',
)
const originalNodeSources = (node.data.variant as BlockVariant).variables.filter(
const originalNodeSources = (node.data as BlockNodeData<BlockVariant>).variant.variables.filter(
(variable) => variable.class === 'output' || variable.class === 'inOut',
)

Expand Down
2 changes: 1 addition & 1 deletion src/frontend/components/_atoms/graphical-editor/fbd/utils/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const getFBDPouVariablesRungNodeAndEdges = (
source: LadderFlowType['rungs'][0]['edges'] | undefined
target: LadderFlowType['rungs'][0]['edges'] | undefined
Comment on lines 25 to 26
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check TypeScript compilation and examine edge type definitions

# Check for TypeScript errors in this file
echo "=== Checking for TypeScript errors ==="
npx tsc --noEmit --project tsconfig.json 2>&1 | rg -C3 "fbd/utils/utils.ts"

# Find and display FBD and Ladder edge type definitions
echo -e "\n=== FBD edge type structure ==="
rg -n -A5 "type FBDFlowType.*=|interface FBDFlowType" --type=ts

echo -e "\n=== Ladder edge type structure ==="
rg -n -A5 "type LadderFlowType.*=|interface LadderFlowType" --type=ts

# Check usages of this function to see what type consumers expect
echo -e "\n=== Function usages ==="
rg -n -B2 -A2 "getFBDPouVariablesRungNodeAndEdges" --type=ts --type=tsx

Repository: Autonomy-Logic/openplc-editor

Length of output: 941

🏁 Script executed:

# Read the utils.ts file to see the full function definition
cat -n src/frontend/components/_atoms/graphical-editor/fbd/utils/utils.ts | head -100

Repository: Autonomy-Logic/openplc-editor

Length of output: 4807

🏁 Script executed:

# Get the FBDRungState type definition
rg -n -A15 "type FBDRungState|interface FBDRungState" src/frontend/store/slices/fbd/types.ts

Repository: Autonomy-Logic/openplc-editor

Length of output: 55

🏁 Script executed:

# Get the RungLadderState type definition for comparison
rg -n -A15 "type RungLadderState|interface RungLadderState" src/frontend/store/slices/ladder/types.ts

Repository: Autonomy-Logic/openplc-editor

Length of output: 408

🏁 Script executed:

# Search for edge-related type definitions in both FBD and Ladder
rg -n "edges.*:" src/frontend/store/slices/fbd/types.ts src/frontend/store/slices/ladder/types.ts

Repository: Autonomy-Logic/openplc-editor

Length of output: 543

🏁 Script executed:

# Get FBDRungState type definition
rg -n -A20 "type FBDRungState|interface FBDRungState" src/frontend/store/slices/fbd/types.ts

Repository: Autonomy-Logic/openplc-editor

Length of output: 55

🏁 Script executed:

# Also check PortRungLadderState to see the full edge definition
rg -n "edges" src/frontend/store/slices/ladder/types.ts | head -20

Repository: Autonomy-Logic/openplc-editor

Length of output: 456

🏁 Script executed:

# Read the entire FBD types file to find FBDRungState
cat -n src/frontend/store/slices/fbd/types.ts | head -150

Repository: Autonomy-Logic/openplc-editor

Length of output: 3957

🏁 Script executed:

# Also search for edge definitions in FBD context more broadly
rg -n "edges" src/frontend/store/slices/fbd/types.ts

Repository: Autonomy-Logic/openplc-editor

Length of output: 248

🏁 Script executed:

# Find and read the FBDRungState definition
rg -n -A20 "type FBDRungState|interface FBDRungState" src/middleware/shared/ports/types.ts

Repository: Autonomy-Logic/openplc-editor

Length of output: 759

🏁 Script executed:

# Also check for RungLadderState in the same location for comparison
rg -n -A20 "type RungLadderState|interface RungLadderState" src/middleware/shared/ports/types.ts

Repository: Autonomy-Logic/openplc-editor

Length of output: 936

🏁 Script executed:

# Verify the correct type path for FBD edges
rg -n "FBDFlowType\['rung'\]\['edges'\]" src/

Repository: Autonomy-Logic/openplc-editor

Length of output: 55

🏁 Script executed:

# Double-check that the return values are actually Edge[] and assignment works
rg -n -B5 -A5 "edgesThatNodeIsSource\|edgesThatNodeIsTarget" src/frontend/components/_atoms/graphical-editor/fbd/utils/utils.ts

Repository: Autonomy-Logic/openplc-editor

Length of output: 55

Correct the edge type references to use FBDFlowType instead of LadderFlowType.

Lines 25–26 should reference FBDFlowType['rung']['edges'] instead of LadderFlowType['rungs'][0]['edges']. The function is FBD-specific, the rung parameter is already typed as FBDFlowType['rung'], and the implementation retrieves edges from rung?.edges. This inconsistency mirrors the fix already applied to line 28 for the node type. While both type references resolve to Edge[] structurally, using FBDFlowType maintains consistency with the function's actual context.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/components/_atoms/graphical-editor/fbd/utils/utils.ts` around
lines 25 - 26, Replace the incorrect LadderFlowType edge references with the
FBD-specific type: change the types of the source and target parameters that
currently use LadderFlowType['rungs'][0]['edges'] to
FBDFlowType['rung']['edges'] so they match the existing rung parameter (typed as
FBDFlowType['rung']) and the implementation that reads rung?.edges; update the
parameter type annotations where source and target are declared accordingly.

}
node: LadderFlowType['rungs'][0]['nodes'][0] | undefined
node: FBDFlowType['rung']['nodes'][0] | undefined
} => {
const pou = pous.find((pou) => pou.name === editor.meta.name)
const rung = fbdFlows.find((flow) => flow.name === editor.meta.name)?.rung
Expand Down
53 changes: 43 additions & 10 deletions src/frontend/components/_atoms/graphical-editor/ladder/autocomplete/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,14 @@ import { toast } from '../../../../_features/[app]/toast/use-toast'
import { GraphicalEditorAutocomplete } from '../../autocomplete'
import { getVariableRestrictionType } from '../../utils'
import { getLadderPouVariablesRungNodeAndEdges } from '../utils'
import { BasicNodeData, BlockNodeData, BlockVariant, LadderBlockConnectedVariables, VariableNode } from '../utils/types'
import {
BasicNodeData,
BlockNodeData,
BlockVariant,
isRungNodeOfType,
LadderBlockConnectedVariables,
VariableNode,
} from '../utils/types'

type VariablesBlockAutoCompleteProps = ComponentPropsWithRef<'div'> & {
block: unknown
Expand Down Expand Up @@ -121,27 +128,29 @@ const VariablesBlockAutoComplete = forwardRef<HTMLDivElement, VariablesBlockAuto
},
})

// Check if the variable is connected to a block
if ((variableNode as VariableNode).data.block === undefined) return
// Variable node's `block` link is the back-reference to the FB-input
// contact-style usage; if absent, this variable isn't attached to a block
// handle, so there's no block connectedVariables list to update.
if (!isRungNodeOfType(variableNode, 'variable')) return
const variableData = variableNode.data
if (variableData.block === undefined) return

// Get the block that is connected to the variable
const relatedBlock = rung.nodes.find((node) => node.id === (variableNode as VariableNode).data.block.id)
const relatedBlock = rung.nodes.find((node) => node.id === variableData.block.id)
if (!relatedBlock) return

const existingConnected = Array.isArray((relatedBlock.data as BlockNodeData<BlockVariant>).connectedVariables)
? (relatedBlock.data as BlockNodeData<BlockVariant>).connectedVariables
: []
const connectedVariables: LadderBlockConnectedVariables = [
...existingConnected.filter(
(v) =>
v.type !== variableNode.data.variant || v.handleId !== (variableNode as VariableNode).data.block.handleId,
(v) => v.type !== variableData.variant || v.handleId !== variableData.block.handleId,
),
{
handleId: (variableNode as VariableNode).data.block.handleId,
handleId: variableData.block.handleId,
handleTableId: (relatedBlock.data as BlockNodeData<BlockVariant>).variant.variables.find(
(v) => v.name === (variableNode as VariableNode).data.block.handleId,
(v) => v.name === variableData.block.handleId,
)?.id,
type: (variableNode as VariableNode).data.variant,
type: variableData.variant,
variable: variable,
},
]
Expand All @@ -163,6 +172,30 @@ const VariablesBlockAutoComplete = forwardRef<HTMLDivElement, VariablesBlockAuto

const submitAddVariable = ({ variableName }: { variableName: string }) => {
if (!variableName.trim()) {
// For variable nodes on block handles, an empty submission clears the
// variable instead of erroring — letting the user pick a different
// variable (or place a branch on the handle once that's supported).
if (blockType === 'variable') {
const { rung, node: variableNode } = getLadderPouVariablesRungNodeAndEdges(editor, pous, ladderFlows, {
nodeId: (block as Node<BasicNodeData>).id,
})
if (rung && variableNode) {
updateNode({
editorName: editor.meta.name,
rungId: rung.id,
nodeId: variableNode.id,
node: {
...variableNode,
data: {
...variableNode.data,
variable: { id: '', name: '' },
},
},
})
}
return
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

toast({
title: 'Invalid variable name',
description: 'Variable name cannot be empty',
Expand Down
68 changes: 50 additions & 18 deletions src/frontend/components/_atoms/graphical-editor/ladder/block.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ import { checkVariableName } from '../../../../store/slices/project/validation/v
import { cn } from '../../../../utils/cn'
import { toast } from '../../../_features/[app]/toast/use-toast'
import { updateDiagramElementsPosition } from '../../../_molecules/graphical-editor/ladder/rung/ladder-utils/elements/diagram'
import {
findInvalidatedBranches,
reconcileBranches,
} from '../../../_molecules/graphical-editor/ladder/rung/ladder-utils/elements/handle-branch'
import { HighlightedTextArea } from '../../highlighted-textarea'
import { InputWithRef } from '../../input'
import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '../../tooltip'
Expand Down Expand Up @@ -60,7 +64,7 @@ export const BlockNodeElement = <T extends object>({
editorActions: { updateModelVariables },
libraries,
ladderFlows,
ladderFlowActions: { setNodes, setEdges },
ladderFlowActions,
project: {
data: { pous },
},
Expand Down Expand Up @@ -176,6 +180,26 @@ export const BlockNodeElement = <T extends object>({
})
if (!pou || !rung || !node) return

/**
* Handle-branch reconciliation: a block-variant change can rename or
* retype handles in ways that orphan existing branches. If any branch
* on this block would be invalidated by the new variant, ask the user
* for confirmation before silently dropping them.
*/
const invalidatedBranches = findInvalidatedBranches(rung, node.id, libraryBlock as BlockVariant)
if (invalidatedBranches.length > 0) {
const handleNames = invalidatedBranches.map((b) => `${b.handleId} (${b.direction})`).join(', ')
const confirmed = window.confirm(
`Changing this block to "${blockNameValue}" will drop ${invalidatedBranches.length} handle ` +
`branch${invalidatedBranches.length === 1 ? '' : 'es'} on the following pin${invalidatedBranches.length === 1 ? '' : 's'}: ${handleNames}.\n\n` +
`Continue?`,
)
if (!confirmed) {
setBlockNameValue(validBlockNameValue)
return
}
}
Comment on lines +189 to +201
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

window.confirm blocks the renderer and bypasses i18n.

window.confirm is a synchronous, blocking call that halts the renderer thread and is generally discouraged in Electron/React apps (some Electron renderer configs disable it altogether, returning false and silently dropping the prompt). The hardcoded English copy also violates the i18n guideline for frontend components.

Consider replacing with an in-app modal (the project already has Modal/ModalContent patterns) and routing the strings through i18next.

As per coding guidelines: "Use i18next for internationalization in frontend components".

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/frontend/components/_atoms/graphical-editor/ladder/block.tsx` around
lines 189 - 201, Replace the blocking window.confirm flow in the block rename
logic (where invalidatedBranches is computed using findInvalidatedBranches and
state setters setBlockNameValue/validBlockNameValue are used) with a
non-blocking in-app confirmation modal using the project's Modal/ModalContent
pattern: when invalidatedBranches.length > 0, open the Modal and render the
message (include handle names from invalidatedBranches.map(b => `${b.handleId}
(${b.direction})`). Provide Confirm/Cancel buttons where Confirm proceeds with
the rename and closes the modal and Cancel calls
setBlockNameValue(validBlockNameValue) then closes the modal; route all
displayed strings through i18next (use translation keys for the headline, body
text with a pluralized branch/pin count, and button labels) so no hardcoded
English remains. Ensure the new flow preserves the original early-return
behavior when cancelled and does not block the renderer thread.


if (libraryBlock && pou.pouType === 'function' && (libraryBlock as BlockVariant).type !== 'function') {
setWrongName(true)
toast({
Expand Down Expand Up @@ -242,7 +266,7 @@ export const BlockNodeElement = <T extends object>({
}
}

let newNodes = [...rung.nodes]
let newNodes: typeof rung.nodes = [...rung.nodes]
let newEdges = [...rung.edges]

/**
Expand All @@ -263,7 +287,7 @@ export const BlockNodeElement = <T extends object>({
variable: variable ?? { name: '' },
}

newNodes = newNodes.map((n) => (n.id === node.id ? newBlockNode : n))
newNodes = newNodes.map((n) => (n.id === node.id ? newBlockNode : n)) as typeof newNodes

edges.source?.forEach((edge) => {
const newEdge = {
Expand All @@ -284,12 +308,23 @@ export const BlockNodeElement = <T extends object>({
newEdges = newEdges.map((e) => (e.id === edge.id ? newEdge : e))
})

// Reconcile any handle branches on this block before layout runs:
// - drop branches whose handle disappeared or stopped being BOOL
// - remap surviving branches' references to the new block id
const reconciled = reconcileBranches(
{ ...rung, nodes: newNodes, edges: newEdges } as typeof rung,
node.id,
newBlockNode.id,
libraryBlock as BlockVariant,
)

const { nodes: variableNodes, edges: variableEdges } = updateDiagramElementsPosition(
{
...rung,
nodes: newNodes,
edges: newEdges,
},
nodes: reconciled.nodes,
edges: reconciled.edges,
handleBranches: reconciled.handleBranches,
} as typeof rung,
[rung.defaultBounds[0], rung.defaultBounds[1]],
)

Expand All @@ -300,15 +335,12 @@ export const BlockNodeElement = <T extends object>({
body: currentPou2?.body.value,
})

setNodes({
ladderFlowActions.updateRungData({
editorName: editor.meta.name,
rungId: rung.id,
nodes: variableNodes,
})
setEdges({
editorName: editor.meta.name,
rungId: rung.id,
edges: variableEdges,
handleBranches: reconciled.handleBranches,
})

setWrongName(false)
Expand Down Expand Up @@ -353,7 +385,7 @@ export const BlockNodeElement = <T extends object>({
<div
key={index}
className='absolute text-xs'
style={{ top: DEFAULT_BLOCK_CONNECTOR_Y + index * DEFAULT_BLOCK_CONNECTOR_Y_OFFSET - 10, left: 6 }}
style={{ top: (data.inputHandles[index]?.relPosition.y ?? 0) - 10, left: 6 }}
>
{connector}
</div>
Expand All @@ -362,7 +394,7 @@ export const BlockNodeElement = <T extends object>({
<div
key={index}
className='absolute text-xs'
style={{ top: DEFAULT_BLOCK_CONNECTOR_Y + index * DEFAULT_BLOCK_CONNECTOR_Y_OFFSET - 10, right: 6 }}
style={{ top: (data.outputHandles[index]?.relPosition.y ?? 0) - 10, right: 6 }}
>
{connector}
</div>
Expand Down Expand Up @@ -619,7 +651,7 @@ export const Block = <T extends object>(block: BlockProps<T>) => {
const libPou = pous.find((pou) => pou.name === libMatch.name)
if (!libPou) return

const blockVariant = node.data.variant as BlockVariant
const blockVariant = (node.data as BlockNodeData<BlockVariant>).variant
const newNodeVariables = (libPou.interface?.variables ?? []).map((variable) => {
let newType
switch (variable.type.definition) {
Expand Down Expand Up @@ -717,10 +749,10 @@ export const Block = <T extends object>(block: BlockProps<T>) => {

const newBlockNode = { ...updatedNewNode }

let newNodes = [...rung.nodes]
let newNodes: typeof rung.nodes = [...rung.nodes]
let newEdges = [...rung.edges]

newNodes = newNodes.map((n) => (n.id === node.id ? newBlockNode : n))
newNodes = newNodes.map((n) => (n.id === node.id ? newBlockNode : n)) as typeof newNodes

edges.source?.forEach((edge) => {
const newEdge = {
Expand All @@ -746,7 +778,7 @@ export const Block = <T extends object>(block: BlockProps<T>) => {
...rung,
nodes: newNodes,
edges: newEdges,
},
} as typeof rung,
[rung.defaultBounds[0], rung.defaultBounds[1]],
)

Expand Down Expand Up @@ -841,7 +873,7 @@ export const Block = <T extends object>(block: BlockProps<T>) => {
rungId: rung.id,
node: {
...node,
draggable: node.data.draggable as boolean,
draggable: node.data.draggable,
},
})
return
Expand Down
Loading
Loading