From d68ee155aca2331b136adf7a2c028737d75d6a1c Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 18 Apr 2023 16:23:33 -0500 Subject: [PATCH 01/15] chore: check-in work --- src/DataTable/DataTable.features.stories.tsx | 66 +++ src/DataTable/Pagination.tsx | 477 +++++++++++++++++++ src/DataTable/Table.tsx | 4 +- src/DataTable/index.ts | 2 + src/DataTable/storybook/data.ts | 89 ++++ src/internal/components/ButtonReset.tsx | 27 ++ src/internal/components/VisuallyHidden.tsx | 25 + 7 files changed, 688 insertions(+), 2 deletions(-) create mode 100644 src/DataTable/Pagination.tsx create mode 100644 src/DataTable/storybook/data.ts create mode 100644 src/internal/components/ButtonReset.tsx create mode 100644 src/internal/components/VisuallyHidden.tsx diff --git a/src/DataTable/DataTable.features.stories.tsx b/src/DataTable/DataTable.features.stories.tsx index 2fc15c7f84c..119c87f065a 100644 --- a/src/DataTable/DataTable.features.stories.tsx +++ b/src/DataTable/DataTable.features.stories.tsx @@ -1359,3 +1359,69 @@ export const WithRightAlignedColumns = () => { ) } + +export const WithPagination = () => { + return ( + + + Repositories + + + A subtitle could appear here to give extra context to the data. + + { + return + }, + }, + { + header: 'Updated', + field: 'updatedAt', + renderCell: row => { + return + }, + }, + { + header: 'Dependabot', + field: 'securityFeatures.dependabot', + renderCell: row => { + return row.securityFeatures.dependabot.length > 0 ? ( + + {row.securityFeatures.dependabot.map(feature => { + return + })} + + ) : null + }, + }, + { + header: 'Code scanning', + field: 'securityFeatures.codeScanning', + renderCell: row => { + return row.securityFeatures.codeScanning.length > 0 ? ( + + {row.securityFeatures.codeScanning.map(feature => { + return + })} + + ) : null + }, + }, + ]} + /> + + + ) +} diff --git a/src/DataTable/Pagination.tsx b/src/DataTable/Pagination.tsx new file mode 100644 index 00000000000..f0f7494beb6 --- /dev/null +++ b/src/DataTable/Pagination.tsx @@ -0,0 +1,477 @@ +import {ChevronLeftIcon, ChevronRightIcon} from '@primer/octicons-react' +import React, {useState} from 'react' +import styled from 'styled-components' +import {get} from '../constants' +import {Button} from '../internal/components/ButtonReset' +import {VisuallyHidden} from '../internal/components/VisuallyHidden' + +const StyledPagination = styled.nav` + display: flex; + align-items: center; + justify-content: space-between; + width: 100%; + grid-area: footer; + padding: 0.5rem 1rem; + border: 1px solid ${get('colors.border.default')}; + border-top-width: 0; + border-end-start-radius: 6px; + border-end-end-radius: 6px; + + .TablePaginationRange { + color: ${get('colors.fg.muted')}; + font-size: 0.75rem; + margin: 0; + } + + .TablePaginationSteps { + display: flex; + list-style: none; + align-items: center; + color: ${get('colors.fg.default')}; + font-size: 0.875rem; + margin: 0; + padding: 0; + } + + .TablePaginationStep:first-of-type { + margin-right: 1rem; + } + + .TablePaginationStep:last-of-type { + margin-left: 1rem; + } + + .TablePaginationAction { + display: flex; + align-items: center; + color: ${get('colors.fg.muted')}; + font-size: 0.875rem; + line-height: calc(20 / 14); + user-select: none; + padding: 0.5rem; + border-radius: 6px; + } + + .TablePaginationAction[data-has-page] { + color: ${get('colors.accent.fg')}; + } + + .TablePaginationPage { + min-width: 2rem; + min-height: 2rem; + display: flex; + align-items: center; + justify-content: center; + font-size: 0.875rem; + line-height: calc(20 / 14); + user-select: none; + border-radius: 6px; + } + + .TablePaginationAction[data-has-page]:hover, + .TablePaginationAction[data-has-page]:focus, + .TablePaginationPage:hover, + .TablePaginationPage:focus { + background-color: ${get('colors.actionListItem.default.hoverBg')}; + transition-duration: 0.1s; + } + + .TablePaginationPage[data-active='true'] { + background-color: ${get('colors.accent.emphasis')}; + color: ${get('colors.fg.onEmphasis')}; + } + + .TablePaginationTruncated { + display: flex; + align-items: center; + justify-content: center; + min-width: 2rem; + min-height: 2rem; + user-select: none; + } +` + +export type PaginationProps = React.ComponentPropsWithoutRef<'nav'> & { + /** + * Provide a label for the navigation landmark rendered by this component + */ + 'aria-label': string + + /** + * Provide an optional index to specify the default selected page + */ + defaultPageIndex?: number + + /** + * Optionally provide an `id` that is placed on the navigation landmark + * rendered by this component + */ + id?: string + + /** + * Optionally provide a handler that is called whenever the pagination state + * is updated + */ + onChange?: (state: PaginationState) => void + + /** + * Optionally specify the number of items within a page + */ + pageSize?: number + + /** + * Specify the total number of items within the collection + */ + totalCount: number +} + +const VISIBLE_PAGE_COUNT = 4 + +export function Pagination({ + 'aria-label': label, + defaultPageIndex, + id, + onChange, + pageSize = 25, + totalCount, +}: PaginationProps) { + const { + pageIndex, + pageStart, + pageEnd, + pageCount, + hasPreviousPage, + hasNextPage, + selectPage, + selectNextPage, + selectPreviousPage, + } = usePagination({ + defaultPageIndex, + onChange, + pageSize, + totalCount, + }) + const { + windowStartIndex, + hasPagesBefore, + hasPagesAfter, + isWithinBounds, + shiftWindowDown, + shiftWindowUp, + shiftStartTo, + shiftEndTo, + } = useWindowing({ + defaultPageIndex, + size: VISIBLE_PAGE_COUNT, + pageCount, + }) + const pages = Array.from({length: VISIBLE_PAGE_COUNT}).map((_, i) => { + const offset = windowStartIndex + i + const active = offset === pageIndex + + return ( +
  • + +
    • + ) + }) + + return ( + + +
      +
    • + +
      • + {hasPagesBefore ? ( + <> +
    • + +
      • + + + ) : null} + {pages} + {hasPagesAfter ? ( + <> + +
    • + +
      • + + ) : null} +
    • + +
      • +
    +     + ) +} + +type RangeProps = { + pageStart: number + pageEnd: number + totalCount: number +} + +function Range({pageStart, pageEnd, totalCount}: RangeProps) { + return ( +

    + {pageStart + 1} +  through  + + {pageEnd} of {totalCount} +

    + ) +} + +interface PaginationState { + /** + * The index of currently selected page + */ + pageIndex: number +} + +interface PaginationConfig { + /** + * Provide an optional index to specify the default selected page + */ + defaultPageIndex?: number + + /** + * Provide an optional handler that is called whenever the pagination state + * has changed + */ + onChange?: (state: PaginationState) => void + + /** + * Specify the number of items within a page + */ + pageSize: number + + /** + * Specify the total number of items within the collection + */ + totalCount: number +} + +interface PaginationResult { + /** + * The index for the currently selected page + */ + pageIndex: number + + /** + * The number that represents the position of the item at the beginning of the + * current page. + */ + pageStart: number + + /** + * The number that represents the position of the item at the end of the + * current page. + */ + pageEnd: number + + /** + * The number of pages in the current pagination context + */ + pageCount: number + + /** + * Indicates if a previous page is available given the current state and + * pagination options + */ + hasPreviousPage: boolean + + /** + * Indicates if a next page is available given the current state and + * pagination options + */ + hasNextPage: boolean + + /** + * Perform an action to select the page at the given index + */ + selectPage: (pageIndex: number) => void + + /** + * Perform an action to select the previous page, if one is available + */ + selectPreviousPage: () => void + + /** + * Perform an action to select the next page, if one is available + */ + selectNextPage: () => void +} + +function usePagination(config: PaginationConfig): PaginationResult { + const {defaultPageIndex, onChange, pageSize, totalCount} = config + const [pageIndex, setPageIndex] = useState(defaultPageIndex ?? 0) + const pageCount = Math.ceil(totalCount / pageSize) + const pageStart = pageIndex * pageSize + const pageEnd = Math.min(pageIndex * pageSize + pageSize, totalCount - 1) + const hasNextPage = pageIndex + 1 < pageCount + const hasPreviousPage = pageIndex > 0 + + function selectPage(pageIndex: number) { + setPageIndex(pageIndex) + onChange?.({pageIndex}) + } + + function selectPreviousPage() { + if (hasPreviousPage) { + selectPage(pageIndex - 1) + } + } + + function selectNextPage() { + if (hasNextPage) { + selectPage(pageIndex + 1) + } + } + + return { + pageIndex, + pageStart, + pageEnd, + pageCount, + hasNextPage, + hasPreviousPage, + selectPage, + selectPreviousPage, + selectNextPage, + } +} + +interface WindowingConfig { + /** + * Provide an optional index to specify the default selected page + */ + defaultPageIndex?: number + + /** + * Specify the size of the window + */ + size: number + + /** + * Specify the total number of items within the collection + */ + pageCount: number +} + +/** + * A helper utility for "windowing" + */ +function useWindowing(config: WindowingConfig) { + const {defaultPageIndex, size, pageCount} = config + const [startIndex, setStartIndex] = useState(defaultPageIndex ?? 0) + const endIndex = Math.min(startIndex + size, pageCount - 1) + + console.log('useWindowing()', startIndex, endIndex, pageCount) + + function isWithinBounds(index: number) { + return index >= startIndex && index < endIndex + } + + function shiftWindowUp() { + setStartIndex(startIndex + 1) + } + + function shiftWindowDown() { + setStartIndex(startIndex - 1) + } + + function shiftStartTo(index: number) { + setStartIndex(index) + } + + function shiftEndTo(index: number) { + setStartIndex(index - size) + } + + return { + windowStartIndex: startIndex, + windowEndIndex: endIndex, + hasPagesBefore: startIndex > 0, + hasPagesAfter: endIndex < pageCount - 1, + isWithinBounds, + shiftWindowUp, + shiftWindowDown, + shiftStartTo, + shiftEndTo, + } +} diff --git a/src/DataTable/Table.tsx b/src/DataTable/Table.tsx index bfa899e2be2..e5f1437f514 100644 --- a/src/DataTable/Table.tsx +++ b/src/DataTable/Table.tsx @@ -94,11 +94,11 @@ const StyledTable = styled.table>` border-top-right-radius: var(--table-border-radius); } - .TableBody .TableRow:last-of-type .TableCell:first-child { + .TableOverflowWrapper:last-child & .TableBody .TableRow:last-of-type .TableCell:first-child { border-bottom-left-radius: var(--table-border-radius); } - .TableBody .TableRow:last-of-type .TableCell:last-child { + .TableOverflowWrapper:last-child & .TableBody .TableRow:last-of-type .TableCell:last-child { border-bottom-right-radius: var(--table-border-radius); } diff --git a/src/DataTable/index.ts b/src/DataTable/index.ts index b6a3ebc63b6..5ca274b0040 100644 --- a/src/DataTable/index.ts +++ b/src/DataTable/index.ts @@ -14,6 +14,7 @@ import { TableDivider, TableSkeleton, } from './Table' +import {Pagination} from './Pagination' const Table = Object.assign(TableImpl, { Container: TableContainer, @@ -28,6 +29,7 @@ const Table = Object.assign(TableImpl, { Row: TableRow, Cell: TableCell, CellPlaceholder: TableCellPlaceholder, + Pagination, }) export {DataTable, Table} diff --git a/src/DataTable/storybook/data.ts b/src/DataTable/storybook/data.ts new file mode 100644 index 00000000000..f61fa64b1e2 --- /dev/null +++ b/src/DataTable/storybook/data.ts @@ -0,0 +1,89 @@ +interface Repo { + id: number + name: string + type: 'public' | 'internal' + updatedAt: number + securityFeatures: { + dependabot: Array + codeScanning: Array + } +} + +const now = Date.now() +const repos: Array = [] + +for (let i = 0; i < 1000; i++) { + repos.push({ + id: i, + name: `Repository ${i + 1}`, + type: i % 3 === 0 ? 'internal' : 'public', + updatedAt: now - 1000 * 60 * 60 * 24 * i, + securityFeatures: { + dependabot: [], + codeScanning: [], + }, + }) +} + +function sleep(ms = 1000) { + return new Promise(resolve => { + setTimeout(resolve, ms) + }) +} + +function random(floor: number, ceiling: number): number { + return Math.floor(Math.random() * ceiling) + floor +} + +const Repo = { + async all() { + await sleep(random(2500, 3500)) + return repos + }, + async create() { + await sleep(random(1000, 5000)) + }, + async delete() { + await sleep(random(1000, 5000)) + }, + async paginate(offset: number, pageSize: number) { + await sleep(random(2500, 3500)) + return repos.slice(offset * pageSize, offset * pageSize + pageSize) + }, + async pageInfo(pageSize: number) { + return { + totalCount: repos.length, + totalPages: repos.length / pageSize, + } + }, +} + +const cache = new Map() + +export function fetchRepos(): Promise> { + if (!cache.has('/repos')) { + cache.set('/repos', Repo.all()) + } + return cache.get('/repos') +} + +export function fetchRepoPage(offset: number, pageSize: number): Promise> { + const url = new URL('/repos', 'https://api.dev') + url.searchParams.set('offset', `${offset}`) + url.searchParams.set('pageSize', `${pageSize}`) + const id = url.toString() + if (!cache.has(id)) { + cache.set(id, Repo.paginate(offset, pageSize)) + } + return cache.get(id) +} + +export function fetchRepoPageInfo(pageSize: number): Promise<{totalCount: number; totalPages: number}> { + const url = new URL('/repos/page-info', 'https://api.dev') + url.searchParams.set('pageSize', `${pageSize}`) + const id = url.toString() + if (!cache.has(id)) { + cache.set(id, Repo.pageInfo(pageSize)) + } + return cache.get(id) +} diff --git a/src/internal/components/ButtonReset.tsx b/src/internal/components/ButtonReset.tsx new file mode 100644 index 00000000000..bfbe0fca21f --- /dev/null +++ b/src/internal/components/ButtonReset.tsx @@ -0,0 +1,27 @@ +import styled from 'styled-components' +import sx, {SxProp} from '../../sx' + +/** + * Provides an unstyled button that can be used styled as-needed for components + */ +export const Button = styled.button` + padding: 0; + border: 0; + margin: 0; + display: inline-flex; + padding: 0; + border: 0; + appearance: none; + background: none; + cursor: pointer; + text-align: start; + font: inherit; + color: inherit; + align-items: center; + + &::-moz-focus-inner { + border: 0; + } + + ${sx} +` diff --git a/src/internal/components/VisuallyHidden.tsx b/src/internal/components/VisuallyHidden.tsx new file mode 100644 index 00000000000..f933770498e --- /dev/null +++ b/src/internal/components/VisuallyHidden.tsx @@ -0,0 +1,25 @@ +import styled from 'styled-components' +import sx, {SxProp} from '../../sx' + +/** + * Provides a component that implements the "visually hidden" technique. This is + * analagous to the common `sr-only` class. Children that are rendered inside + * this component will not be visible but will be available to screen readers. + * + * Note: if this component, or a descendant, has focus then this component will + * no longer be visually hidden. + * + * @see https://www.scottohara.me/blog/2023/03/21/visually-hidden-hack.html + */ +export const VisuallyHidden = styled.div` + &:not(:focus):not(:active):not(:focus-within) { + clip-path: inset(50%); + height: 1px; + overflow: hidden; + position: absolute; + white-space: nowrap; + width: 1px; + } + + ${sx} +` From 0a37cc1411f23e937f8e45db499739f44f93ec3a Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 19 Apr 2023 17:19:26 -0500 Subject: [PATCH 02/15] chore: check-in work --- src/DataTable/DataTable.features.stories.tsx | 7 +- src/DataTable/Pagination.tsx | 234 ++++++++----------- 2 files changed, 100 insertions(+), 141 deletions(-) diff --git a/src/DataTable/DataTable.features.stories.tsx b/src/DataTable/DataTable.features.stories.tsx index 119c87f065a..807c256f480 100644 --- a/src/DataTable/DataTable.features.stories.tsx +++ b/src/DataTable/DataTable.features.stories.tsx @@ -1421,7 +1421,12 @@ export const WithPagination = () => { }, ]} /> - + ) } diff --git a/src/DataTable/Pagination.tsx b/src/DataTable/Pagination.tsx index f0f7494beb6..e315041cd5d 100644 --- a/src/DataTable/Pagination.tsx +++ b/src/DataTable/Pagination.tsx @@ -125,8 +125,6 @@ export type PaginationProps = React.ComponentPropsWithoutRef<'nav'> & { totalCount: number } -const VISIBLE_PAGE_COUNT = 4 - export function Pagination({ 'aria-label': label, defaultPageIndex, @@ -151,39 +149,23 @@ export function Pagination({ pageSize, totalCount, }) - const { - windowStartIndex, - hasPagesBefore, - hasPagesAfter, - isWithinBounds, - shiftWindowDown, - shiftWindowUp, - shiftStartTo, - shiftEndTo, - } = useWindowing({ - defaultPageIndex, - size: VISIBLE_PAGE_COUNT, - pageCount, + + const totalPageCount = pageCount > 2 ? Math.min(pageCount - 2, 5) : 0 + const [offsetStartIndex, setOffsetStartIndex] = useState(() => { + if (pageIndex === 0) { + return 1 + } + return pageIndex }) - const pages = Array.from({length: VISIBLE_PAGE_COUNT}).map((_, i) => { - const offset = windowStartIndex + i - const active = offset === pageIndex - - return ( -
  • - -
    • - ) + const offsetEndIndex = offsetStartIndex + totalPageCount - 1 + const hasLeadingTruncation = offsetStartIndex >= 2 + const hasTrailingTruncation = pageCount - 1 - offsetEndIndex > 1 + + console.log({ + totalPageCount, + offsetStartIndex, + offsetEndIndex, + pageIndex, }) return ( @@ -201,9 +183,10 @@ export function Pagination({ } selectPreviousPage() - - if (!isWithinBounds(pageIndex - 1)) { - shiftWindowDown() + if (hasLeadingTruncation) { + if (pageIndex - 1 < offsetStartIndex + 1) { + setOffsetStartIndex(offsetStartIndex - 1) + } } }} > @@ -212,45 +195,75 @@ export function Pagination({  page - {hasPagesBefore ? ( - <> -
  • - -
    • - - + {pageCount > 0 ? ( +
  • + +
    • ) : null} - {pages} - {hasPagesAfter ? ( - <> - -
  • - -
    • - + {pageCount > 2 + ? Array.from({length: totalPageCount}).map((_, i) => { + if (i === 0 && hasLeadingTruncation) { + return ( + + ) + } + + if (i === totalPageCount - 1 && hasTrailingTruncation) { + return ( + + ) + } + + const page = offsetStartIndex + i + return ( +
  • + +
    • + ) + }) + : null} + {pageCount > 1 ? ( +
  • + +
    • ) : null}
  • -
    • - {pageCount > 0 ? ( + + + + +
      • - ) : null} - {pageCount > 2 - ? Array.from({length: totalPageCount}).map((_, i) => { - if (i === 0 && hasLeadingTruncation) { - return ( - - ) - } + {pageCount > 0 ? ( +
    • + +
      • + ) : null} + {pageCount > 2 + ? Array.from({length: totalPageCount}).map((_, i) => { + if (i === 0 && hasLeadingTruncation) { + return ( + + ) + } + + if (i === totalPageCount - 1 && hasTrailingTruncation) { + return ( + + ) + } - if (i === totalPageCount - 1 && hasTrailingTruncation) { + const page = offsetStartIndex + i return ( -
    • +
      • ) - } - - const page = offsetStartIndex + i - return ( -
    • - -
      • - ) - }) - : null} - {pageCount > 1 ? ( + }) + : null} + {pageCount > 1 ? ( +
    • + +
      • + ) : null}
      • - ) : null} -
    • - -
      • -
    -     + + +     ) } @@ -293,13 +297,18 @@ type RangeProps = { } function Range({pageStart, pageEnd, totalCount}: RangeProps) { + const start = pageStart + 1 + const end = pageEnd === totalCount - 1 ? totalCount : pageEnd return ( -

    - {pageStart + 1} -  through  - - {pageEnd === totalCount - 1 ? totalCount : pageEnd} of {totalCount} -

    + <> + +

    + {start} +  through  + + {end} of {totalCount} +

    + ) } diff --git a/src/internal/components/LiveRegion.tsx b/src/internal/components/LiveRegion.tsx new file mode 100644 index 00000000000..c67ebfb081f --- /dev/null +++ b/src/internal/components/LiveRegion.tsx @@ -0,0 +1,60 @@ +import React from 'react' +import {VisuallyHidden} from './VisuallyHidden' + +type LiveRegionContext = { + announce: (message: string) => void + message: string +} + +const LiveRegionContext = React.createContext(null) + +function useLiveRegion() { + const context = React.useContext(LiveRegionContext) + if (!context) { + throw new Error('useLiveRegion() must be used within a ') + } + return context +} + +function LiveRegion({children}: React.PropsWithChildren<{}>) { + const [message, setMessage] = React.useState('') + const value = React.useMemo(() => { + return { + announce: setMessage, + message, + } + }, [message]) + + return {children} +} + +function LiveRegionOutlet() { + const liveRegion = useLiveRegion() + return ( + + {liveRegion.message ?? null} + + ) +} + +function Message({value}: {value: string}) { + const liveRegion = useLiveRegion() + const committedRef = React.useRef(false) + + React.useEffect(() => { + if (committedRef.current === true) { + liveRegion.announce(value) + } + }, [value]) + + React.useEffect(() => { + committedRef.current = true + return () => { + committedRef.current = false + } + }, []) + + return null +} + +export {LiveRegion, LiveRegionOutlet, Message, useLiveRegion} From 354cadd3b152f5ac2f98e90684d979ad37d9f13e Mon Sep 17 00:00:00 2001 From: Josh Black Date: Thu, 20 Apr 2023 12:53:26 -0500 Subject: [PATCH 05/15] refator: update ul to ol --- src/DataTable/Pagination.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/DataTable/Pagination.tsx b/src/DataTable/Pagination.tsx index fec662ad088..7c46944df61 100644 --- a/src/DataTable/Pagination.tsx +++ b/src/DataTable/Pagination.tsx @@ -167,7 +167,7 @@ export function Pagination({ -
      +
        2. -
    +     ) From 935e80d9a1219274627cbf7dcc80f7671926a46e Mon Sep 17 00:00:00 2001 From: Josh Black Date: Thu, 20 Apr 2023 12:57:17 -0500 Subject: [PATCH 06/15] refactor: add page prefix to page steps --- src/DataTable/Pagination.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/DataTable/Pagination.tsx b/src/DataTable/Pagination.tsx index 7c46944df61..ed58bc27598 100644 --- a/src/DataTable/Pagination.tsx +++ b/src/DataTable/Pagination.tsx @@ -205,6 +205,7 @@ export function Pagination({ } }} > + Page  {1} @@ -239,6 +240,7 @@ export function Pagination({ selectPage(page) }} > + Page  {page + 1} @@ -257,6 +259,7 @@ export function Pagination({ setOffsetStartIndex(pageCount - 1 - totalPageCount) }} > + Page  {pageCount} From 761e3dd9b74ff35ead13635574e8064c57d7f4eb Mon Sep 17 00:00:00 2001 From: Josh Black Date: Thu, 20 Apr 2023 14:53:47 -0500 Subject: [PATCH 07/15] chore: address eslint violations --- src/DataTable/DataTable.features.stories.tsx | 2 -- src/DataTable/Pagination.tsx | 3 +-- src/internal/components/LiveRegion.tsx | 11 ++++++++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/DataTable/DataTable.features.stories.tsx b/src/DataTable/DataTable.features.stories.tsx index 2c53cfb9604..568f3d8de6d 100644 --- a/src/DataTable/DataTable.features.stories.tsx +++ b/src/DataTable/DataTable.features.stories.tsx @@ -14,7 +14,6 @@ import RelativeTime from '../RelativeTime' import VisuallyHidden from '../_VisuallyHidden' import {createColumnHelper} from './column' import {repos} from './storybook/data' -import {LiveRegion, LiveRegionOutlet, Message} from '../internal/components/LiveRegion' export default { title: 'Components/DataTable/Features', @@ -1370,7 +1369,6 @@ export const WithPagination = () => { const start = pageIndex * pageSize const end = start + pageSize const rows = repos.slice(start, end) - const totalCount = repos.length return ( diff --git a/src/DataTable/Pagination.tsx b/src/DataTable/Pagination.tsx index ed58bc27598..063833cc8bb 100644 --- a/src/DataTable/Pagination.tsx +++ b/src/DataTable/Pagination.tsx @@ -3,9 +3,8 @@ import React, {useState} from 'react' import styled from 'styled-components' import {get} from '../constants' import {Button} from '../internal/components/ButtonReset' -import {Message} from '../internal/components/LiveRegion' +import {LiveRegion, LiveRegionOutlet, Message} from '../internal/components/LiveRegion' import {VisuallyHidden} from '../internal/components/VisuallyHidden' -import {LiveRegion, LiveRegionOutlet, useLiveRegion} from '../internal/components/LiveRegion' const StyledPagination = styled.nav` display: flex; diff --git a/src/internal/components/LiveRegion.tsx b/src/internal/components/LiveRegion.tsx index c67ebfb081f..9c648843791 100644 --- a/src/internal/components/LiveRegion.tsx +++ b/src/internal/components/LiveRegion.tsx @@ -16,7 +16,7 @@ function useLiveRegion() { return context } -function LiveRegion({children}: React.PropsWithChildren<{}>) { +function LiveRegion({children}: React.PropsWithChildren) { const [message, setMessage] = React.useState('') const value = React.useMemo(() => { return { @@ -32,18 +32,23 @@ function LiveRegionOutlet() { const liveRegion = useLiveRegion() return ( - {liveRegion.message ?? null} + {liveRegion.message} ) } function Message({value}: {value: string}) { const liveRegion = useLiveRegion() + const savedLiveRegion = React.useRef(liveRegion) const committedRef = React.useRef(false) + React.useEffect(() => { + savedLiveRegion.current = liveRegion + }, [liveRegion]) + React.useEffect(() => { if (committedRef.current === true) { - liveRegion.announce(value) + savedLiveRegion.current.announce(value) } }, [value]) From d2905c5ce4e6c35b5db5c0b315f8c1478495c039 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Fri, 21 Apr 2023 09:47:18 -0500 Subject: [PATCH 08/15] refactor: update render for DataTable Pagionation --- src/DataTable/Pagination.tsx | 114 ++++++++++++++---------- src/internal/components/ButtonReset.tsx | 2 +- 2 files changed, 68 insertions(+), 48 deletions(-) diff --git a/src/DataTable/Pagination.tsx b/src/DataTable/Pagination.tsx index 063833cc8bb..c4ab29895f1 100644 --- a/src/DataTable/Pagination.tsx +++ b/src/DataTable/Pagination.tsx @@ -82,7 +82,7 @@ const StyledPagination = styled.nav` color: ${get('colors.fg.onEmphasis')}; } - .TablePaginationTruncated { + .TablePaginationTruncationStep { display: flex; align-items: center; justify-content: center; @@ -126,6 +126,12 @@ export type PaginationProps = Omit, 'onCha totalCount: number } +/** + * Specifies the maximum number of items in between the first and last page, + * including truncated steps + */ +const MAX_TRUNCATED_STEP_COUNT = 7 + export function Pagination({ 'aria-label': label, defaultPageIndex, @@ -150,14 +156,16 @@ export function Pagination({ pageSize, totalCount, }) - const totalPageCount = pageCount > 2 ? Math.min(pageCount - 2, 7) : 0 + const truncatedPageCount = pageCount > 2 ? Math.min(pageCount - 2, MAX_TRUNCATED_STEP_COUNT) : 0 const [offsetStartIndex, setOffsetStartIndex] = useState(() => { + // Set the offset start index to the page at index 1 since we will have the + // first page already visible if (pageIndex === 0) { return 1 } return pageIndex }) - const offsetEndIndex = offsetStartIndex + totalPageCount - 1 + const offsetEndIndex = offsetStartIndex + truncatedPageCount - 1 const hasLeadingTruncation = offsetStartIndex >= 2 const hasTrailingTruncation = pageCount - 1 - offsetEndIndex > 1 @@ -167,7 +175,7 @@ export function Pagination({
      -
    1. + -
      2. + {pageCount > 0 ? ( -
    2. - -
      3. + + ) : null} {pageCount > 2 - ? Array.from({length: totalPageCount}).map((_, i) => { + ? Array.from({length: truncatedPageCount}).map((_, i) => { if (i === 0 && hasLeadingTruncation) { - return ( - - ) + return } - if (i === totalPageCount - 1 && hasTrailingTruncation) { - return ( - - ) + if (i === truncatedPageCount - 1 && hasTrailingTruncation) { + return } const page = offsetStartIndex + i return ( -
    3. - -
      4. + + ) }) : null} {pageCount > 1 ? ( -
    4. - -
      5. + + ) : null} -
    5. + -
      6. +
    @@ -314,6 +302,38 @@ function Range({pageStart, pageEnd, totalCount}: RangeProps) { ) } +function TruncationStep() { + return ( + + ) +} + +function Step({children}: React.PropsWithChildren) { + return
  • {children}
    • +} + +type PageProps = React.PropsWithChildren<{ + active: boolean + onClick: () => void +}> + +function Page({active, children, onClick}: PageProps) { + return ( + + ) +} + interface PaginationState { /** * The index of currently selected page diff --git a/src/internal/components/ButtonReset.tsx b/src/internal/components/ButtonReset.tsx index bfbe0fca21f..82eb48a14d1 100644 --- a/src/internal/components/ButtonReset.tsx +++ b/src/internal/components/ButtonReset.tsx @@ -2,7 +2,7 @@ import styled from 'styled-components' import sx, {SxProp} from '../../sx' /** - * Provides an unstyled button that can be used styled as-needed for components + * Provides an unstyled button that can be styled as-needed for components */ export const Button = styled.button` padding: 0; From 87e1cf7614a33820941e93fe3ce930c327b56654 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 25 Apr 2023 13:09:43 -0500 Subject: [PATCH 09/15] test(DataTable): add tests for Pagination --- src/DataTable/DataTable.features.stories.tsx | 145 +++++---- src/DataTable/Pagination.tsx | 38 ++- src/DataTable/__tests__/Pagination.test.tsx | 294 +++++++++++++++++++ 3 files changed, 393 insertions(+), 84 deletions(-) create mode 100644 src/DataTable/__tests__/Pagination.test.tsx diff --git a/src/DataTable/DataTable.features.stories.tsx b/src/DataTable/DataTable.features.stories.tsx index 568f3d8de6d..450515f0774 100644 --- a/src/DataTable/DataTable.features.stories.tsx +++ b/src/DataTable/DataTable.features.stories.tsx @@ -1363,84 +1363,79 @@ export const WithRightAlignedColumns = () => { export const WithPagination = () => { const pageSize = 10 + const [pageIndex, setPageIndex] = React.useState(0) + const start = pageIndex * pageSize + const end = start + pageSize + const rows = repos.slice(start, end) - function Story() { - const [pageIndex, setPageIndex] = React.useState(0) - const start = pageIndex * pageSize - const end = start + pageSize - const rows = repos.slice(start, end) - - return ( - - - Repositories - - - A subtitle could appear here to give extra context to the data. - - { - return - }, + return ( + + + Repositories + + + A subtitle could appear here to give extra context to the data. + + { + return }, - { - header: 'Updated', - field: 'updatedAt', - renderCell: row => { - return - }, + }, + { + header: 'Updated', + field: 'updatedAt', + renderCell: row => { + return }, - { - header: 'Dependabot', - field: 'securityFeatures.dependabot', - renderCell: row => { - return row.securityFeatures.dependabot.length > 0 ? ( - - {row.securityFeatures.dependabot.map(feature => { - return - })} - - ) : null - }, + }, + { + header: 'Dependabot', + field: 'securityFeatures.dependabot', + renderCell: row => { + return row.securityFeatures.dependabot.length > 0 ? ( + + {row.securityFeatures.dependabot.map(feature => { + return + })} + + ) : null }, - { - header: 'Code scanning', - field: 'securityFeatures.codeScanning', - renderCell: row => { - return row.securityFeatures.codeScanning.length > 0 ? ( - - {row.securityFeatures.codeScanning.map(feature => { - return - })} - - ) : null - }, + }, + { + header: 'Code scanning', + field: 'securityFeatures.codeScanning', + renderCell: row => { + return row.securityFeatures.codeScanning.length > 0 ? ( + + {row.securityFeatures.codeScanning.map(feature => { + return + })} + + ) : null }, - ]} - /> - { - setPageIndex(pageIndex) - }} - /> - - ) - } - - return + }, + ]} + /> + { + setPageIndex(pageIndex) + }} + /> + + ) } diff --git a/src/DataTable/Pagination.tsx b/src/DataTable/Pagination.tsx index c4ab29895f1..967ef61f07e 100644 --- a/src/DataTable/Pagination.tsx +++ b/src/DataTable/Pagination.tsx @@ -5,6 +5,7 @@ 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' const StyledPagination = styled.nav` display: flex; @@ -172,7 +173,7 @@ export function Pagination({ return ( - +
      @@ -225,7 +226,7 @@ export function Pagination({ const page = offsetStartIndex + i return ( - + { @@ -334,14 +335,14 @@ function Page({active, children, onClick}: PageProps) { ) } -interface PaginationState { +type PaginationState = { /** * The index of currently selected page */ pageIndex: number } -interface PaginationConfig { +type PaginationConfig = { /** * Provide an optional index to specify the default selected page */ @@ -364,7 +365,7 @@ interface PaginationConfig { totalCount: number } -interface PaginationResult { +type PaginationResult = { /** * The index for the currently selected page */ @@ -417,16 +418,35 @@ interface PaginationResult { function usePagination(config: PaginationConfig): PaginationResult { const {defaultPageIndex, onChange, pageSize, totalCount} = config - const [pageIndex, setPageIndex] = useState(defaultPageIndex ?? 0) const pageCount = Math.ceil(totalCount / pageSize) + const [pageIndex, setPageIndex] = useState(() => { + if (defaultPageIndex !== undefined) { + if (defaultPageIndex >= 0 && defaultPageIndex < pageCount) { + return defaultPageIndex + } + + warning( + true, + ' expected `defaultPageIndex` to be less than the ' + + 'total number of pages. Instead, received a `defaultPageIndex` ' + + 'of %s with %s total pages.', + defaultPageIndex, + pageCount, + ) + } + + return 0 + }) const pageStart = pageIndex * pageSize const pageEnd = Math.min(pageIndex * pageSize + pageSize, totalCount - 1) const hasNextPage = pageIndex + 1 < pageCount const hasPreviousPage = pageIndex > 0 - function selectPage(pageIndex: number) { - setPageIndex(pageIndex) - onChange?.({pageIndex}) + function selectPage(newPageIndex: number) { + if (pageIndex !== newPageIndex) { + setPageIndex(newPageIndex) + onChange?.({pageIndex: newPageIndex}) + } } function selectPreviousPage() { diff --git a/src/DataTable/__tests__/Pagination.test.tsx b/src/DataTable/__tests__/Pagination.test.tsx new file mode 100644 index 00000000000..f05abfce28c --- /dev/null +++ b/src/DataTable/__tests__/Pagination.test.tsx @@ -0,0 +1,294 @@ +import React from 'react' +import {Pagination} from '../Pagination' +import {render, screen} from '@testing-library/react' +import userEvent from '@testing-library/user-event' + +describe('Table.Pagination', () => { + it('should render a navigation landmark with an accessible name provided by `aria-label`', () => { + render() + expect( + screen.getByRole('navigation', { + name: 'test', + }), + ).toBeInTheDocument() + }) + + it('should set the initial selected page to the first page', () => { + render() + expect(getCurrentPage()).toEqual(getFirstPage()) + }) + + it('should initialize `pageIndex` to `defaultPageIndex`', () => { + render() + expect(getCurrentPage()).toEqual(getLastPage()) + }) + + it('should warn if `defaultPageIndex` is not a valid `pageIndex`', () => { + const spy = jest.spyOn(console, 'warn').mockImplementation(() => {}) + render() + expect(spy).toHaveBeenCalledWith( + 'Warning:', + expect.stringMatching( + ' expected `defaultPageIndex` to be less than the total number of pages. Instead, received a `defaultPageIndex` of 4 with 4 total pages.', + ), + ) + }) + + it('should set the `id` prop on the rendered navigation landmark', () => { + render() + expect( + screen.getByRole('navigation', { + name: 'test-label', + }), + ).toHaveAttribute('id', 'test-id') + }) + + describe('with one page', () => { + it('should only display one page', () => { + render() + + expect(getPages()).toHaveLength(1) + expect(getCurrentPage()).toEqual(getPage(0)) + expect(getPageRange()).toEqual('1 through 25 of 25') + }) + + it('should not call `onChange` when a page or action is interacted with', async () => { + const user = userEvent.setup() + const onChange = jest.fn() + + render() + + expect(getPages()).toHaveLength(1) + expect(getCurrentPage()).toEqual(getPage(0)) + expect(getPageRange()).toEqual('1 through 25 of 25') + + await user.click(getPage(0)) + expect(onChange).not.toHaveBeenCalled() + + await user.click(getNextPage()) + expect(onChange).not.toHaveBeenCalled() + + await user.click(getPreviousPage()) + expect(onChange).not.toHaveBeenCalled() + }) + }) + + describe('with two pages', () => { + it('should display two pages', () => { + render() + + expect(getPages()).toHaveLength(2) + expect(getCurrentPage()).toEqual(getPage(0)) + expect(getPageRange()).toEqual('1 through 25 of 50') + }) + + it('should call `onChange` when clicking on pages', async () => { + const user = userEvent.setup() + const onChange = jest.fn() + + render() + + await user.click(getPage(1)) + expect(onChange).toHaveBeenCalledWith({ + pageIndex: 1, + }) + expect(getPageRange()).toEqual('26 through 50 of 50') + + await user.click(getPage(0)) + expect(onChange).toHaveBeenCalledWith({ + pageIndex: 0, + }) + expect(getPageRange()).toEqual('1 through 25 of 50') + }) + + it('should call `onChange` when using the keyboard to interact with pages', async () => { + const user = userEvent.setup() + const onChange = jest.fn() + + render() + + await user.tab() + expect(getPreviousPage()).toHaveFocus() + + await user.tab() + expect(getFirstPage()).toHaveFocus() + + await user.tab() + expect(getPage(1)).toHaveFocus() + + await user.keyboard('{Enter}') + expect(onChange).toHaveBeenCalledWith({ + pageIndex: 1, + }) + expect(getPageRange()).toEqual('26 through 50 of 50') + + await user.tab({shift: true}) + expect(getPage(0)).toHaveFocus() + + await user.keyboard('{Enter}') + expect(onChange).toHaveBeenCalledWith({ + pageIndex: 0, + }) + expect(getPageRange()).toEqual('1 through 25 of 50') + }) + + it('should call `onChange` when clicking on previous or next', async () => { + const user = userEvent.setup() + const onChange = jest.fn() + + render() + + await user.click(getPreviousPage()) + expect(onChange).not.toHaveBeenCalled() + + await user.click(getNextPage()) + expect(onChange).toHaveBeenCalledWith({ + pageIndex: 1, + }) + + await user.click(getPreviousPage()) + expect(onChange).toHaveBeenCalledWith({ + pageIndex: 0, + }) + }) + + it('should call `onChange` when using the keyboard to interact with previous or next', async () => { + const user = userEvent.setup() + const onChange = jest.fn() + + render() + + await user.tab() + expect(getPreviousPage()).toHaveFocus() + + await user.keyboard('{Enter}') + expect(onChange).not.toHaveBeenCalled() + + await user.tab() + await user.tab() + await user.tab() + expect(getNextPage()).toHaveFocus() + + await user.keyboard('{Enter}') + expect(onChange).toHaveBeenCalledWith({ + pageIndex: 1, + }) + + await user.tab({shift: true}) + await user.tab({shift: true}) + await user.tab({shift: true}) + expect(getPreviousPage()).toHaveFocus() + + await user.keyboard('{Enter}') + expect(onChange).toHaveBeenCalledWith({ + pageIndex: 0, + }) + }) + }) + + describe('with three or more pages', () => { + it('should have trailing truncation if there are more than two pages between the last page and the last visible page', () => { + render() + + const lastPage = getLastPage() + const previousStep = getPreviousStep(lastPage) + + expect(previousStep).toHaveAttribute('aria-hidden', 'true') + expect(previousStep).toHaveTextContent('…') + }) + + it('should have leading truncation if there are more than two pages between the first page and the first visible page', () => { + render() + + const firstPage = getFirstPage() + const nextStep = getNextStep(firstPage) + + expect(nextStep).toHaveAttribute('aria-hidden', 'true') + expect(nextStep).toHaveTextContent('…') + }) + + it('should have leading and trailing truncation if there are more than two pages between visible pages and first and last pages', () => { + render() + + const firstPage = getFirstPage() + const nextStep = getNextStep(firstPage) + + expect(nextStep).toHaveAttribute('aria-hidden', 'true') + expect(nextStep).toHaveTextContent('…') + + const lastPage = getLastPage() + const previousStep = getPreviousStep(lastPage) + + expect(previousStep).toHaveAttribute('aria-hidden', 'true') + expect(previousStep).toHaveTextContent('…') + }) + }) +}) + +function getPages() { + return screen.getAllByRole('button').filter(button => { + return button.textContent?.includes('Page') + }) +} + +function getPage(index: number) { + const pages = getPages() + return pages[index] +} + +function getCurrentPage() { + const page = getPages().find(button => { + return button.hasAttribute('aria-current') + }) + if (page) { + return page + } + + throw new Error('Unable to find a button with `aria-current`') +} + +function getPreviousPage() { + return screen.getByRole('button', { + name: 'Previous page', + }) +} + +function getNextPage() { + return screen.getByRole('button', { + name: 'Next page', + }) +} + +function getFirstPage() { + const pages = getPages() + return pages[0] +} + +function getLastPage() { + const pages = getPages() + return pages[pages.length - 1] +} + +function getPageRange() { + const element = document.querySelector('.TablePaginationRange') + if (element && element.textContent) { + return element.textContent.replace('‒', '').replaceAll(' ', ' ') + } + throw new Error('Unable to find the text for the page range component') +} + +function getPreviousStep(element: HTMLElement) { + const step = element.closest('li') + if (step) { + return step.previousElementSibling + } + throw new Error(`Unable to find previous step`) +} + +function getNextStep(element: HTMLElement) { + const step = element.closest('li') + if (step) { + return step.nextElementSibling + } + throw new Error(`Unable to find next step`) +} From 59203f6402f5b67db24f8ce62e4779c00c245acb Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 25 Apr 2023 13:53:59 -0500 Subject: [PATCH 10/15] chore: add docs and changeset --- .changeset/small-pumas-relate.md | 5 +++++ src/DataTable/DataTable.docs.json | 34 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 .changeset/small-pumas-relate.md diff --git a/.changeset/small-pumas-relate.md b/.changeset/small-pumas-relate.md new file mode 100644 index 00000000000..cba119c8037 --- /dev/null +++ b/.changeset/small-pumas-relate.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Add support for Pagination in DataTable diff --git a/src/DataTable/DataTable.docs.json b/src/DataTable/DataTable.docs.json index 5a791bade9b..01029237ed9 100644 --- a/src/DataTable/DataTable.docs.json +++ b/src/DataTable/DataTable.docs.json @@ -33,6 +33,9 @@ }, { "id": "components-datatable-features--with-loading" + }, + { + "id": "components-datatable-features--with-pagination" } ], "props": [ @@ -192,6 +195,37 @@ "description": "Optionally specify the number of rows which should be included in the skeleton state of the component" } ] + }, + { + "name": "Table.Pagination", + "props": [ + { + "name": "aria-label", + "type": "string", + "required": true + }, + { + "name": "defaultPageIndex", + "type": "string" + }, + { + "name": "id", + "type": "string" + }, + { + "name": "onChange", + "type": "({ pageIndex }: { pageIndex: number }) => void" + }, + { + "name": "pageSize", + "type": "number" + }, + { + "name": "totalCount", + "type": "number", + "required": true + } + ] } ] } From 42ef00635c365385f522b79d97bc6bbd506fbfc0 Mon Sep 17 00:00:00 2001 From: joshblack Date: Tue, 25 Apr 2023 18:56:00 +0000 Subject: [PATCH 11/15] Update generated/components.json --- generated/components.json | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/generated/components.json b/generated/components.json index 653506bd7fc..65cb65ab07d 100644 --- a/generated/components.json +++ b/generated/components.json @@ -1499,6 +1499,10 @@ { "id": "components-datatable-features--with-loading", "code": "() => {\n const [loading] = React.useState(true)\n return (\n \n \n Repositories\n \n \n A subtitle could appear here to give extra context to the data.\n \n {loading ? (\n \n ) : (\n \n )}\n \n )\n}" + }, + { + "id": "components-datatable-features--with-pagination", + "code": "() => {\n const pageSize = 10\n const [pageIndex, setPageIndex] = React.useState(0)\n const start = pageIndex * pageSize\n const end = start + pageSize\n const rows = repos.slice(start, end)\n return (\n \n \n Repositories\n \n \n A subtitle could appear here to give extra context to the data.\n \n {\n return \n },\n },\n {\n header: 'Updated',\n field: 'updatedAt',\n renderCell: (row) => {\n return \n },\n },\n {\n header: 'Dependabot',\n field: 'securityFeatures.dependabot',\n renderCell: (row) => {\n return row.securityFeatures.dependabot.length > 0 ? (\n \n {row.securityFeatures.dependabot.map((feature) => {\n return \n })}\n \n ) : null\n },\n },\n {\n header: 'Code scanning',\n field: 'securityFeatures.codeScanning',\n renderCell: (row) => {\n return row.securityFeatures.codeScanning.length > 0 ? (\n \n {row.securityFeatures.codeScanning.map((feature) => {\n return \n })}\n \n ) : null\n },\n },\n ]}\n />\n {\n setPageIndex(pageIndex)\n }}\n />\n \n )\n}" } ], "props": [ @@ -1658,6 +1662,37 @@ "description": "Optionally specify the number of rows which should be included in the skeleton state of the component" } ] + }, + { + "name": "Table.Pagination", + "props": [ + { + "name": "aria-label", + "type": "string", + "required": true + }, + { + "name": "defaultPageIndex", + "type": "string" + }, + { + "name": "id", + "type": "string" + }, + { + "name": "onChange", + "type": "({ pageIndex }: { pageIndex: number }) => void" + }, + { + "name": "pageSize", + "type": "number" + }, + { + "name": "totalCount", + "type": "number", + "required": true + } + ] } ] }, From 27c436df2ebfc1540b6ba6cdd922027c65cfd302 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 26 Apr 2023 10:21:40 -0500 Subject: [PATCH 12/15] chore: add flex-wrap and gap to pagination --- src/DataTable/Pagination.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/DataTable/Pagination.tsx b/src/DataTable/Pagination.tsx index 967ef61f07e..2fa9ebefaec 100644 --- a/src/DataTable/Pagination.tsx +++ b/src/DataTable/Pagination.tsx @@ -11,6 +11,7 @@ const StyledPagination = styled.nav` display: flex; align-items: center; justify-content: space-between; + column-gap: 1rem; width: 100%; grid-area: footer; padding: 0.5rem 1rem; @@ -27,8 +28,9 @@ const StyledPagination = styled.nav` .TablePaginationSteps { display: flex; - list-style: none; align-items: center; + flex-wrap: wrap; + list-style: none; color: ${get('colors.fg.default')}; font-size: 0.875rem; margin: 0; From 5a33122406f77a1eb186a198584fe5b8db77ccde Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 26 Apr 2023 12:30:49 -0500 Subject: [PATCH 13/15] chore: address eslint violations --- src/DataTable/Pagination.tsx | 1 + src/DataTable/__tests__/Pagination.test.tsx | 1 + 2 files changed, 2 insertions(+) diff --git a/src/DataTable/Pagination.tsx b/src/DataTable/Pagination.tsx index 2fa9ebefaec..09584912213 100644 --- a/src/DataTable/Pagination.tsx +++ b/src/DataTable/Pagination.tsx @@ -429,6 +429,7 @@ function usePagination(config: PaginationConfig): PaginationResult { warning( true, + // eslint-disable-next-line github/unescaped-html-literal ' expected `defaultPageIndex` to be less than the ' + 'total number of pages. Instead, received a `defaultPageIndex` ' + 'of %s with %s total pages.', diff --git a/src/DataTable/__tests__/Pagination.test.tsx b/src/DataTable/__tests__/Pagination.test.tsx index f05abfce28c..83254604485 100644 --- a/src/DataTable/__tests__/Pagination.test.tsx +++ b/src/DataTable/__tests__/Pagination.test.tsx @@ -29,6 +29,7 @@ describe('Table.Pagination', () => { expect(spy).toHaveBeenCalledWith( 'Warning:', expect.stringMatching( + // eslint-disable-next-line github/unescaped-html-literal ' expected `defaultPageIndex` to be less than the total number of pages. Instead, received a `defaultPageIndex` of 4 with 4 total pages.', ), ) From 2d00d9b7cce7344afc313e11775de6cba2d09eb5 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 26 Apr 2023 16:11:33 -0500 Subject: [PATCH 14/15] feat: add trailing ellipsis for truncation pages --- src/DataTable/Pagination.tsx | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/DataTable/Pagination.tsx b/src/DataTable/Pagination.tsx index 09584912213..34106b8476c 100644 --- a/src/DataTable/Pagination.tsx +++ b/src/DataTable/Pagination.tsx @@ -213,6 +213,7 @@ export function Pagination({ }} > {1} + {hasLeadingTruncation ? … : null} ) : null} @@ -236,6 +237,9 @@ export function Pagination({ }} > {page + 1} + {i === truncatedPageCount - 2 && hasTrailingTruncation ? ( + … + ) : null} ) From 6b92533ed30c753a31b70c9a5798870f62d121d6 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Thu, 27 Apr 2023 10:02:22 -0500 Subject: [PATCH 15/15] fix(LiveRegion): defer announcement to after 750ms --- src/internal/components/LiveRegion.tsx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/internal/components/LiveRegion.tsx b/src/internal/components/LiveRegion.tsx index 9c648843791..1f3a0433ac2 100644 --- a/src/internal/components/LiveRegion.tsx +++ b/src/internal/components/LiveRegion.tsx @@ -47,8 +47,14 @@ function Message({value}: {value: string}) { }, [liveRegion]) React.useEffect(() => { - if (committedRef.current === true) { + if (committedRef.current !== true) { + return + } + const timeoutId = setTimeout(() => { savedLiveRegion.current.announce(value) + }, 750) + return () => { + clearTimeout(timeoutId) } }, [value])