From 3961223cc1d64bda3565062ab5c04e1d548ca64c Mon Sep 17 00:00:00 2001 From: Jacob Fletcher Date: Wed, 27 Nov 2024 14:49:53 -0500 Subject: [PATCH] fix(ui): retains search params when navigating back (#9576) Closes #9132. When query params are present in the URL, such as after searching or filtering in the list view, they are not being retained after navigating back to that view via `history.back()` (i.e. the back button). This makes it difficult to quickly navigate in and out of documents from the list view when an underlying search exists. This was because the `SearchParamsProvider` is stale when the new view renders, which then replaces the URL with these stale params. The fix here is to _not_ use the `SearchParamsProvider` at all, and instead use `next/navigation` directly. Ultimately, this provider should likely be marked deprecated and then removed in the next major release for this very reason. --- packages/ui/src/providers/ListQuery/index.tsx | 9 +++++---- packages/ui/src/providers/SearchParams/index.tsx | 16 +++++++++------- test/admin/e2e/2/e2e.spec.ts | 12 ++++++++++++ test/helpers/e2e/navigateToDoc.ts | 12 ++++++++---- 4 files changed, 34 insertions(+), 15 deletions(-) diff --git a/packages/ui/src/providers/ListQuery/index.tsx b/packages/ui/src/providers/ListQuery/index.tsx index 9afbebfe976..62bbc66a6d2 100644 --- a/packages/ui/src/providers/ListQuery/index.tsx +++ b/packages/ui/src/providers/ListQuery/index.tsx @@ -1,16 +1,16 @@ 'use client' import type { ListQuery, PaginatedDocs, Where } from 'payload' -import { useRouter } from 'next/navigation.js' +import { useRouter, useSearchParams } from 'next/navigation.js' import { isNumber } from 'payload/shared' import * as qs from 'qs-esm' -import React, { createContext, useCallback, useContext, useEffect, useState } from 'react' +import React, { createContext, useCallback, useContext, useEffect, useMemo, useState } from 'react' import type { Column } from '../../elements/Table/index.js' import { useListDrawerContext } from '../../elements/ListDrawer/Provider.js' import { usePreferences } from '../Preferences/index.js' -import { useSearchParams } from '../SearchParams/index.js' +import { createParams } from '../SearchParams/index.js' export type ColumnPreferences = Pick[] @@ -58,7 +58,8 @@ export const ListQueryProvider: React.FC = ({ 'use no memo' const router = useRouter() const { setPreference } = usePreferences() - const { searchParams } = useSearchParams() + const rawSearchParams = useSearchParams() + const searchParams = useMemo(() => createParams(rawSearchParams), [rawSearchParams]) const { onQueryChange } = useListDrawerContext() diff --git a/packages/ui/src/providers/SearchParams/index.tsx b/packages/ui/src/providers/SearchParams/index.tsx index 2deac3bafb7..83f62597e8c 100644 --- a/packages/ui/src/providers/SearchParams/index.tsx +++ b/packages/ui/src/providers/SearchParams/index.tsx @@ -1,4 +1,6 @@ 'use client' +import type { ReadonlyURLSearchParams } from 'next/navigation.js' + import { useSearchParams as useNextSearchParams } from 'next/navigation.js' import * as qs from 'qs-esm' import React, { createContext, useContext } from 'react' @@ -17,20 +19,20 @@ const initialContext: SearchParamsContext = { const Context = createContext(initialContext) -function createParams(search: string) { +export function createParams(params: ReadonlyURLSearchParams): State { + const search = params.toString() + return qs.parse(search, { depth: 10, ignoreQueryPrefix: true, }) } -// TODO: abstract the `next/navigation` dependency out from this provider so that it can be used in other contexts +// TODO: this provider should likely be marked as deprecated and then removed in the next major release export const SearchParamsProvider: React.FC<{ children?: React.ReactNode }> = ({ children }) => { const nextSearchParams = useNextSearchParams() - const searchString = nextSearchParams.toString() - const initialParams = createParams(searchString) - const [searchParams, setSearchParams] = React.useState(initialParams) + const [searchParams, setSearchParams] = React.useState(() => createParams(nextSearchParams)) const stringifyParams = React.useCallback( ({ params, replace = false }: { params: State; replace?: boolean }) => { @@ -46,8 +48,8 @@ export const SearchParamsProvider: React.FC<{ children?: React.ReactNode }> = ({ ) React.useEffect(() => { - setSearchParams(createParams(searchString)) - }, [searchString]) + setSearchParams(createParams(nextSearchParams)) + }, [nextSearchParams]) return {children} } diff --git a/test/admin/e2e/2/e2e.spec.ts b/test/admin/e2e/2/e2e.spec.ts index 1eb5ab296d6..86d2c1becae 100644 --- a/test/admin/e2e/2/e2e.spec.ts +++ b/test/admin/e2e/2/e2e.spec.ts @@ -26,6 +26,7 @@ const description = 'Description' let payload: PayloadTestSDK +import { goToFirstCell, navigateToDoc } from 'helpers/e2e/navigateToDoc.js' import { toggleColumn } from 'helpers/e2e/toggleColumn.js' import path from 'path' import { wait } from 'payload/shared' @@ -174,6 +175,17 @@ describe('admin2', () => { await expect(page.locator(tableRowLocator)).toHaveCount(1) }) + test('search should persist through browser back button', async () => { + const url = `${postsUrl.list}?limit=10&page=1&search=post1` + await page.goto(url) + await page.waitForURL(url) + await expect(page.locator('#search-filter-input')).toHaveValue('post1') + await goToFirstCell(page, postsUrl) + await page.goBack() + await wait(1000) // wait one second to ensure that the new view does not accidentally reset the search + await page.waitForURL(url) + }) + test('search should not persist between navigation', async () => { const url = `${postsUrl.list}?limit=10&page=1&search=test` await page.goto(url) diff --git a/test/helpers/e2e/navigateToDoc.ts b/test/helpers/e2e/navigateToDoc.ts index 288e13bfe29..0c2cdda702a 100644 --- a/test/helpers/e2e/navigateToDoc.ts +++ b/test/helpers/e2e/navigateToDoc.ts @@ -1,12 +1,16 @@ import type { Page } from '@playwright/test' import type { AdminUrlUtil } from 'helpers/adminUrlUtil.js' -export const navigateToDoc = async (page: Page, urlUtil: AdminUrlUtil) => { - await page.goto(urlUtil.list) - const regex = new RegExp(`^${urlUtil.list}(?:\\?.*)?$`) - await page.waitForURL(regex) +export const goToFirstCell = async (page: Page, urlUtil: AdminUrlUtil) => { const cellLink = page.locator(`tbody tr:first-child td a`).first() const linkURL = await cellLink.getAttribute('href') await page.goto(`${urlUtil.serverURL}${linkURL}`) await page.waitForURL(`**${linkURL}`) } + +export const navigateToDoc = async (page: Page, urlUtil: AdminUrlUtil) => { + await page.goto(urlUtil.list) + const regex = new RegExp(`^${urlUtil.list}(?:\\?.*)?$`) + await page.waitForURL(regex) + await goToFirstCell(page, urlUtil) +}