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

UC-Davis-molecular-computing / scadnano

Web application for designing DNA structures such as DNA origami.

https://scadnano.org

MIT License

21 stars 13 forks source link

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

Open dave-doty opened 11 months ago

dave-doty commented 11 months ago

With this option:

If you are at a particular zoom/pan level:

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

pranav-gupta1 commented 11 months ago

With this option:

If you are at a particular zoom/pan level:

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

@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 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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.

pranav-gupta1 commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

Ah, sounds good.