Skip to content

Add feature to add and delete labels to selected nodes #139

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.

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Kr0nox
Copy link
Contributor

@Kr0nox Kr0nox commented May 27, 2025

When having no node selected, the label will be added to all nodes
When having a selection of nodes, the label will be added to the selected nodes only.

fixes DataFlowAnalysis/DataFlowAnalysis#274

@sebinside
Copy link
Member

Ideas discussed:

  • Adding multiple by select / select all
  • Removing multiple
  • Single click on label to apply / double click to edit / still support drag n drop
  • Left mouse to select multiple (draw selection box), right click to move view

@Kr0nox Kr0nox marked this pull request as ready for review May 27, 2025 18:18
@Kr0nox Kr0nox changed the title Add feature to add and delete labels to all nodes/the selection Add feature to add and delete labels to selected nodes May 30, 2025
@Entenwilli Entenwilli requested a review from 01Parzival10 June 3, 2025 11:04
@Kr0nox Kr0nox self-assigned this Jun 3, 2025
01Parzival10
01Parzival10 previously approved these changes Jun 4, 2025
Copy link
Contributor

@01Parzival10 01Parzival10 left a comment

Choose a reason for hiding this comment

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

LGTM

@Kr0nox Kr0nox requested a review from Nicolas-Boltz June 24, 2025 07:43
Copy link
Member

@sebinside sebinside left a comment

Choose a reason for hiding this comment

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

I tested the PR and the majority of features work as intended, thank you. I only noticed two things:

  • Drag n drop of labels is not possible anymore. We said that we want to still support it, if it is possible without major effort. What was your finding here?
  • Selecting multiple is not possible by drawing a selection box with the left mouse button as discussed. If this this would cause a major change, we can also merge this PR first and then add it later, what do you think?

@Kr0nox
Copy link
Contributor Author

Kr0nox commented Jun 24, 2025

Drag n drop of labels is not possible anymore. We said that we want to still support it, if it is possible without major effort. What was your finding here?

This works for me. Can you quickly walk me through the process, so I can make sure we talk about the same feature

Selecting multiple is not possible by drawing a selection box with the left mouse button as discussed. If this this would cause a major change, we can also merge this PR first and then add it later, what do you think?

We agreed on doing this separately due to it being a bigger change

@sebinside
Copy link
Member

This works for me. Can you quickly walk me through the process, so I can make sure we talk about the same feature

I meant dragging and dropping labels, which should perform the same action as clicking (applying them to all selected nodes)

@Kr0nox
Copy link
Contributor Author

Kr0nox commented Jun 25, 2025

The bug was only present on Chromium based browsers.
The issue was that due to the blur being called instantly, the onDrag listener was never triggered.
I fixed this by delaying the blur operation by the smallest possible time.
This can still result in the cursor being visible in the textbox for a short time any time it is clicked or dragged.

@Kr0nox Kr0nox requested a review from sebinside June 25, 2025 11:41
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.

Add global variables to DFDs
3 participants