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 zoomToFitNodes offsets calculation #875

Merged

Conversation

GaryPan-1515
Copy link
Contributor

@GaryPan-1515 GaryPan-1515 commented Aug 13, 2021

Checklist

  • The code has been run through pretty yarn run pretty
  • The tests pass on CircleCI
  • You have referenced the issue(s) or other PR(s) this fixes/relates-to
  • The PR Template has been filled out (see below)
  • Had a beer/coffee because you are awesome

What?

The offset calculation method in the DiagramEngine.zoomToFitNodes has problems. This PR fixes the issue

Before

before

After

after

Why?

How?

The nodes bounding box's coordinates were not scaled by the zoom factor causing this bug

Feel good image:

(Add your own one below :])

LOL

Copy link

@DmitriyMenshayev DmitriyMenshayev left a comment

Choose a reason for hiding this comment

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

Works in practice, didn't check the math though.

@GaryPan-1515
Copy link
Contributor Author

@dylanvorster Would you take a look at this fix? Thanks!

@dalex-am
Copy link

@dylanvorster Would you take a look at this fix? Thanks!

I fixed zoomToFit method in my project and got same results with one difference: in the end of offsets I got margin * zoomFactor instead of just margin

@dylanvorster dylanvorster merged commit e4848b4 into projectstorm:master Feb 7, 2022
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.

4 participants