Skip to content
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

fix: Add factor to prevent overlap #5426

Merged
merged 13 commits into from
Jan 9, 2025
Merged

fix: Add factor to prevent overlap #5426

merged 13 commits into from
Jan 9, 2025

Conversation

anovazzi1
Copy link
Contributor

This pull request fixes an issue where there was potential overlap when pasting a copied selection. The commit adds a random factor to the x and y coordinates of the pasted selection, preventing overlap.

@dosubot dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Dec 24, 2024
@dosubot dosubot bot added the bug Something isn't working label Dec 24, 2024
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Dec 24, 2024
Copy link
Contributor

@ogabrielluiz ogabrielluiz left a comment

Choose a reason for hiding this comment

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

I don't think a random factor is a good idea. If it is zero it will overlap unless you make it (1+random) * 100.

Does this fix all the cases where there could be overlap (e.g. when adding a component by clicking on the sidebar button)?

This commit adds the buildPositionDictionary function to reactflowUtils.ts. This function is used to build a dictionary of positions for nodes in the flow. It is necessary for managing the positions of nodes and preventing overlap.
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Jan 3, 2025
@anovazzi1 anovazzi1 requested a review from ogabrielluiz January 3, 2025 23:09
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Jan 3, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Jan 3, 2025
This commit adds the setPositionDictionary function to the PageComponent in order to set the position dictionary for the flow. This function is used to update the position dictionary when the canvas is moved or resized.

Co-authored-by: [Author Name]
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Jan 6, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Jan 6, 2025
Copy link
Collaborator

@jordanrfrazier jordanrfrazier left a comment

Choose a reason for hiding this comment

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

Seems good to me. Though, should it just increment the position by 1 (or some fixed distance) for x and y so it stays consistently diagonal? (i.e. why does it need the random factor?)

@anovazzi1
Copy link
Contributor Author

nice catch @jordanrfrazier, and it will require less dependencies

@anovazzi1 anovazzi1 changed the title fix: Add random factor to prevent overlap fix: Add factor to prevent overlap Jan 6, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Jan 6, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Jan 6, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jan 6, 2025
@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Jan 6, 2025
@anovazzi1 anovazzi1 enabled auto-merge January 6, 2025 18:59
@anovazzi1 anovazzi1 dismissed ogabrielluiz’s stale review January 9, 2025 13:46

the review is outadted to the current version

@github-actions github-actions bot added bug Something isn't working and removed bug Something isn't working labels Jan 9, 2025
@anovazzi1 anovazzi1 added this pull request to the merge queue Jan 9, 2025
Merged via the queue into main with commit 1f614cc Jan 9, 2025
22 checks passed
@anovazzi1 anovazzi1 deleted the improveCopyPaste branch January 9, 2025 14:06
ogabrielluiz pushed a commit to raphaelchristi/langflow that referenced this pull request Jan 10, 2025
* fix: add random factor to prevent overlap

* update package.lock

* feat: Add positionDictionary and related functions to FlowStoreType

* feat: Add buildPositionDictionary function to reactflowUtils.ts

* feat: Add buildPositionDictionary function to reactflowUtils.ts

This commit adds the buildPositionDictionary function to reactflowUtils.ts. This function is used to build a dictionary of positions for nodes in the flow. It is necessary for managing the positions of nodes and preventing overlap.

* fix: Remove random factor from paste function in PageComponent

* [autofix.ci] apply automated fixes

* feat: Add setPositionDictionary function to PageComponent

This commit adds the setPositionDictionary function to the PageComponent in order to set the position dictionary for the flow. This function is used to update the position dictionary when the canvas is moved or resized.

Co-authored-by: [Author Name]

* [autofix.ci] apply automated fixes

* Refactor position calculation in useFlowStore

* [autofix.ci] apply automated fixes

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants