Shopify / hydrogen

Hydrogen lets you build faster headless storefronts in less time, on Shopify.
https://hydrogen.shop
MIT License
1.21k stars 243 forks source link

Add Pagination #755

Closed cartogram closed 1 year ago

cartogram commented 1 year ago

WHY are these changes introduced?

This PR brings a Pagination component into the skeleton template. I am looking for feedback on this approach and for someone to take this on while I am on vacation for the next few weeks.

WHAT is this pull request doing?

Here is a demo with infinite scrolling

<Pagination connection={products} autoLoadOnScroll />

https://user-images.githubusercontent.com/462077/229853820-c033a34f-c5ca-46e5-a805-9bba98b762d7.mov

Here is a demo with forward and back links

<Pagination connection={products} autoLoadOnScroll={false} />

https://user-images.githubusercontent.com/462077/229855186-de9b1c0e-1a02-4223-8402-75abea110235.mov

Some features this includes:

And simple UI might look like this:

      <Pagination connection={products} autoLoadOnScroll>
        {({
          endCursor,
          hasNextPage,
          hasPreviousPage,
          nextPageUrl,
          nodes,
          prevPageUrl,
          startCursor,
          nextLinkRef,
          isLoading,
        }) => {
          const itemsMarkup = nodes.map((product, i) => (
            <Link
              to={`/products/${product.handle}`}
              key={product.id}
            >
              {product.title}
            </Link>
          ));

          return (
            <>
              {hasPreviousPage && (
                <Link
                  preventScrollReset={true}
                  to={prevPageUrl}
                  prefetch="intent"
                  state={{
                    pageInfo: {
                      endCursor,
                      hasNextPage,
                      startCursor,
                      hasPreviousPage: undefined,
                    },
                    nodes,
                  }}
                >
                  {isLoading ? 'Loading...' : 'Previous'}
                </Link>
              )}
              {itemsMarkup}
              {hasNextPage && (
                <Link
                  preventScrollReset={true}
                  ref={nextLinkRef}
                  to={nextPageUrl}
                  prefetch="intent"
                  state={{
                    pageInfo: {
                      endCursor,
                      hasPreviousPage,
                      hasNextPage: undefined,
                      startCursor,
                    },
                    nodes,
                  }}
                >
                  {isLoading ? 'Loading...' : 'Next'}
                </Link>
              )}
            </>
          );
        }}
      </Pagination>

HOW to test your changes?

Run the skeleton template and visit /products.

Next steps

cartogram commented 1 year ago

I added a more docs-like RFC to the PR, copied below for easy-reading.


Pagination

Pagination is a complex component, that becomes even more complex for online storefronts. The goal of our Pagination component should be to take on the undifferentiated difficult parts of paginating a Storefront APO collection in Hydrogen projects. This includes:

Usage

Create route

Add a /products route is you don't already have one.

touch routes/products.tsx

Fetch a Storefront connection in the loader

Add a loader and query for the products in the shop. This is what a typical loader might look like without pagination applied.

export async function loader({context, request}: LoaderArgs) {
  const {products} = await context.storefront.query<{
    products: ProductConnection;
  }>(PRODUCTS_QUERY, {
    variables: {
      country: context.storefront.i18n?.country,
      language: context.storefront.i18n?.language,
    },
  });

  if (!products) {
    throw new Response(null, {status: 404});
  }

  return json({products});
}

And a sample query:

const PRODUCTS_QUERY = `#graphql
  query (
    $country: CountryCode
    $language: LanguageCode
  ) @inContext(country: $country, language: $language) {
    products() {
      nodes {
        id
        title
        publishedAt
        handle
        variants(first: 1) {
          nodes {
            id
            image {
              url
              altText
              width
              height
            }
          }
        }
      }
    }
  }
`;

Add the pagination variables to the query

First import and use a helper getPaginationVariables(request: Request) to build the pagination variables from the request object. We spread those values into the query, and also need to add those variables to the query along with the associated fragment.

+  import {getPaginationVariables, PAGINATION_PAGE_INFO_FRAGMENT} from '~/components';

export async function loader({context, request}: LoaderArgs) {
  const variables = getPaginationVariables(request, 4);
  const {products} = await context.storefront.query<{
    products: ProductConnection;
  }>(PRODUCTS_QUERY, {
    variables: {
+     ...variables,
      country: context.storefront.i18n?.country,
      language: context.storefront.i18n?.language,
    },
  });

  if (!products) {
    throw new Response(null, {status: 404});
  }

  return json({products});
}

And a add the fragment and variables to the query:

