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 the unintentional movement caused by snapping correction #75

Merged
merged 3 commits into from
Oct 31, 2023

Conversation

fantasydr
Copy link
Contributor

📝 Description of the Change

Check the actual movement before apply the snapping correction.

When the snapping correction is enabled, the graph attempts to align the nodes to the grids.
However, even if a user merely selects a node, it might still move and snap to the grid.
Furthermore, when using grouped nodes, such as comment in the playground example, selecting the comment can inadvertently select many nodes. This can lead to several unintentional movements.

🐛 Possible Drawbacks

Are there any possible side-effects or negative impacts with this code change?

I think not.
Users who wish to adjust nodes based on grid size can easily nudge them slightly to achieve the desired alignment.

…ction is enabled.

This issue causes unintentional movement, especially when using grouped nodes such as the comment node.
@miroiu
Copy link
Owner

miroiu commented Oct 14, 2023

Hi, thanks for your contribution, but the fix also has a side effect. It prevents the snapping when moving the selection only in one axis.

@fantasydr
Copy link
Contributor Author

Hi, thanks for your contribution, but the fix also has a side effect. It prevents the snapping when moving the selection only in one axis.

Oh you're right, it should be || instead of &&.
I just updated the branch.

@miroiu
Copy link
Owner

miroiu commented Oct 31, 2023

Looks good. Please update the changelog with the fix before I merge it. Thanks!

@fantasydr
Copy link
Contributor Author

Looks good. Please update the changelog with the fix before I merge it. Thanks!

Thanks for the review. Updated the changelog file.

@miroiu miroiu merged commit 1f222b8 into miroiu:master Oct 31, 2023
3 checks passed
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