diff --git a/src/DataTable/TableHeaderCell.tsx b/src/DataTable/TableHeaderCell.tsx index a314dadcd85..8b157bf75fe 100644 --- a/src/DataTable/TableHeaderCell.tsx +++ b/src/DataTable/TableHeaderCell.tsx @@ -31,7 +31,7 @@ interface TableHeaderCellProps { render: (type: 'Header') => React.ReactNode; /** Indicates whether the column is sorted in descending order */ isSortedDesc?: boolean; - /** Gets props related to sorting that will be passed to th */ + /** Gets props related to sorting that will be passed to the sort button */ getSortByToggleProps?: (...args: any[]) => Record; /** Indicates whether a column is sortable */ canSort?: boolean; @@ -48,14 +48,44 @@ function TableHeaderCell({ isSortedDesc = false, headerClassName, }: TableHeaderCellProps) { + const headerProps = getHeaderProps(); const toggleProps = canSort && getSortByToggleProps ? getSortByToggleProps() : {}; + // Per WAI-ARIA APG sortable table guidance, only the actively sorted header should expose aria-sort. + let ariaSort: React.AriaAttributes['aria-sort']; + if (isSorted) { + ariaSort = isSortedDesc ? 'descending' : 'ascending'; + } + const headerContentClassName = classNames('pgn__data-table-header-content', headerClassName); return ( - - - {render('Header')} - {canSort && } - + + {canSort ? ( + + ) : ( + + {render('Header')} + + )} ); } diff --git a/src/DataTable/index.scss b/src/DataTable/index.scss index 4292f5bde38..3a52cf35b4c 100644 --- a/src/DataTable/index.scss +++ b/src/DataTable/index.scss @@ -146,6 +146,36 @@ background-color: var(--pgn-color-light-300); padding: var(--pgn-spacing-data-table-padding-cell); text-align: start; + + // The button owns sortable header padding so the full visible header remains the sort hit target. + &.pgn__data-table-sortable-header { + padding: 0; + position: relative; + } + } + + .pgn__data-table-header-content { + display: flex; + align-items: center; + } + + .pgn__data-table-sort-button { + background: transparent; + border: 0; + color: inherit; + cursor: pointer; + font: inherit; + padding: var(--pgn-spacing-data-table-padding-cell); + text-align: inherit; + width: 100%; + box-sizing: border-box; + + // Extend pointer hit testing to the full sortable th when row height is set by another header cell. + &::before { + content: ""; + position: absolute; + inset: 0; + } } td { diff --git a/src/DataTable/tests/DataTable.test.jsx b/src/DataTable/tests/DataTable.test.jsx index d9131aa41b5..e6a95bd3a67 100644 --- a/src/DataTable/tests/DataTable.test.jsx +++ b/src/DataTable/tests/DataTable.test.jsx @@ -1,5 +1,7 @@ import React, { useContext } from 'react'; -import { render, screen, waitFor } from '@testing-library/react'; +import { + render, screen, waitFor, within, +} from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import * as reactTable from 'react-table'; import { IntlProvider } from 'react-intl'; @@ -145,6 +147,36 @@ describe('', () => { expect(screen.getAllByRole('row')).toHaveLength(props.data.length + 1); // (need + 1 to include header row) }); + it('sorts rows when activating a sortable header button', async () => { + const getNameColumnValues = () => screen.getAllByRole('row') + .slice(1) + .map(row => within(row).getAllByRole('cell')[0].textContent); + + render(); + + expect(getNameColumnValues()).toEqual([ + 'Lil Bub', + 'Grumpy Cat', + 'Smoothie', + 'Maru', + 'Keyboard Cat', + 'Long Cat', + 'Zeno', + ]); + + await userEvent.click(screen.getByRole('button', { name: 'Name' })); + + await waitFor(() => expect(getNameColumnValues()).toEqual([ + 'Grumpy Cat', + 'Keyboard Cat', + 'Lil Bub', + 'Long Cat', + 'Maru', + 'Smoothie', + 'Zeno', + ])); + }); + it('displays a table footer', () => { render(); expect(screen.getByTestId('table-footer')).toBeInTheDocument(); diff --git a/src/DataTable/tests/TableHeaderCell.test.jsx b/src/DataTable/tests/TableHeaderCell.test.jsx index 89ac45f1841..7b656ec9343 100644 --- a/src/DataTable/tests/TableHeaderCell.test.jsx +++ b/src/DataTable/tests/TableHeaderCell.test.jsx @@ -1,9 +1,10 @@ import React from 'react'; import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import TableHeaderCell from '../TableHeaderCell'; -const sortByToggleProps = { foo: 'bar' }; +const sortByToggleProps = { 'data-sort-prop': 'bar' }; const props = { getHeaderProps: () => ({ className: 'red' }), render: () => 'Title', @@ -20,21 +21,41 @@ function FakeTable({ ...rest }) { describe('', () => { describe('unsorted', () => { - render(); - const cell = screen.getByRole('columnheader'); - const innerCell = cell.firstChild; - it('renders a table header cell', () => { + render(); + const cell = screen.getByRole('columnheader'); expect(cell).toBeInTheDocument(); }); it('adds props to the cell', () => { + render(); + const cell = screen.getByRole('columnheader'); expect(cell.className).toBe('red'); }); + it('adds column scope to the cell', () => { + render(); + const cell = screen.getByRole('columnheader'); + expect(cell).toHaveAttribute('scope', 'col'); + }); + it('adds the headerClassName to inner span', () => { + render(); + const cell = screen.getByRole('columnheader'); + const innerCell = cell.firstChild; expect(innerCell.className).toContain(props.headerClassName); }); + + it('does not render a button for non-sortable headers', () => { + render(); + expect(screen.queryByRole('button')).not.toBeInTheDocument(); + }); + + it('does not add sortable header styling to non-sortable headers', () => { + render(); + const cell = screen.getByRole('columnheader'); + expect(cell).not.toHaveClass('pgn__data-table-sortable-header'); + }); }); describe('with sorting', () => { @@ -56,10 +77,70 @@ describe('', () => { expect(sortIndicator).toBeInTheDocument(); }); - it('adds the toggle props to the header props if toggle props are available', () => { + it('renders the sort toggle as a button', () => { + render(); + expect(screen.getByRole('button', { name: 'Title' })).toBeInTheDocument(); + }); + + it('adds sortable header styling to preserve the sort button hit target', () => { + render(); + const cell = screen.getByRole('columnheader'); + const button = screen.getByRole('button', { name: 'Title' }); + expect(cell).toHaveClass('pgn__data-table-sortable-header'); + expect(button).toHaveClass('pgn__data-table-sort-button'); + }); + + it('adds headerClassName to sortable header content', () => { + render(); + const button = screen.getByRole('button', { name: 'Title' }); + expect(button).not.toHaveClass(props.headerClassName); + expect(button.firstChild).toHaveClass(props.headerClassName); + }); + + it('adds ascending sort state to the header cell when sorted ascending', () => { + render(); + const cell = screen.getByRole('columnheader'); + expect(cell).toHaveAttribute('aria-sort', 'ascending'); + }); + + it('adds descending sort state to the header cell when sorted descending', () => { + render(); + const cell = screen.getByRole('columnheader'); + expect(cell).toHaveAttribute('aria-sort', 'descending'); + }); + + it('does not add inactive sort state to unsorted header cells', () => { + render(); + const cell = screen.getByRole('columnheader'); + expect(cell).not.toHaveAttribute('aria-sort'); + }); + + it('adds the toggle props to the sort button if toggle props are available', () => { + render(); + const button = screen.getByRole('button', { name: 'Title' }); + expect(button).toHaveAttribute('data-sort-prop', sortByToggleProps['data-sort-prop']); + }); + + it('does not pass toggle props to the header props', () => { const headerPropsSpy = jest.fn().mockReturnValueOnce({}); render(); - expect(headerPropsSpy).toHaveBeenCalledWith(sortByToggleProps); + expect(headerPropsSpy).toHaveBeenCalledWith(); + }); + + it('makes sortable headers keyboard reachable and operable', async () => { + const user = userEvent.setup(); + const handleSort = jest.fn(); + render( ({ onClick: handleSort })} />); + const button = screen.getByRole('button', { name: 'Title' }); + + await user.tab(); + expect(button).toHaveFocus(); + + await user.keyboard('{Enter}'); + expect(handleSort).toHaveBeenCalledTimes(1); + + await user.keyboard(' '); + expect(handleSort).toHaveBeenCalledTimes(2); }); }); });