Next.js: [Examples] Modal dynamic route doesn't keep scroll position

Created on 28 Sep 2020  路  14Comments  路  Source: vercel/next.js

Bug report

The example from https://github.com/vercel/next.js/tree/canary/examples/with-route-as-modal doesn't keep scroll position when modal is opened using the dynamic routing method.

Describe the bug

I'm not sure if the dynamic routing method is supposed to work like the QueryString routing where it keeps the scrolling position of the main page. I'm not able to use the QueryString method due to getStaticProps() not working as a component correctly (can't use mongodb node driver, it errors saying dns could not be found)

The scrolling position is saved when using the QueryString method after you close the modal. But for the dynamic routing method, it unloads the main page and only shows the modal (as seen in the gif below). When the modal is exited, the main page is reloaded again and the scroll position from the previous context is lost.

To Reproduce

  1. yarn create next-app --example with-route-as-modal with-route-as-modal-app
  2. cd with-route-as-modal-app
  3. yarn dev
  4. Go to localhost:3000 in Safari
  5. Click on one of the numbers under "With Dynamic Routing, and reloads will keep the modal"
  6. Exit out of the modal by clicking on the outer area of the screen.

Expected behavior

When exiting out of the modal, it should keep the scroll position of the main page.

Screenshots

2020-09-27 17 09 00

System information

  • OS: macOS 11.0 Beta (20A5364e)
  • Browser: Safari 14.0 (16610.2.2.4)
  • Version of Next.js: v9.5.3
  • Version of Node.js: v12.18.3
good first issue

Most helpful comment

@kyleawayan I threw up a repo where I've used a couple concepts that break with the original example; namely this is using layouts

Seems to be keeping the scroll as desired via scroll={false} on <Link /> when the page components via layouts are essentially all the same.

This was my hunch on how to achieve what you're looking for and I tried to be descriptive in the Headings to illustrate the differences in functionality for query string vs. dynamic routing.

Give it a look and let me know of any questions 馃憤 https://github.com/jmfury/nextjs-route-as-modal-plus-scroll

All 14 comments

Tried it on my Windows 10 PC on Firefox. Both of the QueryString routing and dynamic routing don't keep the scroll position. Although the QueryString routing does kinda keep the scrolling position when the window is really wide (like 1729 x 381), it kinda scrolls up a bit from the original scrolling context. Having it in a regular window size doesn't keep the scrolling position.

@kyleawayan I鈥檝e found reactjs/react-modal#117 which seems to indicate the scroll to top is a react-modal issue, so not specific to Next. On my machine (Firefox/Linux) I can see the scroll happening as soon as the modal opens.

@ludovicofischer Is this happening with both the QueryString and dynamic routing method? I also see that the main page scrolls to the top when the modal is opened in the QueryString method. But with the dynamic routing method, it unloads the whole main page.

@kyleawayan I see this happening just as you describe. This and the surrounding lines might be related to the issue https://github.com/vercel/next.js/blob/816798569a56c97108ecff37a85e6a3fd85648ab/packages/next/next-server/lib/router/router.ts#L428
I was quite surprised as I did not know about the scroll restoration API.
I don鈥檛 know if there is hope to restore scroll position in the QueryString case, as it could be hard for Next to distinguish a popu-triggered scroll from a user-initiated one. In the dynamic routing case, I am not sure about what鈥檚 going on.

@ludovicofischer Yeah I'm not sure if this worked in earlier versions of Next or the modal or something like that. I wish this would work because I have an infinite scrolling gallery with dynamic loading. It's kind of a nightmare to deal with especially with keeping scroll positions... so that's why I wanted to have a modal, especially with the dynamic route so I can keep my current page structure. I'll take a look into this when I have time to see if I can find a solution (maybe, I'm not sure how complicated this may be and I'm not that experienced馃槶).

You can disable scroll restoration using the scroll prop on next/link

File: components/Grid.js

import Link from 'next/link'
import styles from './styles.module.css'

export const data = [1, 2, 3, 4, 5, 6, 7, 8, 9]

export default function PostCardGrid() {
  return (
    <div className={styles.postCardGridWrapper}>
      <h2>With QueryString Routing, and a reload won't use the modal</h2>
      <div className={styles.postCardGrid}>
        {data.map((id, index) => (
          <Link key={index} href={`/?postId=${id}`} as={`/post/${id}`}>
            <a className={styles.postCard}>{id}</a>
          </Link>
        ))}
      </div>

      <h2>With Dynamic Routing, and reloads will keep the modal</h2>
      <div className={styles.postCardGrid}>
        {data.map((id, index) => (
          <Link
            key={index}
            href="/article/[articleId]"
            as={`/article/${id}`}
+           scroll={false}
          >
            <a className={styles.postCard}>{id}</a>
          </Link>
        ))}
      </div>
    </div>
  )
}

@HaNdTriX Unfortunately it didn't work for the dynamic routing as it still unloaded the page and didn't restore the scroll position... but it did work with the QueryString routing.

modal

@kyleawayan I threw up a repo where I've used a couple concepts that break with the original example; namely this is using layouts

Seems to be keeping the scroll as desired via scroll={false} on <Link /> when the page components via layouts are essentially all the same.

This was my hunch on how to achieve what you're looking for and I tried to be descriptive in the Headings to illustrate the differences in functionality for query string vs. dynamic routing.

Give it a look and let me know of any questions 馃憤 https://github.com/jmfury/nextjs-route-as-modal-plus-scroll

@jmfury Wow thank you so much for this! I cloned your repo and it works on my end! Now I just need to figure out how to incorporate it to my website. Although reloading will lose the scroll position, it's good enough as people looking through my gallery won't really need to reload. But you got the exact thing I was looking for!

@jmfury Wow thank you so much for this! I cloned your repo and it works on my end! Now I just need to figure out how to incorporate it to my website. Although reloading will lose the scroll position, it's good enough as people looking through my gallery won't really need to reload. But you got the exact thing I was looking for!

@kyleawayan Glad to help! I think the key here is eliminating the need for next (react on the client?) to unmount your page component which was the case in the original example. The pages were very different components in dynamic routing navigation.

Reloads are essentially a new server-render so I don't think keeping the scroll is typically desired anyway. It is potentially doable with next/router events... not positive, but I found this along the way as food for thought 馃敆

@jmfury Alright so before I begin to try to incorporate this into my website, I looked at the code more closely to understand how everything works. For my website, I need to be able to use getStaticProps() separately on each page, but in your example it looks like the dynamic routing goes to a component called Article.js. I can't use the mongodb driver in a component because the component is only front-end or something like that. I made a drawing on how I understand how its working:

image

I see that there's a page called [articleId].js, which has getStaticProps and getStaticPaths, but it looks like this is only for when a user refreshes the page.

Is this the correct flow of the example? Or is using getStaticProps() from Article.js possible? Or would I need to pass props to the component?

@jmfury Alright so before I begin to try to incorporate this into my website, I looked at the code more closely to understand how everything works. For my website, I need to be able to use getStaticProps() separately on each page, but in your example it looks like the dynamic routing goes to a component called Article.js. I can't use the mongodb driver in a component because the component is only front-end or something like that. I made a drawing on how I understand how its working:

image

I see that there's a page called [articleId].js, which has getStaticProps and getStaticPaths, but it looks like this is only for when a user refreshes the page.

Is this the correct flow of the example? Or is using getStaticProps() from Article.js possible? Or would I need to pass props to the component?

@jmfury Nevermind, I understood the flow wrong. I tried changing the getStaticProps() in [articleId].js to

export function getStaticProps({ params: { articleId } }) {
  return { props: { articleId: "sdfdskfhsdkljh" } };
}

and it reflected on all the pages!

@kyleawayan Great to hear it's working! Feel free to tag me on any code you can share outside this thread as needed. Happy to assist if any of the concepts are giving you any trouble in your implementation!

@jmfury Thank you! I have one problem, I'm going to try to explain what I have right now... but first let me explain how my website works.

If you go to my website, there is an initial gallery that the user sees. They then can click any photo, and then it will go to like a "sub-gallery" you could say.

My home page gallery (like your Grid component in your example) uses getStaticProps() to get photos for the home page. I then pass that on to my HomeGallery layout which it is then rendered.

But when I go to a "sub-gallery," the layout can't load the initial getStaticProps() which was passed on from the home page gallery since it's not on that page (index.js) anymore.

I'm not sure if your example reloads the whole page, or it's because I'm using getStaticProps(), everything is getting messed up. FYI, I'm using router.push as I can't really map <Link>s to each photo in react-photo-gallery. I even tried making a dummy link with scroll={false} just to try it but it's still resulting in the same issue.

Do you recommend any other ways to circumvent this?

Was this page helpful?
0 / 5 - 0 ratings