fix group clone connection

2026-03-27 20:59:08 -07:00
parent 160f714bad
commit 66f1bca046
4 changed files with 309 additions and 12 deletions
--- a/frontend/src/canvasInteractionTargets.js
+++ b/frontend/src/canvasInteractionTargets.js
@@ -0,0 +1,49 @@
+const EXCLUDED_CANVAS_TARGETS = '.context-menu, .react-flow__node, .react-flow__edge, .react-flow__controls, .react-flow__minimap, .surface-view-container';
+const CANVAS_AREA_TARGETS = '.react-flow, .react-flow__renderer, .react-flow__viewport, .react-flow__pane, .react-flow__background, .react-flow__selectionpane';
+
+function getTargetElement(target) {
+  if (!target) return null;
+  if (typeof target.closest === 'function') return target;
+  if (target.parentElement && typeof target.parentElement.closest === 'function') {
+    return target.parentElement;
+  }
+  return null;
+}
+
+function supportsClosest(target) {
+  return !!getTargetElement(target);
+}
+
+function matchesClosest(target, selector) {
+  const element = getTargetElement(target);
+  return !!element && element.closest(selector) !== null;
+}
+
+export function isEditableInteractionTarget(target) {
+  if (!supportsClosest(target)) return false;
+  if (matchesClosest(target, 'input, textarea, select')) return true;
+  return matchesClosest(target, '[contenteditable="true"]');
+}
+
+export function canStartCanvasRightDragZoomTarget(target) {
+  if (!supportsClosest(target)) return false;
+  if (isEditableInteractionTarget(target)) return false;
+  if (matchesClosest(target, EXCLUDED_CANVAS_TARGETS)) {
+    return false;
+  }
+  return matchesClosest(target, CANVAS_AREA_TARGETS);
+}
+
+export function canOpenCanvasContextMenuTarget(target) {
+  if (!supportsClosest(target)) return false;
+  if (isEditableInteractionTarget(target)) return false;
+  if (matchesClosest(target, EXCLUDED_CANVAS_TARGETS)) {
+    return false;
+  }
+  return matchesClosest(target, CANVAS_AREA_TARGETS);
+}
+
+export function isSecondaryCanvasContextEvent(event) {
+  if (!event || typeof event.button !== 'number') return false;
+  return event.button === 2 || (event.button === 0 && !!event.ctrlKey);
+}
--- a/frontend/src/nodeClipboard.js
+++ b/frontend/src/nodeClipboard.js
@@ -20,6 +20,102 @@ function clonePlainObject(value) {
  return cloneValue(value) || {};
 }

