gthole / drink-stash

A simple web app for saving and searching cocktail recipes.
MIT License
10 stars 1 forks source link

Keep my position in the scroll view when i tap into a drink someone is trying and navigate back #57

Closed kateswanson-toast closed 3 years ago

kateswanson-toast commented 4 years ago

when you are scrolling through the feed of recent people who made what and you tap one then go back it scrolls you back to the top .. i use the feed a lot so it would be nice to not have to find my place again!

you guys rock by the way!!

gthole commented 4 years ago

Great suggestion. This will probably require some refactoring since we load resources from the API with each page view, so the previous scroll position is not available until after the state update following the data hook. I'll keep thinking about it and see what I can come up with.

kateswanson-toast commented 4 years ago

You rock! Honestly the scroll thing would probably save me like 10 minutes a day. This app is life-changing by the way - mad kudos. Eventually I'll be a better front end developer and try to help

gthole commented 4 years ago

Definitely working on this, and have some decent ideas, but nothing final yet.

Currently my best plan is to let the browser handle it with history.scrollRestoration, but this will mean tracking the last body height of each path after render in the application context, with hooks for the view component to update the context after render. (That's not always easy, since sometimes the browser height is adjusted down in sub-components, like the activity list.)

So, I'll keep thinking about it. Definitely want to save you the 10 minutes a day (plus reduce frustration when making a drink. Nobody wants that.)

gthole commented 4 years ago

Deployed! Here's the approach I used, which needs some adjustments made to it, but this works for now.

kateswanson-toast commented 4 years ago

You rock Greg!!

On Thu, Oct 22, 2020 at 1:13 PM Greg Thole notifications@github.com wrote:

Closed #57 https://github.com/gthole/drink-stash/issues/57.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/gthole/drink-stash/issues/57#event-3910232192, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANWBWOIQL44PITAGPTCO7UTSMBR3RANCNFSM4SPYINTA .

gthole commented 3 years ago

This has been re-reported.

gthole commented 3 years ago

Re-fixed. Still can't say I'm happy with the race condition on the render/page-height store. Leaving a note for myself on the technical details here.

Alternate approach: make a useScrollMemory hook that stores the page height to the cache based on a state dependency (e.g the resp for RecipeList component, and activity for the ActivityDetails component) and does not set the cache if the dependency is false-y. This makes the cache setting process properly a post-render hook and de-couples it from the useAlertedEffect service hook. But in the RecipeDetails view there are multiple such data loads and it doesn't seem to be restoring the correct page height.

The on-render ScrollMemory component also has some issues - probably need to switch to using a useLayoutEffect with no dependencies to register the onpopstate event handler.

If all else fails, there's always this polyfill which would be less code to maintain. I can't say that I love it either, but I wouldn't have to think about it either. Just drop and go.