const PRODUCTS_QUERY = `#graphql
+ ${PAGINATION_PAGE_INFO_FRAGMENT}
  query (
    $country: CountryCode
    $language: LanguageCode
+   $first: Int
+   $last: Int
+   $startCursor: String
+   $endCursor: String
  ) @inContext(country: $country, language: $language) {
    products(
+     first: $first,
+     last: $last,
+     before: $startCursor,
+     after: $endCursor
    ) {
      nodes {
        id
        title
        publishedAt
        handle
        variants(first: 1) {
          nodes {
            id
            image {
              url
              altText
              width
              height
            }
          }
        }
      }
+     pageInfo {
+       ...PaginationPageInfoFragment
+     }
    }
  }
`;

Render the <Pagination /> component

In the default export, we can start to build our UI. This starts with rendering the <Pagination > component and passing the products loader data to the connection prop. The other prop this component takes is a boolean called autoLoadOnScroll that toggles infinite scrolling.

export default function Products() {
  const {products} = useLoaderData<typeof loader>();

  return (
    <>
      <Pagination connection={products} autoLoadOnScroll />
    </>
  );
}

Next we can expand the render prop to build our grid and navigation elements. We receive a number of helpful bits of information in the render prop that we can use to build the interface we want.

To enable the state-based cache, we pass the variables along to the Link component's state. This may be something we want to abstract away, but wanted to leave these guts-out for now.

export default function Products() {
  const {products} = useLoaderData<typeof loader>();

  return (
    <>
      <Pagination connection={products} autoLoadOnScroll>
        {({
          endCursor,
          hasNextPage,
          hasPreviousPage,
          nextPageUrl,
          nodes,
          prevPageUrl,
          startCursor,
          nextLinkRef,
          isLoading,
        }) => {
          const itemsMarkup = nodes.map((product, i) => (
            <Link to={`/products/${product.handle}`} key={product.id}>
              {product.title}
            </Link>
          ));

          return (
            <>
              {hasPreviousPage && (
                <Link
                  preventScrollReset={true}
                  to={prevPageUrl}
                  prefetch="intent"
                  state={{
                    pageInfo: {
                      endCursor,
                      hasNextPage,
                      startCursor,
                      hasPreviousPage: undefined,
                    },
                    nodes,
                  }}
                >
                  {isLoading ? 'Loading...' : 'Previous'}
                </Link>
              )}
              {itemsMarkup}
              {hasNextPage && (
                <Link
                  preventScrollReset={true}
                  ref={nextLinkRef}
                  to={nextPageUrl}
                  prefetch="intent"
                  state={{
                    pageInfo: {
                      endCursor,
                      hasPreviousPage,
                      hasNextPage: undefined,
                      startCursor,
                    },
                    nodes,
                  }}
                >
                  {isLoading ? 'Loading...' : 'Next'}
                </Link>
              )}
            </>
          );
        }}
      </Pagination>
    </>
  );
}

Conclusion

And that's it! You should now have a working pagination with all goals we outlined above.

benjaminsehl commented 1 year ago

An issue to consider as we move through build: https://github.com/Shopify/hydrogen/issues/596

wizardlyhel commented 1 year ago

I feel PAGINATION_PAGE_INFO_FRAGMENT is a not needed abstraction. It creates a disconnect between the graphql construction. I think it's fine if we just instruct the developers to make sure they have pageInfo connection in their products query. Personally, I find it not bringing much value. It is the difference between 5 lines vs 6 lines.

+ import {PAGINATION_PAGE_INFO_FRAGMENT} from '~/components';

const PRODUCTS_QUERY = `#graphql
+ ${PAGINATION_PAGE_INFO_FRAGMENT}
   query (... ) {
     products(... ) {
       ...
+     pageInfo {
+       ...PaginationPageInfoFragment
+     }
     }
   }
`;

vs

const PRODUCTS_QUERY = `#graphql
  query (... ) {
    products(... ) {
       ...
+     pageInfo {
+       hasPreviousPage
+       hasNextPage
+       startCursor
+       endCursor
+     }
    }
  }
`;

usePagination and getPaginationVariables are interesting combination but also exposes the cursor as raw URL. What changes do this function will need if there is a desire to represent cursors with just ?page=1 in the search param?

Example: I want to navigate between paginations like Dawn https://theme-dawn-demo.myshopify.com/collections/bags?page=2 and I also want to have infinite scroll.

Definitely +1 on the abstracting away the previous/next <Link>. Maybe pass the component back from the <Pagination> render prop?

Other questions:

blittle commented 1 year ago

