Feature Idea: Restore overflowing div's scroll position on popstate about scroll-plugin HOT 14 CLOSED

Makes sense! I haven't tried the plugin myself. In that case, I like your suggestion of implementing it for the scroll plugin and then deciding if it feels right to include. Let's do that!

from scroll-plugin.

The indentation is working fine in VSCode for me, must be some setting I've forgotten about long ago 🤠

from scroll-plugin.

But yes, can you create an issue for the .editorconfig? I'll see if I can cobble it together from the prettier config.

from scroll-plugin.

There's a third-party Preserve Scroll Plugin that's pretty close in spirit. That might be a good fit for that functionality; not sure how much of it it already covers. I worry a bit that supporting arbitrary numbers of other scroll containers will end up making the plugin rather complex. Not sure how @gmrchk thinks about this?

from scroll-plugin.

Oops, didn't mean to close.

from scroll-plugin.

Thanks for the pointer to Preserve Scroll Plugin! Yes, at first I thought this plugin would exactly solve my needs – until I found out that it doesn't... 🤖 ...adding my proposed feature to said plugin would be pretty confusing for users, I imagine, since the two features sound so similar, but are so different.

With PreserveScrollPlugin out of the race, I see three alternatives:

1. Add the functionality to ScrollPlugin (my favorite. Based on the plugin's name I would expect it to handle all scroll-related stuff)
2. Create a new plugin called "RestoreScrollPlugin" or "CacheScrollPlugin"
   • con: could easily be confused with "PreserveScrollPlugin" or "ScrollPlugin")
   • con: one more plugin
3. Incorporate this into swup's core

We should probably think about also taking care of restoring the scroll position on the window if animateHistoryBrowsing is set to true, as described here

I am not too worried about the complexity. Maybe I could just try to implement it in scroll-plugin and then we could decide together if it became too complex?

from scroll-plugin.

I've never used animateHistoryBrowsing — does that mean the scroll position isn't restored on back/forward? In that case, we should definitely have this on the radar.

from scroll-plugin.

I haven't and probably neither will ever use animateHistoryBrowsing, either. But that's what it says in the docs. The scroll position isn't being restored automatically. @gmrchk , maybe you could explain to us, what your reasoning was behind leaving the implementation of the scroll restoration to the end users of swup?

from scroll-plugin.

I implemented the described logic for automatically storing and restoring the scroll positions. Looking forward to hear what you think! :)

One thing I noticed: My editor (VS Code) keeps messing up the indentation of your source code. Do you have an .editorconfig that you would care to add to the repo? I have also been trying to let npx prettier --write . do it's thing, but it doesn't correctly indent the code, either 🤷‍♂️

from scroll-plugin.

I basically couldn't tell what's the best way for handling such scenario, since I wouldn't use it myself, so I didn't implement it at all.
animateHistoryBrowsing is this things that was widely requested, but I kinda regret. 😄
I used Swup to animate transitions, but I always felt that once user goes back in history, they want to get exactly where they were quickly. That's why the transitions are by default disabled for history jumps. This is also handy because the scroll position is automatically handled by browser just like it would without Swup.
However, many people requested to have option to animate transitions on back/forward navigation also, so the option was added. The problem with that is that you likely don't want the browser to simply (natively) jump to the previous position right away during animation. We suddenly need to handle possibly complex logic of when and where to scroll - Should it scroll right away? Should it go all the way to the top, or previous cached position? Should it animate the scroll? That's why I opted to not handle it at all, and let user implement their own behaviour on Swup events, possibly using Swups scroll animation.

from scroll-plugin.

For the feature idea - I like the opt-in nature of this and Scroll plugin sounds as a good place. I think it could handle all things scrolling - no need for granural plugins IMO. Some things to consider maybe:

• Is this something to differentiate per element?
• Are there some different options to apply per element too?

Just thinking that we could have multiple ScrollPlugin instances allowed, with target being window by default, but configurable, or something in that sense. So the options for restoration and animation and everything could be set granually for main window, but also other elements, with all the functionality being configurable. Only having [data-swup-restore-scroll] is very limited as API, although it's enough for such use case maybe.

from scroll-plugin.

@gmrchk thanks for your explanations on animateHistoryBrowsing. I completely agree with your reasoning against using it. But since it's now part of Swup, we will have to deal with it, one way or the other 💁‍♂️ Maybe we could add a note to the docs that we recommend to not use the option, or use at own risk or the like?

Following your logic with not touching the scroll position at all if animateHistoryBrowsing is set to true, we could maybe do the same in scroll-plugin? I think this would probably be the most consistent approach, at least for the window's scroll position.

Regarding your feedback on the feature idea:

• Is this something to differentiate per element?

I can't think of any reason why this should be the case. Basically this feature would just restore the previous scroll positions for all elements that match [data-swup-scroll-container] (Note, I changed the attribute name).

• Are there some different options to apply per element too?

In the current PR the scroll positions aren't being animated. They just snap back to their previous position. That's how I always needed it in the websites that made use of overflowing divs. And since there are no animation options, we wouldn't need different animation options per element, either :)

Just thinking that we could have multiple ScrollPlugin instances allowed, with target being window by default, but configurable, or something in that sense. So the options for restoration and animation and everything could be set granually for main window, but also other elements, with all the functionality being configurable. Only having [data-swup-restore-scroll] is very limited as API, although it's enough for such use case maybe.

Interesting idea – but it feels a bit like overkill to me. Also, users could get confused and bind multiple ScrollPlugin instances to the same target element (for example, two instances trying to control the window scroll position and getting in a fight with each other). Another argument for the simple approach in my PR is that these overflowing divs wouldn't need to exist throughout a website. They could be defined on each page independently.

from scroll-plugin.

So, obviously this was too much text ;)

Should we maybe introduce a new label pending triage? Vite does this, for example.

Some things are just better discussed verbally, if they exceed a certain complexity.

from scroll-plugin.

Added in #26

from scroll-plugin.