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

Restrict Graph to Viewport Hotfix: Navbar #1119

Merged
merged 2 commits into from
Oct 2, 2024
Merged

Conversation

ceciliazaragoza
Copy link
Collaborator

Hotfix for 7.1 release #1077. Graph is updated to fit within bounds when zoomed into the graph when Restrict Graph to Viewport is selected in navbar. Do this by calling tick() when the restrict graph to viewport option is clicked in navbar, just like how tick is called when click Restrict Graph to Viewport option in sidebar.

Copy link
Owner

@dondi dondi left a comment

Choose a reason for hiding this comment

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

Great find!

One side thought—should invocation of tick be a standard action for all cases when restrictGraphToViewport is called? Because if so, maybe we can put the tick() call inside restrictGraphToViewport instead?

@ceciliazaragoza
Copy link
Collaborator Author

I placed tick() call at the end of restrictGraphToViewport method and it works!

Copy link
Owner

@dondi dondi left a comment

Choose a reason for hiding this comment

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

Nice! Anytime we can identify repeated code that turns out to be an inherent part of an operation, we same on code lines and are less error-prone (in case someone performs the operation elsewhere but forgets to add that essential repetition)

@dondi dondi merged commit bd823fc into master Oct 2, 2024
@dondi dondi deleted the cecilia-1083-hotfix branch October 2, 2024 18:01
@dondi
Copy link
Owner

dondi commented Oct 2, 2024

@ceciliazaragoza this has been deployed to production—you can verify if the fix now shows up on our website

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants