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

zoom/pan should be saved and restored on reloading if "autofit on new design" is off #894

Open
dave-doty opened this issue Aug 5, 2023 · 7 comments
Assignees
Labels
enhancement New feature or request

Comments

@dave-doty
Copy link
Member

With this option:

image

If you are at a particular zoom/pan level:

image

Reloading the page should restore the same zoom/pan. However, it resets it to something that is neither the previous zoom/pan, nor autofit:

image

@dave-doty dave-doty added the bug Something isn't working label Aug 5, 2023
@pranav-gupta1
Copy link
Contributor

pranav-gupta1 commented Aug 9, 2023

With this option:

image

If you are at a particular zoom/pan level:

image

Reloading the page should restore the same zoom/pan. However, it resets it to something that is neither the previous zoom/pan, nor autofit:

image

@dave-doty Do I need to use the same design to replicate this problem? I tried to replace this bug with a different design and when I have the "auto-fit on loading new design" box checked, if I zoom/pan to a random amount and then I reload the page, it is being auto-fitted to the default autofit size every time. That is what we want, correct? Please let me know as I feel like I may be misunderstanding something.

@dave-doty
Copy link
Member Author

I tried to replace this bug with a different design and when I have the "auto-fit on loading new design" box checked, if I zoom/pan to a random amount and then I reload the page, it is being auto-fitted to the default autofit size every time. That is what we want, correct? Please let me know as I feel like I may be misunderstanding something.

Then I guess we need to figure out why it's doing the correct thing on one design and not another. :)

I suspect it might be a difference between our browsers, maybe you'll get the correct behavior on this design also, and the problem is my browser. Then it's a headache to figure out why we are getting different behavior. But any design where you can replicate this bug is good to find.

@pranav-gupta1
Copy link
Contributor

I tried to replace this bug with a different design and when I have the "auto-fit on loading new design" box checked, if I zoom/pan to a random amount and then I reload the page, it is being auto-fitted to the default autofit size every time. That is what we want, correct? Please let me know as I feel like I may be misunderstanding something.

Then I guess we need to figure out why it's doing the correct thing on one design and not another. :)

I suspect it might be a difference between our browsers, maybe you'll get the correct behavior on this design also, and the problem is my browser. Then it's a headache to figure out why we are getting different behavior. But any design where you can replicate this bug is good to find.

Oh, okay. Yeah, it is probably the browser. I will find a design that can replicate the bug, and then move forward from there.

@dave-doty
Copy link
Member Author

dave-doty commented Aug 9, 2023

If you are seeing that it behaves correctly, then actually I'm a bit confused by that. I am trying to find where we would store the pan/zoom level in between page refreshes, and don't see it. It's not stored in the Redux state (so not written to localStorage as part of app_ui_state_storables) since the panning and zooming is handled by an external JS (non-React) library. You can see the JS code that sets it up in web/index.html around line 349 (with some other setup code before then).

I don't see any code in index.html that writes the pan/zoom level to localStorage so that it can be reloaded on page refresh. So I think this feature was actually never implemented. I'll reclassify the issue as an enhancement instead of a bug for that reason.

@dave-doty dave-doty added enhancement New feature or request and removed bug Something isn't working labels Aug 9, 2023
@pranav-gupta1
Copy link
Contributor

If you are seeing that it behaves correctly, then actually I'm a bit confused by that. I am trying to find where we would store the pan/zoom level in between page refreshes, and don't see it. It's not stored in the Redux state (so not written to localStorage as part of app_ui_state_storables) since the panning and zooming is handled by an external JS (non-React) library. You can see the JS code that sets it up in web/index.html around line 349 (with some other setup code before then).

I don't see any code in index.html that writes the pan/zoom level to localStorage so that it can be reloaded on page refresh. So I think this feature was actually never implemented. I'll reclassify the issue as an enhancement instead of a bug for that reason.

What I am seeing is that half of it behaves correctly. The part where it should auto-fit on page refreshes works, but the part where it should reset to something that is the previous zoom/pan level is not happening. I just looked through the web/index.html file and the set-up code near line 349, and it matches what you just clarified. Thanks for making this clearer.

@dave-doty
Copy link
Member Author

dave-doty commented Aug 9, 2023

So to be clear, I think the way to handle this is to set up a listener for when the pan/zoom changes (I think there already are such listeners set up at index.html called before_pan and before_zoom), then write the new pan or zoom to localStorage, associated to their own localStorage keys.

Name the localStorage keys scadnano:main_zoom, scadnano:main_pan, scadnano:side_zoom, scadnano:side_pan. Then when the page is first loaded, if app_state.app_ui_state.autofit is false, then use those values to set the initial pan/zoom. This can be done from within Dart by calling appropriate functions defined near the top of lib/src/util.dart (annotated with @JS() and the external keyword), which are JS functions defined in index.html that can be called from within Dart.

@pranav-gupta1
Copy link
Contributor

Ah, sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants