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 reset of viewport does not set the origin to (0, 0) #89

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jpk6789
Copy link

@jpk6789 jpk6789 commented Feb 25, 2024

Describe the bug
If the ResetViewportLocation gesture is performed, the viewport is not reset to (0, 0) as stated in the summary. Instead, the point (0, 0) is centered.

To Reproduce

  1. Move the viewport to any location
  2. Hit the "Home" button / call OnBringIntoView

This PR fixes this behavior by changing the PringIntoView point to a nullable and use this info to decide if the ViewportSize shall be taken into account or not.

@miroiu
Copy link
Owner

miroiu commented Feb 25, 2024

Hi! The NodifyEditor.BringIntoView method centers the viewport to the specified location (as written in the summary: Moves the viewport center at the specified location.), and it's working as intended. However, the editor command bound to the ResetViewportLocation gesture is wrong and the summary of the BringIntoView routed UI command is also wrong.

I would not change the behavior of NodifyEditor.BringIntoView but instead create another command for resetting the viewport location. Setting the ViewportLocation to any location moves the top-left corner of the viewport to that location.

@jpk6789
Copy link
Author

jpk6789 commented Feb 25, 2024

Ok, then I change my PR accordingly and create a new command for that. Yes, I saw that the top-left corner is set by the ViewportLocation :)
But if I add this new function, would it be ok to also reset the zoom factor to 1 in it? I expected such a function to fully reset the location / scale in the first place.

@miroiu
Copy link
Owner

miroiu commented Feb 25, 2024

It's ok to reset the zooming. But in this case, I would give it a generic name like ResetViewport.

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