Skip to content

fix: v2 - proper undo for del action on canvas#2194

Open
maxy-shpfy wants to merge 1 commit intomasterfrom
05-01-fix_v2_-_proper_undo_for_del_action_on_canvas
Open

fix: v2 - proper undo for del action on canvas#2194
maxy-shpfy wants to merge 1 commit intomasterfrom
05-01-fix_v2_-_proper_undo_for_del_action_on_canvas

Conversation

@maxy-shpfy
Copy link
Copy Markdown
Collaborator

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

Description

Closes https://github.com/Shopify/oasis-frontend/issues/609

Delete and Backspace key handling for the canvas has been moved out of ReactFlow's built-in deleteKeyCode prop and into a dedicated useCanvasDeleteShortcuts hook. This gives full control over deletion logic, allowing mixed selections of nodes and edges to be deleted together in a single undo group.

The useNodeEdgeChanges hook no longer handles remove-type changes from ReactFlow — those change events are now filtered out entirely, preventing ReactFlow from triggering its own deletion path. Instead, useCanvasDeleteShortcuts drives all deletion through the editor's action layer, correctly handling conduit metadata cleanup (stripBindingIdsFromConduitMetadata) and undo grouping.

deleteSelectedNodesCore has been extracted from deleteSelectedNodes to allow node deletion to be composed with edge deletion inside a shared undo group. edgeIdToBindingId and removeBindingsAndStripConduits have been introduced to consolidate edge-to-binding resolution and conduit cleanup. DELETE and BACKSPACE key constants are now exported from keys.ts for use in the new hook.

Related Issue and Pull requests

Type of Change

  • Bug fix
  • Improvement
  • Cleanup/Refactor

Checklist

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

Screenshots (if applicable)

Screen Recording 2026-05-03 at 4.35.52 PM.mov (uploaded via Graphite)

Test Instructions

  1. Select one or more nodes on the canvas and press Delete or Backspace — nodes should be removed and the action should be undoable as a single step.
  2. Select one or more edges and press Delete or Backspace — edges and their associated bindings should be removed, with conduit metadata cleaned up.
  3. Select a mix of nodes and edges and delete — all should be removed in a single undo group.
  4. Verify that undo correctly restores the deleted nodes and edges in all three cases.

Additional Comments

The actions.ts barrel file has been marked with a todo: remove this file comment and deleteEdge has been removed from its exports as part of the ongoing cleanup of the action layer.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 2, 2026

🎩 Preview

A preview build has been created at: 05-01-fix_v2_-_proper_undo_for_del_action_on_canvas/4b8814b

Copy link
Copy Markdown
Collaborator Author

maxy-shpfy commented May 2, 2026

Copy link
Copy Markdown
Collaborator

camielvs commented May 4, 2026

I'm curious the motivation to skip over ReactFlow's built-in deletion and make our own custom logic?
Can't we hook into the native delete flow using https://reactflow.dev/api-reference/types/on-delete?

@maxy-shpfy maxy-shpfy force-pushed the 05-01-fix_v2_-_proper_undo_for_del_action_on_canvas branch from 5a1f6ef to 4b8814b Compare May 5, 2026 05:48
Copy link
Copy Markdown
Collaborator Author

@camielvs fair question. I explored the option. it is doable via onBeforeDelete. I applied the change

@maxy-shpfy maxy-shpfy requested a review from camielvs May 5, 2026 05:49
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.

2 participants