I think this is a great proof of concept. A few thoughts and questions:

  1. autoLoadOnScroll isn't obvious as a prop that would enable forward and back links.
  2. It's unfortunate that forward and back doesn't work for auto loading
  3. The back and forth doesn't seem to completely work properly even with auto load on scroll disabled. I click through two the second page, scroll, select a product, then go back. And my scroll is not restored properly.
  4. Is there a reason search params are used, instead of the invisible URL state? It seems there might be conflicting requirements. I can't think of an inf-scroll implementation that persists in the URL. Like imagine how that would look copying and sharing a twitter URL where the scrolled state is preserved? At the same time, if you deliberately select page 2, perhaps that should be stored in the URL?
  5. It would be awesome if content unloaded from the dom when no longer visible to the page. I'm not sure the current implementation will scale to a page like skims that has thousands of products. Making elements remove from the dom when not in view would probably mean the API needs to drastically change, because each element would need a deterministic height.
wizardlyhel commented 1 year ago

Here is an example of semi infinite pagination implementation: https://www.fashionnova.com/pages/search-results/clothing?page=3

wizardlyhel commented 1 year ago

I actually quite like how Ikea does it https://www.ikea.com/ca/en/search/?q=table

Not sure how it is done but doing a page refresh does not lose the set of results you have already loaded - including if you click to a product and click back on the browser

but I don't like for it not having a ?page=3 indicator

frehner commented 1 year ago

I was playing around with this by changing the grid to only show one product per row, and ran into some weird jank when using the default settings; it appears to kick me back to the top when loading more products or something weird like that.


Thinking about this, I wonder what our desired experience is, before hydration / when JS is disabled. Would we ideally change the <button>s to <Link> instead, and have it be an actual next page? Seems like it would probably have to work that way if we wanted to go down that route, right?

But maybe that also works better with sharing a link: what if I wanted to share with you a product collection on page 3 of pagination? 🤷

cartogram commented 1 year ago

Thanks @lordofthecactus 🙏 Responses to your feedback below:

  1. How could someone modify the URL to have page=1, page=2 instead of the large cursor? I imagine merchants not fond of the large url .e.g fashionova

This is a limitation provided by the Storefront API which uses cursor-based pagination. We could potentially write code to map cursors to pages (which I will explore next), but it would be better to support this at the API layer.

  1. WIP maybe but I'm noticed some jankiness: https://screenshot.click/20-26-f2gby-8ilm2.mp4

This video and the one below are the same, is this intentional?

  1. Is going back and being in the same scroll position part of the implementation? It doesn't seem to be working https://screenshot.click/20-26-f2gby-8ilm2.mp4

It doesn't :( Will address this next.

  1. getPaginationVariables forces someone to use the request or url, could there be a reason a dev would want to not use a url for the pagination state?

I'm not sure I understand that use case, but re-routing this question to @benjaminsehl, is this a requirement?

  1. [nit] suggestion getPaginationVariables could have different signature
getPaginationVariables(request, { 
  first: 4
})

this would allow us to expand variables, or overwriting any output in case it is needed without breaking changes.

I like it! ❤️

  1. Maybe this is a limitation from storefront API. Is it possible to implement pagination like this?

Also forward to @benjaminsehl, but I think no and no we don't want to support that.

Give each page a unique URL. For example, include a ?page=n query parameter, as URLs in a paginated sequence are treated as separate pages by Google.

It does this (at least currently).

cartogram commented 1 year ago

Also, what do you think about separating this in 2 components? One with infinite scrolling and another without it. It might simplify the implementation (or not?) and allow tree-shaking for those who don't need react-intersection-observer.

@frandiox I didn't consider the tree-shaking aspect. @blittle perhaps we do a staggered release, makes even more sense if they are separate components. cc @benjaminsehl for your thoughts too.

benjaminsehl commented 1 year ago

cc @benjaminsehl for your thoughts too.

I think there's enough reason for us to not do infinite scroll at first and see how it lands. What do you think?

We could also provide an example of how to add infinite scroll manually.

Conceptually I'm not fond of providing two separate components.

github-actions[bot] commented 1 year ago

We detected some changes in packages/*/package.json or packages/*/src, and there are no updates in the .changeset. If the changes are user-facing and should cause a version bump, run npm run changeset add to track your changes and include them in the next release CHANGELOG. If you are making simple updates to examples or documentation, you do not need to add a changeset.

frehner commented 1 year ago

The deploy to oxygen preview isn't working because there's an issue resolving the storefront-api-types from hydrogen. I bet we could figure that out if you want to try