climbjios-sg / climbjios-app

The community for climbers 🧗
https://www.climbjios.com/
GNU General Public License v3.0
26 stars 4 forks source link

Disable scroll to top when clicking back button on a Jio Card #220

Open canny[bot] opened 1 year ago

canny[bot] commented 1 year ago

https://climbjios.canny.io/admin/board/feedback/p/disable-scroll-to-top-when-clicking-back-button-on-a-jio-card

canny[bot] commented 1 year ago

This issue has been linked to a Canny post: Disable scroll to top when clicking back button on a Jio Card :tada:

Artemis-Hunt commented 1 year ago

Also, don't clear search filters if the user enter the Jio Card with search filters active and click back button

wgao19 commented 1 year ago

Hey @therizhao I'm seeing a hardcoded logic to always scroll to top on every page change, may I ask what was the intention for it?

therizhao commented 1 year ago

Hello @wgao19 ! Thanks for getting started! Scroll to top should be there for certain page navigation. E.g. Going from Betas tab to Jios tab.

But it shouldn't be for all. E.g. Going to user's profile and clicking the back button.

Currently well don't have a granular control for this and we default to scroll to top for all pages.

This issue should fix this behavior. Perhaps by having some way of controlling if user should scroll to top on navigation. And also using the prop to decide on scroll behavior for each scenario.

nathantew14 commented 1 year ago

Scroll to top should be there for certain page navigation. E.g. Going from Betas tab to Jios tab.

@therizhao IMO I'd rather my scroll position be retained if I swap to different tabs, etc. if I was watching a beta video I found after scrolling a while, I wouldn't want the page to refresh and scroll to the top if I accidentally switch tabs.

nathantew14 commented 1 year ago
useEffect(() => {
    if (!jioSearchValues) {
      dispatch(listJios({}));
      return;
    }

    const searchParams = {} as GetJioListRequest;
    if (jioSearchValues.date) {
      searchParams.startDateTime = getDateTimeString(jioSearchValues.date, '00:00');
      searchParams.endDateTime = getDateTimeString(jioSearchValues.date, '23:59');
    }

    if (jioSearchValues.gymId) {
      searchParams.gymId = jioSearchValues.gymId;
    }

    if (jioSearchValues.type) {
      searchParams.type = jioSearchValues.type;
    }

    dispatch(listJios(searchParams));
  }, [version, dispatch, jioSearchValues]);

for now, since all the dependencies of useEffect start off as undefined when the page is rendered, it forces listJios action to be dispatched, meaning the list is necessarily refreshed (at least) once whenever you leave the list (either by switching tabs or opening a climber's profile).

nathantew14 commented 1 year ago

for now, since all the dependencies of useEffect start off as undefined when the page is rendered, it forces listJios action to be dispatched, meaning the list is necessarily refreshed (at least) once whenever you leave the list (either by switching tabs or opening a climber's profile).

can use useRef to prevent fetching a new version upon initial render: https://stackoverflow.com/questions/53253940/make-react-useeffect-hook-not-run-on-initial-render

but still need to retain scroll position.