fix(ui): pagination resets perPage (#10199)

When using various controls within the List View, those selections are sometimes not persisted. This is especially evident when selecting `perPage` from the List View, where the URL and UI would reflect this selection, but the controls would be stale. Similarly, after changing `perPage` then navigating to another page through the pagination controls, `perPage` would reset back to the original value. Same with the sort controls, where sorting by a particular column would not be reflected in the UI. This was because although we modify the URL search params and fire off a new query with those changes, we were not updating local component state.
payloadcms · Dec 27, 2024 · fad4ee6 · fad4ee6
1 parent 6b4842d
commit fad4ee6
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 86 deletions.
diff --git a/packages/ui/src/providers/ListQuery/index.tsx b/packages/ui/src/providers/ListQuery/index.tsx
@@ -63,14 +63,17 @@ export const ListQueryProvider: React.FC<ListQueryProps> = ({
 
   const { onQueryChange } = useListDrawerContext()
 
-  const [currentQuery, setCurrentQuery] = useState(() => {
+  const [currentQuery, setCurrentQuery] = useState<ListQuery>(() => {
     if (modifySearchParams) {
       return searchParams
     } else {
       return {}
     }
   })
 
+  const currentQueryRef = React.useRef(currentQuery)
+
+  // If the search params change externally, update the current query
   useEffect(() => {
     if (modifySearchParams) {
       setCurrentQuery(searchParams)
@@ -79,10 +82,10 @@ export const ListQueryProvider: React.FC<ListQueryProps> = ({
 
   const refineListData = useCallback(
     async (query: ListQuery) => {
-      let pageQuery = 'page' in query ? query.page : currentQuery?.page
+      let page = 'page' in query ? query.page : currentQuery?.page
 
       if ('where' in query || 'search' in query) {
-        pageQuery = '1'
+        page = '1'
       }
 
       const updatedPreferences: Record<string, unknown> = {}
@@ -103,14 +106,11 @@ export const ListQueryProvider: React.FC<ListQueryProps> = ({
       }
 
       const newQuery: ListQuery = {
-        limit:
-          'limit' in query
-            ? query.limit
-            : ((currentQuery?.limit as string) ?? String(defaultLimit)),
-        page: pageQuery as string,
-        search: 'search' in query ? query.search : (currentQuery?.search as string),
+        limit: 'limit' in query ? query.limit : (currentQuery?.limit ?? String(defaultLimit)),
+        page,
+        search: 'search' in query ? query.search : currentQuery?.search,
         sort: 'sort' in query ? query.sort : ((currentQuery?.sort as string) ?? defaultSort),
-        where: 'where' in query ? query.where : (currentQuery?.where as Where),
+        where: 'where' in query ? query.where : currentQuery?.where,
       }
 
       if (modifySearchParams) {
@@ -122,6 +122,8 @@ export const ListQueryProvider: React.FC<ListQueryProps> = ({
         const onChangeFn = onQueryChange || onQueryChangeFromProps
         onChangeFn(newQuery)
       }
+
+      setCurrentQuery(newQuery)
     },
     [
       currentQuery?.page,
@@ -176,27 +178,30 @@ export const ListQueryProvider: React.FC<ListQueryProps> = ({
     [refineListData],
   )
 
+  // If `defaultLimit` or `defaultSort` are updated externally, update the query
   useEffect(() => {
     if (modifySearchParams) {
       let shouldUpdateQueryString = false
+      const newQuery = { ...(currentQueryRef.current || {}) }
 
-      if (isNumber(defaultLimit) && !('limit' in currentQuery)) {
-        currentQuery.limit = String(defaultLimit)
+      // Allow the URL to override the default limit
+      if (isNumber(defaultLimit) && !('limit' in currentQueryRef.current)) {
+        newQuery.limit = String(defaultLimit)
         shouldUpdateQueryString = true
       }
 
-      if (defaultSort && !('sort' in currentQuery)) {
-        currentQuery.sort = defaultSort
+      // Allow the URL to override the default sort
+      if (defaultSort && !('sort' in currentQueryRef.current)) {
+        newQuery.sort = defaultSort
         shouldUpdateQueryString = true
       }
 
-      setCurrentQuery(currentQuery)
-
       if (shouldUpdateQueryString) {
-        router.replace(`?${qs.stringify(currentQuery)}`)
+        setCurrentQuery(newQuery)
+        router.replace(`?${qs.stringify(newQuery)}`)
       }
     }
-  }, [defaultSort, defaultLimit, router, modifySearchParams, currentQuery])
+  }, [defaultSort, defaultLimit, router, modifySearchParams])
 
   return (
     <Context.Provider

diff --git a/packages/ui/src/views/List/index.tsx b/packages/ui/src/views/List/index.tsx
@@ -109,6 +109,7 @@ export const DefaultListView: React.FC<ListViewClientProps> = (props) => {
     handlePerPageChange,
     query,
   } = useListQuery()
+
   const { openModal } = useModal()
   const { setCollectionSlug, setCurrentActivePath, setOnSuccess } = useBulkUpload()
   const { drawerSlug: bulkUploadDrawerSlug } = useBulkUpload()

diff --git a/test/admin/e2e/2/e2e.spec.ts b/test/admin/e2e/2/e2e.spec.ts
@@ -439,27 +439,25 @@ describe('admin2', () => {
 
       test('should reset page when filters are applied', async () => {
         await deleteAllPosts()
-        await mapAsync([...Array(6)], async () => {
-          await createPost()
-        })
-        await page.reload()
-        await mapAsync([...Array(6)], async () => {
-          await createPost({ title: 'test' })
-        })
+
+        await Promise.all(
+          Array.from({ length: 12 }, async (_, i) => {
+            if (i < 6) {
+              await createPost()
+            } else {
+              await createPost({ title: 'test' })
+            }
+          }),
+        )
+
         await page.reload()
 
-        const pageInfo = page.locator('.collection-list__page-info')
-        const perPage = page.locator('.per-page')
         const tableItems = page.locator(tableRowLocator)
 
         await expect(tableItems).toHaveCount(10)
-        await expect(pageInfo).toHaveText('1-10 of 12')
-        await expect(perPage).toContainText('Per Page: 10')
-
-        // go to page 2
+        await expect(page.locator('.collection-list__page-info')).toHaveText('1-10 of 12')
+        await expect(page.locator('.per-page')).toContainText('Per Page: 10')
         await page.goto(`${postsUrl.list}?limit=10&page=2`)
-
-        // add filter
         await openListFilters(page, {})
         await page.locator('.where-builder__add-first-filter').click()
         await page.locator('.condition__field .rs__control').click()
@@ -468,9 +466,7 @@ describe('admin2', () => {
         await page.locator('.condition__operator .rs__control').click()
         await options.locator('text=equals').click()
         await page.locator('.condition__value input').fill('test')
-
-        // expect to be on page 1
-        await expect(pageInfo).toHaveText('1-6 of 6')
+        await expect(page.locator('.collection-list__page-info')).toHaveText('1-6 of 6')
       })
     })
 
@@ -685,28 +681,52 @@ describe('admin2', () => {
     describe('pagination', () => {
       test('should paginate', async () => {
         await deleteAllPosts()
+
         await mapAsync([...Array(11)], async () => {
           await createPost()
         })
-        await page.reload()
 
-        const pageInfo = page.locator('.collection-list__page-info')
-        const perPage = page.locator('.per-page')
-        const paginator = page.locator('.paginator')
+        await page.reload()
         const tableItems = page.locator(tableRowLocator)
-
         await expect(tableItems).toHaveCount(10)
-        await expect(pageInfo).toHaveText('1-10 of 11')
-        await expect(perPage).toContainText('Per Page: 10')
-
-        // Forward one page and back using numbers
-        await paginator.locator('button').nth(1).click()
+        await expect(page.locator('.collection-list__page-info')).toHaveText('1-10 of 11')
+        await expect(page.locator('.per-page')).toContainText('Per Page: 10')
+        await page.locator('.paginator button').nth(1).click()
         await expect.poll(() => page.url(), { timeout: POLL_TOPASS_TIMEOUT }).toContain('page=2')
         await expect(tableItems).toHaveCount(1)
-        await paginator.locator('button').nth(0).click()
+        await page.locator('.paginator button').nth(0).click()
         await expect.poll(() => page.url(), { timeout: POLL_TOPASS_TIMEOUT }).toContain('page=1')
         await expect(tableItems).toHaveCount(10)
       })
+
+      test('should paginate and maintain perPage', async () => {
+        await deleteAllPosts()
+
+        await mapAsync([...Array(26)], async () => {
+          await createPost()
+        })
+
+        await page.reload()
+        const tableItems = page.locator(tableRowLocator)
+        await expect(tableItems).toHaveCount(10)
+        await expect(page.locator('.collection-list__page-info')).toHaveText('1-10 of 26')
+        await expect(page.locator('.per-page')).toContainText('Per Page: 10')
+        await page.locator('.per-page .popup-button').click()
+
+        await page
+          .locator('.per-page button.per-page__button', {
+            hasText: '25',
+          })
+          .click()
+
+        await expect(tableItems).toHaveCount(25)
+        await expect(page.locator('.per-page .per-page__base-button')).toContainText('Per Page: 25')
+        await page.locator('.paginator button').nth(1).click()
+        await expect.poll(() => page.url(), { timeout: POLL_TOPASS_TIMEOUT }).toContain('page=2')
+        await expect(tableItems).toHaveCount(1)
+        await expect(page.locator('.per-page')).toContainText('Per Page: 25')
+        await expect(page.locator('.collection-list__page-info')).toHaveText('26-26 of 26')
+      })
     })
 
     // TODO: Troubleshoot flaky suite

diff --git a/test/admin/payload-types.ts b/test/admin/payload-types.ts
@@ -125,8 +125,6 @@ export interface Upload {
   focalY?: number | null;
 }
 /**
- * This is a custom collection description.
- *
  * This interface was referenced by `Config`'s JSON-Schema
  * via the `definition` "posts".
  */
@@ -167,9 +165,6 @@ export interface Post {
   defaultValueField?: string | null;
   relationship?: (string | null) | Post;
   customCell?: string | null;
-  /**
-   * This is a very long description that takes many characters to complete and hopefully will wrap instead of push the sidebar open, lorem ipsum dolor sit amet consectetur adipisicing elit. Quisquam, voluptatum voluptates. Quisquam, voluptatum voluptates.
-   */
   sidebarField?: string | null;
   updatedAt: string;
   createdAt: string;
@@ -251,13 +246,7 @@ export interface CustomField {
   id: string;
   customTextServerField?: string | null;
   customTextClientField?: string | null;
-  /**
-   * Static field description.
-   */
   descriptionAsString?: string | null;
-  /**
-   * Function description
-   */
   descriptionAsFunction?: string | null;
   descriptionAsComponent?: string | null;
   customSelectField?: string | null;
@@ -339,34 +328,6 @@ export interface Geo {
   updatedAt: string;
   createdAt: string;
 }
-/**
- * Description
- *
- * This interface was referenced by `Config`'s JSON-Schema
- * via the `definition` "customIdTab".
- */
-export interface CustomIdTab {
-  title?: string | null;
-  id: string;
-  description?: string | null;
-  number?: number | null;
-  updatedAt: string;
-  createdAt: string;
-}
-/**
- * Description
- *
- * This interface was referenced by `Config`'s JSON-Schema
- * via the `definition` "customIdRow".
- */
-export interface CustomIdRow {
-  title?: string | null;
-  id: string;
-  description?: string | null;
-  number?: number | null;
-  updatedAt: string;
-  createdAt: string;
-}
 /**
  * This interface was referenced by `Config`'s JSON-Schema
  * via the `definition` "disable-duplicate".