diff --git a/cardigan/stories/components/Pagination/Pagination.stories.mdx b/cardigan/stories/components/Pagination/Pagination.stories.mdx index 224e880b60..de05619b4c 100644 --- a/cardigan/stories/components/Pagination/Pagination.stories.mdx +++ b/cardigan/stories/components/Pagination/Pagination.stories.mdx @@ -6,18 +6,8 @@ import * as stories from './Pagination.stories.tsx'; # Pagination -<Description> - If somebody is midway through a list of pages, we show both the previous and - next buttons: -</Description> +<Story story={stories.startOfPagination} /> <Story story={stories.middleOfPagination} /> -<Description> - If somebody is at the start or end of a list of pages, we only show the - next/previous button as appropriate, for example: -</Description> - -<Story story={stories.startOfPagination} /> - <ArgsTable /> diff --git a/cardigan/stories/components/Pagination/Pagination.stories.tsx b/cardigan/stories/components/Pagination/Pagination.stories.tsx index cf29093818..9f413283ad 100644 --- a/cardigan/stories/components/Pagination/Pagination.stories.tsx +++ b/cardigan/stories/components/Pagination/Pagination.stories.tsx @@ -3,14 +3,36 @@ import Pagination, { } from '@weco/content/components/Pagination/Pagination'; const Template = (args: Props) => ( - <Pagination totalPages={args.totalPages} ariaLabel="Cardigan pagination" /> + <div + style={{ + padding: '10px', + backgroundColor: args.hasDarkBg ? 'black' : 'transparent', + color: args.hasDarkBg ? 'white' : 'black', + }} + > + <Pagination {...args} ariaLabel="Cardigan pagination" /> + </div> ); export const middleOfPagination = Template.bind({}); middleOfPagination.args = { totalPages: 10, + hasDarkBg: false, + isHiddenMobile: false, }; -middleOfPagination.storyName = 'Midway through pagination'; +middleOfPagination.argTypes = { + totalPages: { + options: [5, 10, 250], + control: { type: 'radio' }, + }, + hasDarkBg: { control: 'boolean' }, + ariaLabel: { + table: { + disable: true, + }, + }, +}; +middleOfPagination.storyName = 'Midway or end of pagination'; middleOfPagination.parameters = { nextRouter: { query: { @@ -22,5 +44,19 @@ middleOfPagination.parameters = { export const startOfPagination = Template.bind({}); startOfPagination.args = { totalPages: 10, + hasDarkBg: false, + isHiddenMobile: false, +}; +startOfPagination.storyName = 'On first page'; +startOfPagination.argTypes = { + totalPages: { + options: [1, 5, 10, 250], + control: { type: 'radio' }, + }, + hasDarkBg: { control: 'boolean' }, + ariaLabel: { + table: { + disable: true, + }, + }, }; -startOfPagination.storyName = 'Start of pagination'; diff --git a/content/webapp/components/Pagination/Pagination.tsx b/content/webapp/components/Pagination/Pagination.tsx index 419db80255..76d7d9b30f 100644 --- a/content/webapp/components/Pagination/Pagination.tsx +++ b/content/webapp/components/Pagination/Pagination.tsx @@ -1,12 +1,10 @@ -import { useEffect, useState, FunctionComponent, useContext } from 'react'; +import { useEffect, useState, FunctionComponent } from 'react'; import Link from 'next/link'; import { useRouter } from 'next/router'; import styled from 'styled-components'; -import { AppContext } from '@weco/common/views/components/AppContext/AppContext'; import { chevron } from '@weco/common/icons'; import Icon from '@weco/common/views/components/Icon/Icon'; -import ConditionalWrapper from '@weco/common/views/components/ConditionalWrapper/ConditionalWrapper'; import { font } from '@weco/common/utils/classnames'; import { formatNumber } from '@weco/common/utils/grammar'; @@ -15,7 +13,6 @@ export type Props = { ariaLabel: string; hasDarkBg?: boolean; isHiddenMobile?: boolean; - formId?: string; }; const Container = styled.nav.attrs({ @@ -77,33 +74,17 @@ const ChevronWrapper = styled.a<{ $prev?: boolean; $hasDarkBg?: boolean }>` `} `; -const PageSelectorInput = styled.input<{ $darkBg?: boolean }>` - height: 36px; - width: 36px; - max-width: 50px; - background: none; - color: ${({ $darkBg, theme }) => theme.color($darkBg ? 'white' : 'black')}; - border: ${({ $darkBg, theme }) => - theme.color($darkBg ? 'neutral.300' : 'neutral.600')} - 1px solid; - text-align: center; - margin: 0 10px; -`; - export const Pagination: FunctionComponent<Props> = ({ totalPages, ariaLabel, hasDarkBg, isHiddenMobile, - formId, }) => { const router = useRouter(); const { query, pathname } = router; const pageNumber = query.page ? Number(query.page) : 1; const [currentPage, setCurrentPage] = useState(pageNumber); - const [isFocused, setIsFocused] = useState(false); - const { isEnhanced } = useContext(AppContext); useEffect(() => { // Only push changes if the page number is a different one than on currently @@ -147,34 +128,10 @@ export const Pagination: FunctionComponent<Props> = ({ </Link> )} - {isEnhanced ? ( - <ConditionalWrapper - condition={!formId} - wrapper={children => <form>{children}</form>} - > - <span aria-hidden>Showing page</span> - <PageSelectorInput - name="page" - onFocus={() => setIsFocused(true)} - onBlur={() => setIsFocused(false)} - // We only use the formId if the input is focused - // as we can have more than one paginator on the same page - // and don't want to submit the same input with different values - form={isFocused ? formId : ''} - aria-label={`Jump to page ${currentPage} of ${formatNumber( - totalPages - )}`} - value={currentPage} - onChange={e => setCurrentPage(Number(e.target.value))} - $darkBg={hasDarkBg} - /> - <span aria-hidden>/ {formatNumber(totalPages)}</span> - </ConditionalWrapper> - ) : ( - <span> - Page <strong>{currentPage}</strong> of {formatNumber(totalPages)} - </span> - )} + <span> + Page <strong data-testid="current-page">{currentPage}</strong> of{' '} + {formatNumber(totalPages)} + </span> {showNext && ( <Link diff --git a/content/webapp/pages/search/events.tsx b/content/webapp/pages/search/events.tsx index 5e77e129c9..34beb858ea 100644 --- a/content/webapp/pages/search/events.tsx +++ b/content/webapp/pages/search/events.tsx @@ -166,7 +166,6 @@ export const EventsSearchPage: NextPageWithLayout<Props> = ({ }} /> <Pagination - formId={SEARCH_PAGES_FORM_ID} totalPages={eventResponseList.totalPages} ariaLabel="Events search pagination" isHiddenMobile @@ -180,7 +179,6 @@ export const EventsSearchPage: NextPageWithLayout<Props> = ({ <PaginationWrapper $verticalSpacing="l" $alignRight> <Pagination - formId={SEARCH_PAGES_FORM_ID} totalPages={eventResponseList.totalPages} ariaLabel="Events search pagination" /> diff --git a/content/webapp/pages/search/images.tsx b/content/webapp/pages/search/images.tsx index 8fc3fc419e..8b96f0607a 100644 --- a/content/webapp/pages/search/images.tsx +++ b/content/webapp/pages/search/images.tsx @@ -220,7 +220,6 @@ const ImagesSearchPage: NextPageWithLayout<Props> = ({ /> <Pagination - formId={SEARCH_PAGES_FORM_ID} totalPages={images.totalPages} ariaLabel="Image search pagination" hasDarkBg @@ -235,7 +234,6 @@ const ImagesSearchPage: NextPageWithLayout<Props> = ({ <PaginationWrapper $verticalSpacing="l" $alignRight> <Pagination - formId={SEARCH_PAGES_FORM_ID} totalPages={images.totalPages} ariaLabel="Image search pagination" hasDarkBg diff --git a/content/webapp/pages/search/stories.tsx b/content/webapp/pages/search/stories.tsx index f0b78d7995..3365b6100d 100644 --- a/content/webapp/pages/search/stories.tsx +++ b/content/webapp/pages/search/stories.tsx @@ -174,7 +174,6 @@ export const StoriesSearchPage: NextPageWithLayout<Props> = ({ }} /> <Pagination - formId={SEARCH_PAGES_FORM_ID} totalPages={storyResponseList.totalPages} ariaLabel="Stories search pagination" isHiddenMobile @@ -197,7 +196,6 @@ export const StoriesSearchPage: NextPageWithLayout<Props> = ({ <PaginationWrapper $verticalSpacing="l" $alignRight> <Pagination - formId={SEARCH_PAGES_FORM_ID} totalPages={storyResponseList.totalPages} ariaLabel="Stories search pagination" /> diff --git a/content/webapp/pages/search/works.tsx b/content/webapp/pages/search/works.tsx index 8d37ca4971..7d6f167838 100644 --- a/content/webapp/pages/search/works.tsx +++ b/content/webapp/pages/search/works.tsx @@ -203,7 +203,6 @@ const CatalogueSearchPage: NextPageWithLayout<Props> = ({ /> <Pagination - formId={SEARCH_PAGES_FORM_ID} totalPages={works.totalPages} ariaLabel="Catalogue search pagination" isHiddenMobile @@ -217,7 +216,6 @@ const CatalogueSearchPage: NextPageWithLayout<Props> = ({ <PaginationWrapper $verticalSpacing="l" $alignRight> <Pagination - formId={SEARCH_PAGES_FORM_ID} totalPages={works.totalPages} ariaLabel="Catalogue search pagination" /> diff --git a/playwright/test/helpers/search.ts b/playwright/test/helpers/search.ts index a3d5995456..ae7ec188e1 100644 --- a/playwright/test/helpers/search.ts +++ b/playwright/test/helpers/search.ts @@ -46,18 +46,22 @@ export const selectAndWaitForFilter = async ( }; export const navigateToNextPageAndConfirmNavigation = async (page: Page) => { - const paginationInput = page.getByTestId('pagination').getByRole('textbox'); + const firstPage = await page + .getByTestId('pagination') + .getByTestId('current-page') + .textContent(); - const currentPage = await paginationInput.inputValue(); const nextButton = page .getByTestId('pagination') .getByRole('link', { name: 'Next' }); await nextButton.click(); - await slowExpect(paginationInput).toHaveValue( - String(Number(currentPage) + 1) - ); + const newPage = await page + .getByTestId('pagination') + .getByTestId('current-page'); + + await slowExpect(newPage).toHaveText(String(Number(firstPage) + 1)); }; export const navigateToResultAndConfirmTitleMatches = async ( diff --git a/playwright/test/search-works.test.ts b/playwright/test/search-works.test.ts index eb8c9a9b35..f03c6d6894 100644 --- a/playwright/test/search-works.test.ts +++ b/playwright/test/search-works.test.ts @@ -106,8 +106,8 @@ test('(4) | The user is sorting by production dates in search; sort updates URL await select.selectOption({ index: 1 }); await expect(select).toHaveValue('production.dates.asc'); await expect( - page.getByTestId('pagination').locator('input[name="page"]') - ).toHaveValue('1'); + page.getByTestId('pagination').getByTestId('current-page') + ).toHaveText('1'); }); test('(5) | The user is coming from a prefiltered series search; they should be able to add more filters', async ({