Merge pull request #10804 from wellcomecollection/pagination-input-re…

…moved Simplify pagination and tweak story
wellcomecollection · Apr 18, 2024 · e837e02 · e837e02
2 parents 9a11efb + 699ad79
commit e837e02
Show file tree

Hide file tree

Showing 9 changed files with 56 additions and 77 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 ({