+function encodeProxyHandleRef(handleId) {
+  return encodeURIComponent(String(handleId || ''));
+}
+
+function decodeProxyHandleRef(encoded) {
+  try {
+    return decodeURIComponent(String(encoded || ''));
+  } catch {
+    return String(encoded || '');
+  }
+}
+
+function parseGroupProxyHandle(handleId) {
+  const text = String(handleId || '');
+  if (!text.startsWith('group-proxy::')) return null;
+  const parts = text.split('::');
+  if (parts.length < 5) return null;
+  return {
+    direction: parts[1],
+    nodeId: parts[2],
+    type: parts[3],
+    realHandle: decodeProxyHandleRef(parts.slice(4).join('::')),
+  };
+}
+
+function hasOwn(obj, key) {
+  return Object.prototype.hasOwnProperty.call(obj, key);
+}
+
+function remapNodeId(value, idMap) {
+  if (value == null) return value;
+  return idMap.get(String(value)) || String(value);
+}
+
+function remapGroupProxyHandle(handleId, idMap) {
+  const proxy = parseGroupProxyHandle(handleId);
+  if (!proxy) return handleId;
+  return `group-proxy::${proxy.direction}::${remapNodeId(proxy.nodeId, idMap)}::${proxy.type}::${encodeProxyHandleRef(proxy.realHandle)}`;
+}
+
+function remapGroupProxyDescriptors(items, idMap) {
+  if (!Array.isArray(items)) return items;
+  return items.map((item) => {
+    if (!item || typeof item !== 'object') return item;
+    const nextItem = { ...item };
+    if (typeof nextItem.key === 'string') {
+      const separator = nextItem.key.indexOf('::');
+      if (separator !== -1) {
+        const handleId = nextItem.key.slice(separator + 2);
+        nextItem.key = `${remapNodeId(nextItem.key.slice(0, separator), idMap)}::${remapGroupProxyHandle(handleId, idMap)}`;
+      }
+    }
+    if (typeof nextItem.handleId === 'string') {
+      nextItem.handleId = remapGroupProxyHandle(nextItem.handleId, idMap);
+    }
+    return nextItem;
+  });
+}
+
+function remapClipboardExtraData(extraData, idMap) {
+  const nextExtraData = clonePlainObject(extraData);
+  if (Array.isArray(nextExtraData.proxyInputs)) {
+    nextExtraData.proxyInputs = remapGroupProxyDescriptors(nextExtraData.proxyInputs, idMap);
+  }
+  if (Array.isArray(nextExtraData.proxyOutputs)) {
+    nextExtraData.proxyOutputs = remapGroupProxyDescriptors(nextExtraData.proxyOutputs, idMap);
+  }
+  return nextExtraData;
+}
+
+function remapClipboardEdgeData(data, idMap) {
+  if (!data || typeof data !== 'object' || Array.isArray(data)) return cloneValue(data);
+
+  const nextData = cloneValue(data);
+  if (hasOwn(nextData, 'groupInternalHiddenBy')) {
+    nextData.groupInternalHiddenBy = remapNodeId(nextData.groupInternalHiddenBy, idMap);
+  }
+  if (hasOwn(nextData, 'groupProxyOwner')) {
+    nextData.groupProxyOwner = remapNodeId(nextData.groupProxyOwner, idMap);
+  }
+
+  const original = nextData.groupProxyOriginal;
+  if (original && typeof original === 'object' && !Array.isArray(original)) {
+    if (hasOwn(original, 'source')) original.source = remapNodeId(original.source, idMap);
+    if (hasOwn(original, 'target')) original.target = remapNodeId(original.target, idMap);
+    if (hasOwn(original, 'sourceHandle')) {
+      original.sourceHandle = remapGroupProxyHandle(original.sourceHandle, idMap);
+    }
+    if (hasOwn(original, 'targetHandle')) {
+      original.targetHandle = remapGroupProxyHandle(original.targetHandle, idMap);
+    }
+  }
+
+  return nextData;
+}
+
 function collectSelectedNodeIds(nodes, nodeIds) {
  const selectedIdSet = new Set((Array.isArray(nodeIds) ? nodeIds : []).map((id) => String(id)));
  if (selectedIdSet.size === 0) return selectedIdSet;
@@ -161,6 +257,7 @@ export function instantiateNodeClipboardPayload(
    const newId = idMap.get(String(node.id));
    const className = node.data?.className || '';
    const definition = className ? defs[className] || null : null;
+    const extraData = remapClipboardExtraData(node.data?.extraData, idMap);

    return {
      id: newId,
@@ -181,7 +278,7 @@ export function instantiateNodeClipboardPayload(
        className,
        widgetValues: clonePlainObject(node.data?.widgetValues),
        runtimeValues: clonePlainObject(node.data?.runtimeValues),
-        ...(clonePlainObject(node.data?.extraData)),
+        ...extraData,
        definition,
        previewImage: null,
        tableRows: null,
@@ -199,17 +296,21 @@ export function instantiateNodeClipboardPayload(
      idMap.has(String(edge.target))
      && (idMap.has(String(edge.source)) || keepExternalSources)
    ))
-    .map((edge, index) => ({
-      id: `e${idMap.get(String(edge.source)) || String(edge.source)}-${idMap.get(String(edge.target))}-${index}`,
-      source: idMap.get(String(edge.source)) || String(edge.source),
-      sourceHandle: edge.sourceHandle,
-      target: idMap.get(String(edge.target)),
-      targetHandle: edge.targetHandle,
-      selected: false,
-      ...(edge.style ? { style: { ...edge.style } } : {}),
-      ...(edge.hidden ? { hidden: true } : {}),
-      ...(edge.data ? { data: cloneValue(edge.data) } : {}),
-    }));
+    .map((edge, index) => {
+      const source = idMap.get(String(edge.source)) || String(edge.source);
+      const target = idMap.get(String(edge.target));
+      return {
+        id: `e${source}-${target}-${index}`,
+        source,
+        sourceHandle: remapGroupProxyHandle(edge.sourceHandle, idMap),
+        target,
+        targetHandle: remapGroupProxyHandle(edge.targetHandle, idMap),
+        selected: false,
+        ...(edge.style ? { style: { ...edge.style } } : {}),
+        ...(edge.hidden ? { hidden: true } : {}),
+        ...(edge.data ? { data: remapClipboardEdgeData(edge.data, idMap) } : {}),
+      };
+    });

  return {
    nodes,
--- a/frontend/tests/canvasInteractionTargets.test.mjs
+++ b/frontend/tests/canvasInteractionTargets.test.mjs
@@ -0,0 +1,49 @@
+import test from 'node:test';
+import assert from 'node:assert/strict';
+
+import {
+  canOpenCanvasContextMenuTarget,
+  canStartCanvasRightDragZoomTarget,
+  isEditableInteractionTarget,
+  isSecondaryCanvasContextEvent,
+} from '../src/canvasInteractionTargets.js';
+
+function makeTarget(activeSelectors = []) {
+  const selectorSet = new Set(activeSelectors);
+  return {
+    closest(selector) {
+      const parts = String(selector).split(',').map((part) => part.trim());
+      return parts.some((part) => selectorSet.has(part)) ? {} : null;
+    },
+  };
+}
+
+test('editable canvas targets stay editable', () => {
+  const inputTarget = makeTarget(['input']);
+  assert.equal(isEditableInteractionTarget(inputTarget), true);
+  assert.equal(canOpenCanvasContextMenuTarget(inputTarget), false);
+  assert.equal(canStartCanvasRightDragZoomTarget(inputTarget), false);
+});
+
+test('empty pane targets allow the custom canvas context menu', () => {
+  const paneTarget = makeTarget(['.react-flow__pane']);
+  assert.equal(canOpenCanvasContextMenuTarget(paneTarget), true);
+  assert.equal(canStartCanvasRightDragZoomTarget(paneTarget), true);
+});
+
+test('viewport-level targets also allow the custom canvas context menu', () => {
+  const viewportTarget = makeTarget(['.react-flow__viewport']);
+  assert.equal(canOpenCanvasContextMenuTarget(viewportTarget), true);
+  assert.equal(canStartCanvasRightDragZoomTarget(viewportTarget), true);
+});
+
+test('node and surface-view targets do not open the canvas context menu', () => {
+  assert.equal(canOpenCanvasContextMenuTarget(makeTarget(['.react-flow__node', '.react-flow__pane'])), false);
+  assert.equal(canOpenCanvasContextMenuTarget(makeTarget(['.surface-view-container', '.react-flow__pane'])), false);
+});
+
+test('secondary canvas context detection includes macOS ctrl-click', () => {
+  assert.equal(isSecondaryCanvasContextEvent({ button: 2, ctrlKey: false }), true);
+  assert.equal(isSecondaryCanvasContextEvent({ button: 0, ctrlKey: true }), true);
+  assert.equal(isSecondaryCanvasContextEvent({ button: 0, ctrlKey: false }), false);
+});
--- a/frontend/tests/nodeClipboard.test.mjs
+++ b/frontend/tests/nodeClipboard.test.mjs
@@ -290,3 +290,101 @@ test('clipboard payload preserves wrapper class names for group shells', () => {
  assert.equal(payload.nodes[0].className, 'group-shell');
  assert.equal(instantiated.nodes[0].className, 'group-shell');
 });
+
+test('instantiateNodeClipboardPayload remaps collapsed group proxy metadata to cloned child ids', () => {
+  const payload = {
+    kind: NODE_CLIPBOARD_KIND,
+    version: 1,
+    nodes: [
+      {
+        id: '10',
+        type: 'custom',
+        className: 'group-shell',
+        position: { x: 40, y: 50 },
+        style: { width: 320, height: 220 },
+        data: {
+          label: 'group',
+          className: 'Group',
+          widgetValues: {},
+          extraData: {
+            collapsed: true,
+            childCount: 1,
+            proxyInputs: [
+              {
+                key: '2::input::field::DATA_FIELD',
+                type: 'DATA_FIELD',
+                label: 'field',
+                handleId: 'group-proxy::in::2::DATA_FIELD::input%3A%3Afield%3A%3ADATA_FIELD',
+              },
+            ],
+          },
+        },
+      },
+      {
+        id: '2',
+        type: 'custom',
+        parentId: '10',
+        extent: 'parent',
+        hidden: true,
+        position: { x: 24, y: 48 },
+        data: {
+          label: 'preview',
+          className: 'Preview',
+          widgetValues: {},
+        },
+      },
+    ],
+    edges: [
+      {
+        source: '1',
+        sourceHandle: 'output::0::DATA_FIELD',
+        target: '10',
+        targetHandle: 'group-proxy::in::2::DATA_FIELD::input%3A%3Afield%3A%3ADATA_FIELD',
+        data: {
+          groupProxyOwner: '10',
+          groupProxyOriginal: {
+            target: '2',
+            targetHandle: 'input::field::DATA_FIELD',
+          },
+        },
+      },
+    ],
+  };
+
+  const instantiated = instantiateNodeClipboardPayload(
+    payload,
+    {},
+    20,
+    { x: 0, y: 0 },
+    { keepExternalSources: true },
+  );
+
+  assert.equal(instantiated.nextNodeId, 22);
+  assert.deepEqual(instantiated.nodes.map((node) => node.id), ['20', '21']);
+  assert.deepEqual(instantiated.edges, [
+    {
+      id: 'e1-20-0',
+      source: '1',
+      sourceHandle: 'output::0::DATA_FIELD',
+      target: '20',
+      targetHandle: 'group-proxy::in::21::DATA_FIELD::input%3A%3Afield%3A%3ADATA_FIELD',
+      selected: false,
+      data: {
+        groupProxyOwner: '20',
+        groupProxyOriginal: {
+          target: '21',
+          targetHandle: 'input::field::DATA_FIELD',
+        },
+      },
+    },
+  ]);
+
+  assert.deepEqual(instantiated.nodes[0].data.proxyInputs, [
+    {
+      key: '21::input::field::DATA_FIELD',
+      type: 'DATA_FIELD',
+      label: 'field',
+      handleId: 'group-proxy::in::21::DATA_FIELD::input%3A%3Afield%3A%3ADATA_FIELD',
+    },
+  ]);
+});