Delete Edge Not Working

[satpalsr]

Describe the bug
I'm on latest dev branch.

I am not able to delete an edge between two nodes.
I have check logs and see that edge id is empty when pressing delete button.
Also may be edge should glow up a bit when selected to show clear indication of it being selected.
or a (x) cross button can be added between edges to easily delete it.

Browser and Version

• Chrome

To Reproduce
Steps to reproduce the behavior:

1. Connect two Nodes
2. Left click on edge (I do see cursor changing to hand icon)
3. Press delete.

[dosubot]

To address the edge deletion issue and improve usability in your project, here are some targeted solutions:

1. Edge Deletion Not Working: The problem might stem from the event listener not correctly identifying the delete key press or not having the correct edge ID to perform the deletion. Ensure your event listener for the delete action is correctly set up to capture the Delete or Backspace key presses and is calling the deleteEdge function with the correct edge ID. Here's a snippet to guide you:

   document.addEventListener("keydown", (event) => {
     if ((event.key === "Delete" || event.key === "Backspace") && lastSelection) {
       event.preventDefault();
       // Assuming deleteNode and deleteEdge functions are defined elsewhere
       deleteEdge(lastSelection.edges.map((edge) => edge.id));
     }
   });

   Make sure lastSelection accurately captures the selected edges and that deleteEdge is implemented to handle edge deletion based on the edge's ID.

2. Improving Edge Visibility: To make an edge more visible when selected, consider dynamically adjusting its style based on its selection state. This involves modifying the edge's style properties when it's selected to make it stand out. You'll need to track edge selection and apply style changes accordingly.

3. Easier Edge Deletion via UI: Introducing a cross button (X) on the edge for easier deletion can enhance user interaction. This requires customizing the edge component to include a button when rendering edges and managing the button's visibility and the deletion logic upon interaction.

These steps should help you resolve the edge deletion issue and improve the user experience by making edges more visible when selected and easier to delete.

Details

Sources

• src/frontend/src/stores/flowStore.ts
• src/frontend/src/pages/FlowPage/components/PageComponent/index.tsx
• src/frontend/src/stores/flowStore.ts
• src/frontend/src/pages/FlowPage/components/PageComponent/index.tsx
• src/frontend/src/utils/reactflowUtils.ts

About Dosu
This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

To continue the conversation, mention @dosu.

[satpalsr]

@dosu When I select edge and press delete, the edge id or info does not appear in lastselection
lastSelection: {"nodes":[],"edges":[]}

[dosubot]

To resolve the issue where lastSelection does not update with the edge's ID upon selection, ensuring that the onSelectionChange callback is correctly implemented is crucial. This callback should update lastSelection with the selected edge's information. If lastSelection remains empty, it indicates that the selection event might not be capturing or updating the state as expected. Here's a quick check to ensure your callback is set up correctly:

const onSelectionChange = useCallback(
  (flow: OnSelectionChangeParams): void => {
    setLastSelection(flow);
  },
  [],
);

Ensure that OnSelectionChangeParams correctly handles edge data and that setLastSelection is being called with the right parameters.

For the UI improvements and making edge deletion more intuitive:

1. Highlight Selected Edges: Implement a function to visually highlight an edge when it's selected. This could involve changing the edge's stroke color or width to make it stand out.

2. Improve Edge Selection Detection: Adjust the hitbox size for edge selection or implement custom logic to ensure clicks near an edge path are detected accurately.

3. Delete Key Binding: Confirm that the delete key event listener is set up to identify when an edge is selected. It should call the deleteEdge function with the correct edge ID. Here's an example of how you might set up such an event listener:

useEffect(() => {
  const onKeyDown = (event: KeyboardEvent) => {
    if ((event.key === "Delete" || event.key === "Backspace") && lastSelection?.edges.length > 0) {
      event.preventDefault();
      deleteEdge(lastSelection.edges.map((edge) => edge.id));
    }
  };

  document.addEventListener("keydown", onKeyDown);
  return () => {
    document.removeEventListener("keydown", onKeyDown);
  };
}, [lastSelection, deleteEdge]);

4. Visual Feedback on Selection: Ensure there's immediate visual feedback when an edge is selected, such as a change in color or a tooltip, to confirm it's ready for deletion.

Implementing these steps should help in resolving the edge deletion issue and improving the overall user experience in your project.

Details

Sources

• src/frontend/src/pages/FlowPage/components/PageComponent/index.tsx
• src/frontend/src/pages/FlowPage/components/PageComponent/index.tsx
• src/frontend/src/stores/flowStore.ts
• src/frontend/src/stores/flowStore.ts
• src/frontend/src/stores/flowsManagerStore.ts

About Dosu
This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

To continue the conversation, mention @dosu.

[satpalsr]

@dosu The edge is correctly shown here when selected

const onSelectionChange = useCallback(
  (flow: OnSelectionChangeParams): void => {
    setLastSelection(flow);
  },
  [],
);

However, the lastSelection is empty here

if (!isWrappedWithClass(event, "nodelete")) {
        if (
          (event.key === "Delete" || event.key === "Backspace") &&
          lastSelection
        ) {
          event.preventDefault();
          takeSnapshot();
          console.log(`lastSelection: ${JSON.stringify(lastSelection)}`);
          console.log(`DeleteNode ${lastSelection.nodes.map((node) => node.id)}`);
          deleteNode(lastSelection.nodes.map((node) => node.id));
          console.log(`DeleteEdge ${lastSelection.edges.map((edge) => edge.id)}`);
          deleteEdge(lastSelection.edges.map((edge) => edge.id));
        }
      }
    };
    ```
    
    However, If I hardcode the id here for deleteEdge then it works. So for sure the problem is that lastselection don't have the selected edge here before deleting it.

[dosubot]

To tackle the issue where lastSelection doesn't contain the selected edge's information before deletion, despite the edge being correctly shown as selected, consider the following steps:

1. Ensure Event Firing: Verify that the onSelectionChange event consistently fires and updates lastSelection whenever an edge is selected. Adding console logs or using a debugger can help confirm that setLastSelection is called with the correct edge information.

2. Check Conditional Logic: Examine any conditional logic within your event handlers or useEffect hooks that might prevent lastSelection from updating. It's crucial that no conditions inadvertently block the state update.

3. Review State Management: Look for any parts of your code that might unintentionally reset or overwrite lastSelection. This includes all occurrences of setLastSelection and any other state manipulations that could affect it.

4. Direct Edge Deletion Handling: If the problem persists, consider a more direct approach to handling edge deletion that doesn't rely on lastSelection. Managing edge selection state more explicitly and using this state for deletion could offer a more reliable solution.

By focusing on these areas, you should be able to ensure that lastSelection accurately reflects the selected edge's information, allowing for successful deletion.

Details

Sources

• src/frontend/src/pages/FlowPage/components/PageComponent/index.tsx
• src/frontend/src/stores/flowStore.ts
• src/frontend/src/pages/FlowPage/components/SelectionMenuComponent/index.tsx
• src/frontend/src/utils/utils.ts
• src/frontend/src/pages/FlowPage/components/PageComponent/index.tsx
• src/frontend/src/stores/flowsManagerStore.ts

About Dosu
This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

To continue the conversation, mention @dosu.