Use pagination instead of infinite scrolling

idk this has been a long-time popular request, so I'm just gonna like. throw it all the way out there. and see what people think of it

I'm a bit worried it might change up the mobile experience too much? But like. let's find out!

src/app/WardrobePage/Item.js

@@ -303,17 +303,21 @@ function LinkOrButton({ href, component = Button, ...props }) {
  * ItemListContainer is a container for Item components! Wrap your Item
  * components in this to ensure a consistent list layout.
  */
-export function ItemListContainer({ children }) {
+export function ItemListContainer({ children, ...props }) {
-  return <Flex direction="column">{children}</Flex>;
+  return (
+    <Flex direction="column" {...props}>
+      {children}
+    </Flex>
+  );
 }
 
 /**
  * ItemListSkeleton is a placeholder for when an ItemListContainer and its
  * Items are loading.
  */
-export function ItemListSkeleton({ count }) {
+export function ItemListSkeleton({ count, ...props }) {
   return (
-    <ItemListContainer>
+    <ItemListContainer {...props}>
       {Array.from({ length: count }).map((_, i) => (
         <ItemSkeleton key={i} />
       ))}

src/app/WardrobePage/ItemsAndSearchPanels.js

@@ -38,7 +38,7 @@ function ItemsAndSearchPanels({
     <Sentry.ErrorBoundary fallback={MajorErrorMessage}>
       <TestErrorSender />
       <Flex direction="column" height="100%">
-        <Box px="5" py="3" boxShadow="sm">
+        <Box paddingX="5" paddingTop="3" paddingBottom="2" boxShadow="sm">
           <SearchToolbar
             query={searchQuery}
             searchQueryRef={searchQueryRef}
@@ -49,30 +49,23 @@ function ItemsAndSearchPanels({
         {!searchQueryIsEmpty(searchQuery) ? (
           <Box
             key="search-panel"
-            gridArea="items"
+            flex="1 0 0"
             position="relative"
-            overflow="auto"
+            overflowY="scroll"
             ref={scrollContainerRef}
             data-test-id="search-panel-scroll-container"
           >
-            <Box px="4" py="2">
+            <SearchPanel
-              <SearchPanel
+              query={searchQuery}
-                query={searchQuery}
+              outfitState={outfitState}
-                outfitState={outfitState}
+              dispatchToOutfit={dispatchToOutfit}
-                dispatchToOutfit={dispatchToOutfit}
+              scrollContainerRef={scrollContainerRef}
-                scrollContainerRef={scrollContainerRef}
+              searchQueryRef={searchQueryRef}
-                searchQueryRef={searchQueryRef}
+              firstSearchResultRef={firstSearchResultRef}
-                firstSearchResultRef={firstSearchResultRef}
+            />
-              />
-            </Box>
           </Box>
         ) : (
-          <Box
+          <Box position="relative" overflow="auto" key="items-panel">
-            gridArea="items"
-            position="relative"
-            overflow="auto"
-            key="items-panel"
-          >
             <Box px="4" py="2">
               <ItemsPanel
                 loading={loading}

src/app/WardrobePage/SearchPanel.js

@@ -1,12 +1,15 @@
 import React from "react";
 import gql from "graphql-tag";
-import { Box, Text, VisuallyHidden } from "@chakra-ui/react";
+import { Box, Text, useColorModeValue, VisuallyHidden } from "@chakra-ui/react";
 import { useQuery } from "@apollo/client";
 
-import { Delay, useDebounce } from "../util";
+import { useDebounce } from "../util";
 import { emptySearchQuery } from "./SearchToolbar";
 import Item, { ItemListContainer, ItemListSkeleton } from "./Item";
 import { itemAppearanceFragment } from "../components/useOutfitAppearance";
+import PaginationToolbar from "../components/PaginationToolbar";
+
+const SEARCH_PER_PAGE = 30;
 
 /**
  * SearchPanel shows item search results to the user, so they can preview them
@@ -50,19 +53,15 @@ function SearchPanel({
     >
       <SearchResults
         // When the query changes, replace the SearchResults component with a
-        // new instance, to reset `itemIdsToReconsider`. That way, if you find
+        // new instance. This resets both `currentPageNumber`, to take us back
-        // an item you like in one search, then immediately do a second search
+        // to page 1; and also `itemIdsToReconsider`. That way, if you find an
-        // and try a conflicting item, we'll restore the item you liked from
+        // item you like in one search, then immediately do a second search and
-        // your first search!
+        // try a conflicting item, we'll restore the item you liked from your
-        //
+        // first search!
-        // NOTE: I wonder how this affects things like state. This component
-        //       also tries to gracefully handle changes in the query, but tbh
-        //       I wonder whether that's still necessary...
         key={serializeQuery(query)}
         query={query}
         outfitState={outfitState}
         dispatchToOutfit={dispatchToOutfit}
-        scrollContainerRef={scrollContainerRef}
         firstSearchResultRef={firstSearchResultRef}
         onMoveFocusUpToQuery={onMoveFocusUpToQuery}
       />
@@ -82,15 +81,15 @@ function SearchResults({
   query,
   outfitState,
   dispatchToOutfit,
-  scrollContainerRef,
   firstSearchResultRef,
   onMoveFocusUpToQuery,
 }) {
-  const { loading, loadingMore, error, items, fetchMore } = useSearchResults(
+  const [currentPageNumber, setCurrentPageNumber] = React.useState(1);
+  const { loading, error, items, numTotalPages } = useSearchResults(
     query,
-    outfitState
+    outfitState,
+    currentPageNumber
   );
-  useScrollTracker(scrollContainerRef, 300, fetchMore);
 
   // This will save the `wornItemIds` when the SearchResults first mounts, and
   // keep it saved even after the outfit changes. We use this to try to restore
@@ -123,14 +122,10 @@ function SearchResults({
     }
   }, []);
 
+  const searchPanelBackground = useColorModeValue("white", "gray.900");
+
   // If the results aren't ready, we have some special case UI!
-  if (loading) {
+  if (error) {
-    return (
-      <Delay ms={500}>
-        <ItemListSkeleton count={8} />
-      </Delay>
-    );
-  } else if (error) {
     return (
       <Text>
         We hit an error trying to load your search results{" "}
@@ -140,24 +135,30 @@ function SearchResults({
         Try again?
       </Text>
     );
-  } else if (items.length === 0) {
-    return (
-      <Text>
-        We couldn't find any matching items{" "}
-        <span role="img" aria-label="(thinking emoji)">
-          🤔
-        </span>{" "}
-        Try again?
-      </Text>
-    );
   }
 
   // Finally, render the item list, with checkboxes and Item components!
   // We also render some extra skeleton items at the bottom during infinite
   // scroll loading.
   return (
-    <>
+    <Box>
-      <ItemListContainer>
+      <Box
+        position="sticky"
+        top="0"
+        background={searchPanelBackground}
+        zIndex="2"
+        paddingX="5"
+        paddingBottom="2"
+        paddingTop="1"
+      >
+        <PaginationToolbar
+          numTotalPages={numTotalPages}
+          currentPageNumber={currentPageNumber}
+          goToPageNumber={setCurrentPageNumber}
+          buildPageUrl={() => null}
+        />
+      </Box>
+      <ItemListContainer paddingX="4" paddingBottom="2">
         {items.map((item, index) => (
           <SearchResultItem
             key={item.id}
@@ -172,8 +173,23 @@ function SearchResults({
           />
         ))}
       </ItemListContainer>
-      {loadingMore && <ItemListSkeleton count={8} />}
+      {loading && (
-    </>
+        <ItemListSkeleton
+          count={SEARCH_PER_PAGE}
+          paddingX="4"
+          paddingBottom="2"
+        />
+      )}
+      {!loading && items.length === 0 && (
+        <Text paddingX="4">
+          We couldn't find any matching items{" "}
+          <span role="img" aria-label="(thinking emoji)">
+            🤔
+          </span>{" "}
+          Try again?
+        </Text>
+      )}
+    </Box>
   );
 }
 
@@ -245,9 +261,13 @@ function SearchResultItem({
  * useSearchResults manages the actual querying and state management of search!
  * It's hefty, infinite-scroll pagination is a bit of a thing!
  */
-function useSearchResults(query, outfitState) {
+function useSearchResults(
+  query,
+  outfitState,
+  currentPageNumber,
+  { skip = false } = {}
+) {
   const { speciesId, colorId } = outfitState;
-  const [isEndOfResults, setIsEndOfResults] = React.useState(false);
 
   // We debounce the search query, so that we don't resend a new query whenever
   // the user types anything.
@@ -256,12 +276,6 @@ function useSearchResults(query, outfitState) {
     initialValue: emptySearchQuery,
   });
 
-  // When the query changes, we should update our impression of whether we've
-  // reached the end!
-  React.useEffect(() => {
-    setIsEndOfResults(false);
-  }, [query]);
-
   // NOTE: This query should always load ~instantly, from the client cache.
   const { data: zoneData } = useQuery(gql`
     query SearchPanelZones {
@@ -277,13 +291,11 @@ function useSearchResults(query, outfitState) {
     : [];
   const filterToZoneIds = filterToZones.map((z) => z.id);
 
+  const currentPageIndex = currentPageNumber - 1;
+  const offset = currentPageIndex * SEARCH_PER_PAGE;
+
   // Here's the actual GQL query! At the bottom we have more config than usual!
-  const {
+  const { loading: loadingGQL, error, data } = useQuery(
-    loading: loadingGQL,
-    error,
-    data,
-    fetchMore: fetchMoreGQL,
-  } = useQuery(
     gql`
       query SearchPanel(
         $query: String!
@@ -294,6 +306,7 @@ function useSearchResults(query, outfitState) {
         $speciesId: ID!
         $colorId: ID!
         $offset: Int!
+        $perPage: Int!
       ) {
         itemSearch: itemSearchV2(
           query: $query
@@ -302,11 +315,8 @@ function useSearchResults(query, outfitState) {
           currentUserOwnsOrWants: $currentUserOwnsOrWants
           zoneIds: $zoneIds
         ) {
-          query
+          numTotalItems
-          zones {
+          items(offset: $offset, limit: $perPage) {
-            id
-          }
-          items(offset: $offset, limit: 50) {
             # TODO: De-dupe this from useOutfitState?
             id
             name
@@ -346,139 +356,32 @@ function useSearchResults(query, outfitState) {
         itemKind: debouncedQuery.filterToItemKind,
         currentUserOwnsOrWants: debouncedQuery.filterToCurrentUserOwnsOrWants,
         zoneIds: filterToZoneIds,
-        offset: 0,
         speciesId,
         colorId,
+        offset,
+        perPage: SEARCH_PER_PAGE,
       },
       context: { sendAuth: true },
       skip:
-        !debouncedQuery.value &&
+        skip ||
-        !debouncedQuery.filterToItemKind &&
+        (!debouncedQuery.value &&
-        !debouncedQuery.filterToZoneLabel &&
+          !debouncedQuery.filterToItemKind &&
-        !debouncedQuery.filterToCurrentUserOwnsOrWants,
+          !debouncedQuery.filterToZoneLabel &&
-      notifyOnNetworkStatusChange: true,
+          !debouncedQuery.filterToCurrentUserOwnsOrWants),
-      onCompleted: (d) => {
-        // This is called each time the query completes, including on
-        // `fetchMore`, with the extended results. But, on the first time, this
-        // logic can tell us whether we're at the end of the list, by counting
-        // whether there was <30. We also have to check in `fetchMore`!
-        const items = d && d.itemSearch && d.itemSearch.items;
-        if (items && items.length < 30) {
-          setIsEndOfResults(true);
-        }
-      },
       onError: (e) => {
         console.error("Error loading search results", e);
       },
+      // Return `numTotalItems` from the GQL cache while waiting for next page!
+      returnPartialData: true,
     }
   );
 
-  // Smooth over the data a bit, so that we can use key fields with confidence!
+  const loading = debouncedQuery !== query || loadingGQL;
-  const result = data?.itemSearch;
+  const items = data?.itemSearch?.items ?? [];
-  const resultValue = result?.query;
+  const numTotalItems = data?.itemSearch?.numTotalItems ?? null;
-  const zoneStr = [...filterToZoneIds].sort().join(",");
+  const numTotalPages = Math.ceil(numTotalItems / SEARCH_PER_PAGE);
-  const resultZoneStr = (result?.zones || [])
-    .map((z) => z.id)
-    .sort()
-    .join(",");
-  const queriesMatch = resultValue === query.value && resultZoneStr === zoneStr;
-  const items = result?.items || [];
 
-  // Okay, what kind of loading state is this?
+  return { loading, error, items, numTotalPages };
-  let loading;
-  let loadingMore;
-  if (loadingGQL && items.length > 0 && queriesMatch) {
-    // If we already have items for this query, but we're also loading GQL,
-    // then we're `loadingMore`.
-    loading = false;
-    loadingMore = true;
-  } else if (loadingGQL || query !== debouncedQuery) {
-    // Otherwise, if we're loading GQL or the user has changed the query, we're
-    // just `loading`.
-    loading = true;
-    loadingMore = false;
-  } else {
-    // Otherwise, we're not loading at all!
-    loading = false;
-    loadingMore = false;
-  }
-
-  // When SearchResults calls this, we'll resend the query, with the `offset`
-  // increased. We'll append the results to the original query!
-  const fetchMore = React.useCallback(() => {
-    if (!loadingGQL && !error && !isEndOfResults) {
-      fetchMoreGQL({
-        variables: {
-          offset: items.length,
-        },
-        updateQuery: (prev, { fetchMoreResult }) => {
-          // Note: This is a bit awkward because, if the results count ends on
-          // a multiple of 30, the user will see a flash of loading before
-          // getting told it's actually the end. Ah well :/
-          //
-          // We could maybe make this more rigorous later with
-          // react-virtualized to have a better scrollbar anyway, and then
-          // we'd need to return the total result count... a bit annoying to
-          // potentially double the query runtime? We'd need to see how slow it
-          // actually makes things.
-          if (fetchMoreResult.itemSearch.items.length < 30) {
-            setIsEndOfResults(true);
-          }
-
-          return {
-            ...prev,
-            itemSearch: {
-              ...(prev?.itemSearch || {}),
-              items: [
-                ...(prev?.itemSearch?.items || []),
-                ...(fetchMoreResult?.itemSearch?.items || []),
-              ],
-            },
-          };
-        },
-      }).catch((e) => {
-        console.error("Error loading more search results pages", e);
-      });
-    }
-  }, [loadingGQL, error, isEndOfResults, fetchMoreGQL, items.length]);
-
-  return { loading, loadingMore, error, items, fetchMore };
-}
-
-/**
- * useScrollTracker watches for the given scroll container to scroll near the
- * bottom, then fires a callback. We use this to fetch more search results!
- */
-function useScrollTracker(scrollContainerRef, threshold, onScrolledToBottom) {
-  const onScroll = React.useCallback(
-    (e) => {
-      const topEdgeScrollPosition = e.target.scrollTop;
-      const bottomEdgeScrollPosition =
-        topEdgeScrollPosition + e.target.clientHeight;
-      const remainingScrollDistance =
-        e.target.scrollHeight - bottomEdgeScrollPosition;
-      if (remainingScrollDistance < threshold) {
-        onScrolledToBottom();
-      }
-    },
-    [onScrolledToBottom, threshold]
-  );
-
-  React.useLayoutEffect(() => {
-    const scrollContainer = scrollContainerRef.current;
-
-    if (!scrollContainer) {
-      return;
-    }
-
-    scrollContainer.addEventListener("scroll", onScroll);
-
-    return () => {
-      if (scrollContainer) {
-        scrollContainer.removeEventListener("scroll", onScroll);
-      }
-    };
-  }, [onScroll, scrollContainerRef]);
 }
 
 /**

src/app/components/PaginationToolbar.js

@@ -22,6 +22,11 @@ function PaginationToolbar({
     <Flex align="center" justify="space-between" {...props}>
       <LinkOrButton
         href={prevPageUrl}
+        onClick={
+          prevPageUrl == null
+            ? () => goToPageNumber(currentPageNumber - 1)
+            : undefined
+        }
         _disabled={{
           cursor: isLoading ? "wait" : "not-allowed",
           opacity: 0.4,
@@ -30,7 +35,7 @@ function PaginationToolbar({
       >
         ← Prev
       </LinkOrButton>
-      {numTotalPages && (
+      {numTotalPages > 0 && (
         <Flex align="center">
           <Box flex="0 0 auto">Page</Box>
           <Box width="1" />
@@ -46,6 +51,11 @@ function PaginationToolbar({
       )}
       <LinkOrButton
         href={nextPageUrl}
+        onClick={
+          nextPageUrl == null
+            ? () => goToPageNumber(currentPageNumber + 1)
+            : undefined
+        }
         _disabled={{
           cursor: isLoading ? "wait" : "not-allowed",
           opacity: 0.4,