From b0d1f9a4e1fd55df3dc016290e45342dddd53234 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 7 Jun 2023 16:54:23 -0400 Subject: [PATCH 1/9] makes the showPages prop on our Pagination components responsive --- src/DataTable/Pagination.tsx | 56 +++++++++++++++++-- .../Pagination.features.stories.tsx | 13 +++++ src/Pagination/Pagination.stories.tsx | 38 ++++++++++++- src/Pagination/Pagination.tsx | 23 +++++++- src/Pagination/model.tsx | 28 +++++++++- 5 files changed, 143 insertions(+), 15 deletions(-) diff --git a/src/DataTable/Pagination.tsx b/src/DataTable/Pagination.tsx index 34106b8476c..ec786a62d90 100644 --- a/src/DataTable/Pagination.tsx +++ b/src/DataTable/Pagination.tsx @@ -1,11 +1,12 @@ import {ChevronLeftIcon, ChevronRightIcon} from '@primer/octicons-react' -import React, {useState} from 'react' +import React, {useCallback, useState} from 'react' import styled from 'styled-components' import {get} from '../constants' import {Button} from '../internal/components/ButtonReset' import {LiveRegion, LiveRegionOutlet, Message} from '../internal/components/LiveRegion' import {VisuallyHidden} from '../internal/components/VisuallyHidden' import {warning} from '../utils/warning' +import {ResponsiveValue, viewportRanges} from '../hooks/useResponsiveValue' const StyledPagination = styled.nav` display: flex; @@ -93,6 +94,22 @@ const StyledPagination = styled.nav` min-height: 2rem; user-select: none; } + + ${ + // Hides pages based on the viewport range passed to `showPages` + Object.keys(viewportRanges) + .map(viewportRangeKey => { + return ` + @media (${viewportRanges[viewportRangeKey as keyof typeof viewportRanges]}) { + .TablePaginationStep[data-hidden-viewport-ranges*='${viewportRangeKey}'], + .TablePaginationStep[data-hidden-viewport-ranges*='${viewportRangeKey}'] + .TablePaginationTruncationStep { + display: none; + } + } + ` + }) + .join('') + } ` export type PaginationProps = Omit, 'onChange'> & { @@ -123,6 +140,11 @@ export type PaginationProps = Omit, 'onCha */ pageSize?: number + /** + * Whether to show the page numbers + */ + showPages?: boolean | ResponsiveValue + /** * Specify the total number of items within the collection */ @@ -141,6 +163,7 @@ export function Pagination({ id, onChange, pageSize = 25, + showPages = {narrow: false}, totalCount, }: PaginationProps) { const { @@ -171,6 +194,19 @@ export function Pagination({ const offsetEndIndex = offsetStartIndex + truncatedPageCount - 1 const hasLeadingTruncation = offsetStartIndex >= 2 const hasTrailingTruncation = pageCount - 1 - offsetEndIndex > 1 + const getViewportRangesToHidePages = useCallback(() => { + if (typeof showPages !== 'boolean') { + return Object.keys(showPages).filter(key => !showPages[key as keyof typeof viewportRanges]) as Array< + keyof typeof viewportRanges + > + } + + if (showPages) { + return [] + } else { + return Object.keys(viewportRanges) as Array + } + }, [showPages]) return ( @@ -202,7 +238,7 @@ export function Pagination({ {pageCount > 0 ? ( - + { @@ -229,7 +265,7 @@ export function Pagination({ const page = offsetStartIndex + i return ( - + { @@ -246,7 +282,7 @@ export function Pagination({ }) : null} {pageCount > 1 ? ( - + { @@ -317,12 +353,20 @@ function TruncationStep() { ) } -function Step({children}: React.PropsWithChildren) { - return
  • {children}
    • +function Step({ + children, + hiddenViewportRanges, +}: React.PropsWithChildren & {hiddenViewportRanges?: Array}) { + return ( +
  • + {children} +
    • + ) } type PageProps = React.PropsWithChildren<{ active: boolean + hiddenViewportRanges?: Array onClick: () => void }> diff --git a/src/Pagination/Pagination.features.stories.tsx b/src/Pagination/Pagination.features.stories.tsx index 36cd0bbb475..cb935341697 100644 --- a/src/Pagination/Pagination.features.stories.tsx +++ b/src/Pagination/Pagination.features.stories.tsx @@ -16,6 +16,19 @@ export const HidePageNumbers = () => ( e.preventDefault()} /> ) +export const HidePageNumbersByViewport = () => ( + <> + e.preventDefault()} /> +

    Page numbers are hidden on narrow viewports.

    + +) + +HidePageNumbersByViewport.parameters = { + viewport: { + defaultViewport: 'small', + }, +} + export const HigherSurroundingPageCount = () => ( e.preventDefault()} /> ) diff --git a/src/Pagination/Pagination.stories.tsx b/src/Pagination/Pagination.stories.tsx index b44d16258e2..b9e83dc8bf7 100644 --- a/src/Pagination/Pagination.stories.tsx +++ b/src/Pagination/Pagination.stories.tsx @@ -8,12 +8,38 @@ export default { component: Pagination, } as Meta> -export const Default = () => e.preventDefault()} /> +const parseShowPagesArg = (value: boolean | string) => { + if (typeof value === 'boolean') { + return value + } -export const Playground: Story> = args => ( - e.preventDefault()} {...args} /> + if (value === 'hide at narrow') { + return {narrow: false} + } + + if (value === 'hide at regular') { + return {regular: false} + } + + if (value === 'hide at wide') { + return {wide: false} + } +} + +export const Default = () => ( + e.preventDefault()} showPages={{narrow: false}} /> ) +export const Playground: Story> = ({showPages, ...args}) => { + return ( + e.preventDefault()} + showPages={parseShowPagesArg(showPages as boolean | string)} + {...args} + /> + ) +} + Playground.args = { currentPage: 5, marginPageCount: 1, @@ -22,6 +48,12 @@ Playground.args = { surroundingPageCount: 2, } Playground.argTypes = { + showPages: { + control: { + type: 'radio', + }, + options: [true, false, 'hide at narrow', 'hide at regular', 'hide at wide'], + }, hrefBuilder: { control: false, table: { diff --git a/src/Pagination/Pagination.tsx b/src/Pagination/Pagination.tsx index a193f4b1217..a46ce313a24 100644 --- a/src/Pagination/Pagination.tsx +++ b/src/Pagination/Pagination.tsx @@ -5,6 +5,7 @@ import {get} from '../constants' import sx, {SxProp} from '../sx' import getGlobalFocusStyles from '../_getGlobalFocusStyles' import {buildComponentData, buildPaginationModel} from './model' +import {ResponsiveValue, viewportRanges} from '../hooks/useResponsiveValue' const Page = styled.a` display: inline-block; @@ -78,6 +79,22 @@ const Page = styled.a` background-color: transparent; } + ${ + // Hides pages based on the viewport range passed to `showPages` + Object.keys(viewportRanges) + .map(viewportRangeKey => { + return ` + @media (${viewportRanges[viewportRangeKey as keyof typeof viewportRanges]}) { + &[data-hidden-viewport-ranges*='${viewportRangeKey}'], + &[data-hidden-viewport-ranges*='${viewportRangeKey}'] + .paginationBreak { + display: none; + } + } + ` + }) + .join('') + } + @supports (clip-path: polygon(50% 0, 100% 50%, 50% 100%)) { &[rel='prev']::before, &[rel='next']::after { @@ -130,7 +147,7 @@ type UsePaginationPagesParameters = { onPageChange: (e: React.MouseEvent, n: number) => void hrefBuilder: (n: number) => string marginPageCount: number - showPages?: boolean + showPages?: PaginationProps['showPages'] surroundingPageCount: number } @@ -147,7 +164,7 @@ function usePaginationPages({ const pageChange = React.useCallback((n: number) => (e: React.MouseEvent) => onPageChange(e, n), [onPageChange]) const model = React.useMemo(() => { - return buildPaginationModel(pageCount, currentPage, !!showPages, marginPageCount, surroundingPageCount) + return buildPaginationModel(pageCount, currentPage, marginPageCount, surroundingPageCount, showPages) }, [pageCount, currentPage, showPages, marginPageCount, surroundingPageCount]) const children = React.useMemo(() => { @@ -178,7 +195,7 @@ export type PaginationProps = { onPageChange?: (e: React.MouseEvent, n: number) => void hrefBuilder?: (n: number) => string marginPageCount?: number - showPages?: boolean + showPages?: boolean | ResponsiveValue surroundingPageCount?: number } diff --git a/src/Pagination/model.tsx b/src/Pagination/model.tsx index e8e80c7a682..bce6e5f5c42 100644 --- a/src/Pagination/model.tsx +++ b/src/Pagination/model.tsx @@ -1,13 +1,30 @@ +import {viewportRanges} from '../hooks/useResponsiveValue' +import {PaginationProps} from './Pagination' + export function buildPaginationModel( pageCount: number, currentPage: number, - showPages: boolean, marginPageCount: number, surroundingPageCount: number, + showPages?: PaginationProps['showPages'], ) { const pages = [] - if (showPages) { + const getViewportRangesToHidePages = () => { + if (showPages && typeof showPages !== 'boolean') { + return Object.keys(showPages).filter(key => !showPages[key as keyof typeof viewportRanges]) as Array< + keyof typeof viewportRanges + > + } + + if (showPages) { + return [] + } else { + return Object.keys(viewportRanges) as Array + } + } + + if (showPages !== false) { const pageNums: Array = [] const addPage = (n: number) => { if (n >= 1 && n <= pageCount) { @@ -85,6 +102,7 @@ export function buildPaginationModel( num, selected, precedesBreak, + hiddenViewportRanges: getViewportRangesToHidePages(), }) } else { if (lastDelta === 1) { @@ -93,6 +111,7 @@ export function buildPaginationModel( num, selected, precedesBreak, + hiddenViewportRanges: getViewportRangesToHidePages(), }) } else { // We skipped some, so add a break @@ -105,6 +124,7 @@ export function buildPaginationModel( num, selected, precedesBreak: false, + hiddenViewportRanges: getViewportRangesToHidePages(), }) } } @@ -132,6 +152,7 @@ type PageType = { disabled?: boolean selected?: boolean precedesBreak?: boolean + hiddenViewportRanges?: Array } export function buildComponentData( @@ -185,13 +206,14 @@ export function buildComponentData( 'aria-label': `Page ${page.num}${page.precedesBreak ? '...' : ''}`, onClick, 'aria-current': page.selected ? 'page' : undefined, + 'data-hidden-viewport-ranges': page.hiddenViewportRanges?.join(' '), }) break } case 'BREAK': { key = `page-${page.num}-break` content = '…' - Object.assign(props, {as: 'span', role: 'presentation'}) + Object.assign(props, {as: 'span', role: 'presentation', className: 'paginationBreak'}) } } From a10321155e46a0ac5ba787a1943a0a9fa87784c2 Mon Sep 17 00:00:00 2001 From: mperrotti Date: Wed, 7 Jun 2023 21:01:30 +0000 Subject: [PATCH 2/9] Update generated/components.json --- generated/components.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generated/components.json b/generated/components.json index 2bc6b888da9..8fae3b25bff 100644 --- a/generated/components.json +++ b/generated/components.json @@ -2932,7 +2932,7 @@ "stories": [ { "id": "components-pagination--default", - "code": "() => (\n e.preventDefault()}\n />\n)" + "code": "() => (\n e.preventDefault()}\n showPages={{\n narrow: false,\n }}\n />\n)" } ], "props": [ From 9655024d05752c50bc67954452b37162cfe1bc88 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 7 Jun 2023 17:02:10 -0400 Subject: [PATCH 3/9] updates docs --- src/Pagination/Pagination.docs.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Pagination/Pagination.docs.json b/src/Pagination/Pagination.docs.json index e564857f20f..1f6ba984d4e 100644 --- a/src/Pagination/Pagination.docs.json +++ b/src/Pagination/Pagination.docs.json @@ -39,7 +39,7 @@ }, { "name": "showPages", - "type": "boolean", + "type": "boolean | { narrow?: boolean, regular?: boolean, wide?: boolean }", "defaultValue": "true", "description": "Whether or not to show the individual page links." }, From 7361970c1df04d13ab080d2012e6b3c21221af7b Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 7 Jun 2023 17:03:50 -0400 Subject: [PATCH 4/9] adds changeset --- .changeset/green-rules-tickle.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/green-rules-tickle.md diff --git a/.changeset/green-rules-tickle.md b/.changeset/green-rules-tickle.md new file mode 100644 index 00000000000..76ac723b307 --- /dev/null +++ b/.changeset/green-rules-tickle.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +The showPages prop on both Pagination components can now accept a responsive value. From 802f13951714403b99147f7e95fda2afd489f2b2 Mon Sep 17 00:00:00 2001 From: mperrotti Date: Wed, 7 Jun 2023 21:10:03 +0000 Subject: [PATCH 5/9] Update generated/components.json --- generated/components.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/generated/components.json b/generated/components.json index f23915bbf7b..9484ec87c32 100644 --- a/generated/components.json +++ b/generated/components.json @@ -2970,7 +2970,7 @@ }, { "name": "showPages", - "type": "boolean", + "type": "boolean | { narrow?: boolean, regular?: boolean, wide?: boolean }", "defaultValue": "true", "description": "Whether or not to show the individual page links." }, From 7e52c8f8c2f85562d815365d4a34c6178ac66000 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 7 Jun 2023 17:42:09 -0400 Subject: [PATCH 6/9] updates tests --- .../Pagination/PaginationModel.test.tsx | 22 +++++++------- .../__snapshots__/Pagination.test.tsx.snap | 30 ++++++++++++++++++- 2 files changed, 40 insertions(+), 12 deletions(-) diff --git a/src/__tests__/Pagination/PaginationModel.test.tsx b/src/__tests__/Pagination/PaginationModel.test.tsx index 3a3ba2e6273..fa5829afd08 100644 --- a/src/__tests__/Pagination/PaginationModel.test.tsx +++ b/src/__tests__/Pagination/PaginationModel.test.tsx @@ -15,33 +15,33 @@ function last(array: Array, count = 1) { describe('Pagination model', () => { it('sets disabled on prev links', () => { - const model1 = buildPaginationModel(10, 1, true, 1, 2) + const model1 = buildPaginationModel(10, 1, 1, 2, true) expect(first(model1).type).toEqual('PREV') expect(first(model1).disabled).toBe(true) - const model2 = buildPaginationModel(10, 2, true, 1, 2) + const model2 = buildPaginationModel(10, 2, 1, 2, true) expect(first(model2).type).toEqual('PREV') expect(first(model2).disabled).toBe(false) }) it('sets disabled on next links', () => { - const model1 = buildPaginationModel(10, 10, true, 1, 2) + const model1 = buildPaginationModel(10, 10, 1, 2, true) expect(last(model1).type).toEqual('NEXT') expect(last(model1).disabled).toBe(true) - const model2 = buildPaginationModel(10, 9, true, 1, 2) + const model2 = buildPaginationModel(10, 9, 1, 2, true) expect(last(model2).type).toEqual('NEXT') expect(last(model2).disabled).toBe(false) }) it('sets the page number for prev and next links', () => { - const model = buildPaginationModel(10, 5, true, 1, 2) + const model = buildPaginationModel(10, 5, 1, 2, true) expect(first(model).num).toEqual(4) expect(last(model).num).toEqual(6) }) it('ensures margin pages on the left', () => { - const model = buildPaginationModel(10, 10, true, 2, 0) + const model = buildPaginationModel(10, 10, 2, 0, true) const slice = first(model, 5) const expected = [ @@ -56,7 +56,7 @@ describe('Pagination model', () => { }) it('ensures margin pages on the right', () => { - const model = buildPaginationModel(10, 1, true, 2, 0) + const model = buildPaginationModel(10, 1, 2, 0, true) const slice = last(model, 5) const expected = [ @@ -71,7 +71,7 @@ describe('Pagination model', () => { }) it('ensures that the current page is surrounded by the right number of pages', () => { - const model = buildPaginationModel(10, 5, true, 1, 1) + const model = buildPaginationModel(10, 5, 1, 1, true) const expected = [ {type: 'PREV', num: 4}, {type: 'NUM', num: 1, precedesBreak: true}, @@ -87,7 +87,7 @@ describe('Pagination model', () => { }) it('adds items to the right if it hits bounds to the left', () => { - const model = buildPaginationModel(15, 2, true, 1, 1) + const model = buildPaginationModel(15, 2, 1, 1, true) const expected = [ {type: 'PREV', num: 1}, {type: 'NUM', num: 1}, @@ -102,7 +102,7 @@ describe('Pagination model', () => { }) it('adds items to the left if it hits bounds to the right', () => { - const model = buildPaginationModel(15, 14, true, 1, 1) + const model = buildPaginationModel(15, 14, 1, 1, true) const expected = [ // normally with a surround of 1, only 13 and 15 would be shown // however, since 15 was already shown, we extend to 12 @@ -117,7 +117,7 @@ describe('Pagination model', () => { }) it('correctly creates breaks next to the next/prev links when margin is 0', () => { - const model = buildPaginationModel(10, 5, true, 0, 1) + const model = buildPaginationModel(10, 5, 0, 1, true) const expected = [ {type: 'PREV'}, {type: 'BREAK', num: 1}, diff --git a/src/__tests__/Pagination/__snapshots__/Pagination.test.tsx.snap b/src/__tests__/Pagination/__snapshots__/Pagination.test.tsx.snap index b3b1834f906..25f423a5449 100644 --- a/src/__tests__/Pagination/__snapshots__/Pagination.test.tsx.snap +++ b/src/__tests__/Pagination/__snapshots__/Pagination.test.tsx.snap @@ -103,6 +103,27 @@ exports[`Pagination renders consistently 1`] = ` text-align: center; } +@media ((max-width:calc(768px - 0.02px))) { + .c2[data-hidden-viewport-ranges*='narrow'], + .c2[data-hidden-viewport-ranges*='narrow'] + .paginationBreak { + display: none; + } +} + +@media ((min-width:768px)) { + .c2[data-hidden-viewport-ranges*='regular'], + .c2[data-hidden-viewport-ranges*='regular'] + .paginationBreak { + display: none; + } +} + +@media ((min-width:1400px)) { + .c2[data-hidden-viewport-ranges*='wide'], + .c2[data-hidden-viewport-ranges*='wide'] + .paginationBreak { + display: none; + } +} + @supports (-webkit-clip-path:polygon(50% 0,100% 50%,50% 100%)) or (clip-path:polygon(50% 0,100% 50%,50% 100%)) { .c2[rel='prev']::before, .c2[rel='next']::after { @@ -145,6 +166,7 @@ exports[`Pagination renders consistently 1`] = ` aria-current="page" aria-label="Page 1" className="c2" + data-hidden-viewport-ranges="" href="#1" onClick={[Function]} > @@ -153,6 +175,7 @@ exports[`Pagination renders consistently 1`] = ` @@ -161,6 +184,7 @@ exports[`Pagination renders consistently 1`] = ` @@ -169,6 +193,7 @@ exports[`Pagination renders consistently 1`] = ` @@ -177,6 +202,7 @@ exports[`Pagination renders consistently 1`] = ` @@ -185,13 +211,14 @@ exports[`Pagination renders consistently 1`] = ` 6 … @@ -199,6 +226,7 @@ exports[`Pagination renders consistently 1`] = ` From 8cb045a90293435d544df5eddad7a38997bdaf3b Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Fri, 7 Jul 2023 13:00:44 -0400 Subject: [PATCH 7/9] updates changeset with changed components --- .changeset/green-rules-tickle.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.changeset/green-rules-tickle.md b/.changeset/green-rules-tickle.md index 76ac723b307..82f249817c1 100644 --- a/.changeset/green-rules-tickle.md +++ b/.changeset/green-rules-tickle.md @@ -3,3 +3,5 @@ --- The showPages prop on both Pagination components can now accept a responsive value. + + \ No newline at end of file From d799b8cd5928658b96c80735fb03f1cfe7b92d21 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Fri, 7 Jul 2023 13:10:38 -0400 Subject: [PATCH 8/9] updates changeset with changed components again --- .changeset/green-rules-tickle.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/green-rules-tickle.md b/.changeset/green-rules-tickle.md index 82f249817c1..42b83cab770 100644 --- a/.changeset/green-rules-tickle.md +++ b/.changeset/green-rules-tickle.md @@ -4,4 +4,4 @@ The showPages prop on both Pagination components can now accept a responsive value. - \ No newline at end of file + \ No newline at end of file From 1fee6f0ae2e66df5764df011064abfb3e4492200 Mon Sep 17 00:00:00 2001 From: Mike Perrotti Date: Wed, 12 Jul 2023 16:26:59 -0400 Subject: [PATCH 9/9] refactor to only calculate viewport sizes once --- src/DataTable/Pagination.tsx | 31 ++++---- src/Pagination/Pagination.tsx | 63 +++++++++++----- src/Pagination/model.tsx | 28 +------ .../Pagination/PaginationModel.test.tsx | 22 +++--- .../__snapshots__/Pagination.test.tsx.snap | 75 +++++++++++-------- 5 files changed, 119 insertions(+), 100 deletions(-) diff --git a/src/DataTable/Pagination.tsx b/src/DataTable/Pagination.tsx index ec786a62d90..2769a4e943e 100644 --- a/src/DataTable/Pagination.tsx +++ b/src/DataTable/Pagination.tsx @@ -101,10 +101,17 @@ const StyledPagination = styled.nav` .map(viewportRangeKey => { return ` @media (${viewportRanges[viewportRangeKey as keyof typeof viewportRanges]}) { - .TablePaginationStep[data-hidden-viewport-ranges*='${viewportRangeKey}'], - .TablePaginationStep[data-hidden-viewport-ranges*='${viewportRangeKey}'] + .TablePaginationTruncationStep { + .TablePaginationSteps[data-hidden-viewport-ranges*='${viewportRangeKey}'] > *:not(:first-child):not(:last-child) { display: none; } + + .TablePaginationSteps[data-hidden-viewport-ranges*='${viewportRangeKey}'] > *:first-child { + margin-inline-end: 0; + } + + .TablePaginationSteps[data-hidden-viewport-ranges*='${viewportRangeKey}'] > *:last-child { + margin-inline-start: 0; + } } ` }) @@ -213,7 +220,7 @@ export function Pagination({ -
      +