Skip to content
This repository has been archived by the owner on Dec 25, 2023. It is now read-only.

Scroll position should only be restored when navigating from photos back to their album #346

Closed
nicokaiser opened this issue Oct 26, 2022 · 9 comments

Comments

@nicokaiser
Copy link
Contributor

I see that remembering the scroll position is a good thing when navigating from a single photo view back to the album view.
However in other situations it is very distracted and does not make sense, especially if it is permanently saved in localStorage:

  1. Open a large album
  2. Scroll down
  3. Use the "back" link to open the album list
  4. Click on the same album again

Alternatively, open the same album via direct link.

I would expect to see the top of the album again, but instead, the bottom of the album is displayed, which is very confusing.

=> Scroll position should only be restored when navigating back from a photo to its album. Otherwise (direct album requests or navigation from parent albums) the position should be set to top.

I don't know if this is doable with the current architecture...

@nagmat84
Copy link
Collaborator

What you suggest is actually quite easy, because we don't need to remember the scroll position for that at all.

If you open the photo view, the album view is not closed, but remains open in the background. The photo view is implemented as an overlay with an opaque background which covers the album view, but it does not replace the album view. (Indeed, the black background of the photo view is transparent in "TV mode" such that you still see the album view around the big photo in the center.) Hence, we do not need to store the scroll position at all, the browser already does that for us.

The feature to remember and restore the scroll position has actually been added only for the exact purpose which you deem annoying. (Me, too, by the way.) But someone found that particular use case important enough to add this feature.

@nicokaiser
Copy link
Contributor Author

Thanks @nagmat84! Then maybe there should be a configuration option to be able to disable this behaviour. I already added this to Lychee-front but still need to figure out how to add it to the backend (maybe I make it an "advanced" configuration option which needs to be set using the "More" button)...

@kamil4
Copy link
Contributor

kamil4 commented Oct 26, 2022

Things become a bit more difficult with nested albums. True, we don't normally need to record the position of the immediate parent album; however, the position of that album's parent would be lost if one were to push the back button with in the subalbum (the parent album needs to get reloaded then).

In principle though, I agree with you and I think that it shouldn't be too hard to distinguish between clicking on an album vs using the back button. We already do that in the front end when deciding whether to autoplay videos (yes on direct click, no on previous/next).

Personally, I've wondered since this feature was first proposed whether the positions should really be stored as permanently as they are, but I guess I was swayed by the "what if somebody hits the reload" argument...

Another annoying feature of the current implementation is that it doesn't seem to handle changing window geometry correctly. I believe it records an absolute position in pixels, which causes problems e.g. when switching from portrait to landscape on a phone. We should probably record percentages instead...

@nicokaiser
Copy link
Contributor Author

I see that nested albums might be a use case for restoring scroll position – if (and only if) there are many albums/items on the "previous" page.

However if enabled, this should only trigger when navigating through the browser's prev/back buttons (like browsers implement this on static pages), and IMO not when using the "back" arrow in the title bar – and especially never ever when opening an album at a later time.

I would very much prefer the scroll position to never be restored when navigating nested albums, than sometimes seeing the album page scroll to (seemingly) random positions when opening them.

So my suggestion is to make this feature optional for now ("remember_scroll" config option, default=1 for backward compatibility), and fix it (to only affect browser prev/next navigation when enabled) later.

@d7415
Copy link
Contributor

d7415 commented Oct 27, 2022

and IMO not when using the "back" arrow in the title bar

I'm curious about this. I would generally expect a "back" button on a website to match the behaviour of the browser's back button.

@nicokaiser
Copy link
Contributor Author

I'm curious about this. I would generally expect a "back" button on a website to match the behaviour of the browser's back button.

This depends. If it's a real "back" button, then maybe. Otherwise if it's rather meant as "open parent level", then I would expect to see, well, the parent album (from the top).

@nagmat84
Copy link
Collaborator

and IMO not when using the "back" arrow in the title bar

I'm curious about this. I would generally expect a "back" button on a website to match the behaviour of the browser's back button.

Me, too.

I am also thinking ahead. After #343 will have been implemented, the frontend will properly use the History API and needs to intercept the browser back button to support client-side navigation based on the URL path component instead of the fragment. In order to keep things easy and clean, I would have expected that the intercepted back button of the browser and the integrated "back" button of the website call the same event handler. (Of course, that is nothing for the momentary architecture where most stuff is done in the god-like function lychee.load).

IMHO, a different behavior of the browser's back button and a "back" button on a website is not only super-counterintuitive, but also adds a different code path to the architecture which must be maintained separately.

@d7415
Copy link
Contributor

d7415 commented Oct 27, 2022

This depends. If it's a real "back" button, then maybe. Otherwise if it's rather meant as "open parent level", then I would expect to see, well, the parent album (from the top).

Yeah, I'd expect it to keep the position in that case too. I can't think of an advantage of starting at the top, but if I'm working my way through sub albums it would be really annoying to have to scroll to where I left off each time. IIRC even Windows does this.

@ildyria
Copy link
Member

ildyria commented Dec 25, 2023

Fixed.

@ildyria ildyria closed this as completed Dec 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants