Skip to content

fix: v2 - sync visual selected state of the node#2196

Open
maxy-shpfy wants to merge 1 commit into05-03-fix_v2_-_rectangle_selection_of_one_nodefrom
05-03-fix_v2_-_sync_visual_selected_state_of_the_node
Open

fix: v2 - sync visual selected state of the node#2196
maxy-shpfy wants to merge 1 commit into05-03-fix_v2_-_rectangle_selection_of_one_nodefrom
05-03-fix_v2_-_sync_visual_selected_state_of_the_node

Conversation

@maxy-shpfy
Copy link
Copy Markdown
Collaborator

@maxy-shpfy maxy-shpfy commented May 4, 2026

Description

Fix-try

Fixes a visual bug where two nodes could simultaneously display the "selected" ring when switching between nodes. React Flow's selected prop can lag one frame behind the store's selectedNodeId, causing both the previously selected and newly selected nodes to appear highlighted at the same time.

A new utility function isEditorVisualNodeSelected is introduced to determine the correct selected state for a node by using the editor store as the source of truth:

  • If editor.selectedNodeId is set, only that node is highlighted.
  • If editor.multiSelection is non-empty, the store's list is used.
  • Otherwise, React Flow's selected prop is used as a fallback (e.g., during rectangle drag before multi-selection sync).

This logic is applied to TaskNode, IONode, and EditorV2FlexNode.

Related Issue and Pull requests

Type of Change

  • Bug fix

Checklist

  • I have tested this does not break current pipelines / runs functionality
  • I have tested the changes on staging

Screenshots (if applicable)

Test Instructions

  1. Open the editor canvas with multiple nodes.
  2. Click one node to select it, then immediately click another node.
  3. Verify that only the newly clicked node shows the selected ring, with no lingering highlight on the previously selected node.
  4. Test rectangle drag selection to confirm multi-select still highlights all selected nodes correctly.

Selection ring vs context panel (v2 canvas)

Root cause

Canvas nodes were styling “selected” from React Flow’s selected prop, while the context panel and most editor behavior use EditorStore (selectedNodeId, multiSelection). Those two sources can diverge briefly:

  1. useFlowCanvasState syncs spec-derived nodes in a useLayoutEffect via preserveSelection(current, specNodes). If a MobX-driven specNodes refresh runs close to a click, current may not yet include React Flow’s latest select change, so preserveSelection sees no selected ids and reapplies nodes without selected: true. The store updates from onSelectionChange / click handlers (context panel looks correct), but the ring can disappear until selection changes again.
  2. A naive fix (OR !!selected with editor.selectedNodeId === id) fixed the missing ring but introduced two rings on fast sequential clicks: editor.selectNode(newId) updates immediately, while the previous node can still receive selected: true from React Flow for a frame—so both old (RF) and new (store) appeared selected.

Options considered

Approach Pros Cons
Fix inuseFlowCanvasStateonly (e.g. fold editor.selectedNodeId into preserveSelection, or restore render-time sync) Addresses divergence at the source Touches all canvas consumers; higher regression risk; trickier ordering vs RF internals
Visual selection = store only Single source of truth in UI Rectangle multi-select is debounced in useSelectionBehavior; rings would lag until multiSelection updates
Precedence helperisEditorVisualNodeSelected Scoped to node views; keeps immediate rect-drag feedback from RF; fixes both “no ring” and “double ring” Extra helper + call sites; precedence rules must stay documented

What we chose and why

We added isEditorVisualNodeSelected(editor, nodeId, rfSelected) with explicit precedence:

  1. If editor.selectedNodeId is set → only that node is visually selected (stale RF selected on other nodes is ignored — fixes double ring).
  2. Else if editor.multiSelection is non-empty → visuals follow the store list (post-debounce multi-select truth).
  3. Else → use React Flow’s selected (rectangle drag before debounced multi-sync, empty store, etc.).

This aligns single-select visuals with the store (and the context panel), preserves immediate multi-select feedback from RF before debounce, and avoids a larger, riskier refactor of the global canvas sync pipeline for this change.

Copy link
Copy Markdown
Collaborator Author

maxy-shpfy commented May 4, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 4, 2026

🎩 Preview

A preview build has been created at: 05-03-fix_v2_-_sync_visual_selected_state_of_the_node/9c718ae

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant