Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Table): updated a11y for empty/nontext Th components #10152

Merged
merged 4 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -651,7 +651,7 @@ export const FilterAttributeSearch: React.FunctionComponent = () => {
<Table aria-label="Selectable table">
<Thead>
<Tr>
<Th />
<Th screenReaderText="Row select" />
<Th width={20}>{columnNames.name}</Th>
<Th width={10}>{columnNames.threads}</Th>
<Th width={10}>{columnNames.apps}</Th>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ export const FilterCheckboxSelect: React.FunctionComponent = () => {
<Table aria-label="Selectable table">
<Thead>
<Tr>
<Th />
<Th screenReaderText="Row select" />
<Th width={20}>{columnNames.name}</Th>
<Th width={10}>{columnNames.threads}</Th>
<Th width={10}>{columnNames.apps}</Th>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ export const FilterFaceted: React.FunctionComponent = () => {
<Table aria-label="Selectable table">
<Thead>
<Tr>
<Th />
<Th screenReaderText="Row select" />
<Th width={20}>{columnNames.name}</Th>
<Th width={10}>{columnNames.threads}</Th>
<Th width={10}>{columnNames.apps}</Th>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ export const FilterMixedSelectGroup: React.FunctionComponent = () => {
<Table aria-label="Selectable table">
<Thead>
<Tr>
<Th />
<Th screenReaderText="Row select" />
<Th width={20}>{columnNames.name}</Th>
<Th width={10}>{columnNames.threads}</Th>
<Th width={10}>{columnNames.apps}</Th>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ export const FilterSameSelectGroup: React.FunctionComponent = () => {
<Table aria-label="Selectable table">
<Thead>
<Tr>
<Th />
<Th screenReaderText="Row select" />
<Th width={20}>{columnNames.name}</Th>
<Th width={10}>{columnNames.threads}</Th>
<Th width={10}>{columnNames.apps}</Th>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ export const FilterSearchInput: React.FunctionComponent = () => {
<Table aria-label="Selectable table">
<Thead>
<Tr>
<Th />
<Th screenReaderText="Row select" />
<Th width={20}>{columnNames.name}</Th>
<Th width={10}>{columnNames.threads}</Th>
<Th width={10}>{columnNames.apps}</Th>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ export const FilterSingleSelect: React.FunctionComponent = () => {
<Table aria-label="Selectable table">
<Thead>
<Tr>
<Th />
<Th screenReaderText="Row select" />
<Th width={20}>{columnNames.name}</Th>
<Th width={10}>{columnNames.threads}</Th>
<Th width={10}>{columnNames.apps}</Th>
Expand Down
2 changes: 1 addition & 1 deletion packages/react-core/src/demos/examples/Card/CardStatus.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export const CardStatus: React.FunctionComponent = () => {
<Table variant="compact">
<Thead>
<Tr>
<Th />
<Th screenReaderText="Row expansion" />
{columns.map((column, columnIndex) => (
<Th key={columnIndex} modifier="fitContent">
{column}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ export const TablesAndTabs = () => {
<Table aria-label="`Composable` table">
<Thead noWrap>
<Tr>
<Th />
<Th screenReaderText="Row select" />
<Th>{columnNames.name}</Th>
<Th>{columnNames.branches}</Th>
<Th>{columnNames.prs}</Th>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@ describe('Table Selectable Test', () => {
});

it('Check number of columns', () => {
cy.get('thead').find('th').should('have.length', 5);

// There should be a canSelectAll input
cy.get('thead').find('td').should('have.length', 1);
cy.get('thead').find('th').should('have.length', 6);
});

it('Test selectable checkbox', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ export const TableComposableDemo = () => {
<Table aria-label="Radio selectable table">
<Thead>
<Tr>
<Th />
<Th screenReaderText="Row select" />
<Th>{columns[0]}</Th>
<Th>{columns[1]}</Th>
<Th>{columns[2]}</Th>
Expand Down Expand Up @@ -439,7 +439,7 @@ export const TableComposableDemo = () => {
<Th>{columns[2]}</Th>
<Th>{columns[3]}</Th>
<Th>{columns[4]}</Th>
<Th />
<Th screenReaderText="Actions" />
</Tr>
</Thead>
<Tbody>
Expand Down Expand Up @@ -616,7 +616,7 @@ export const TableComposableDemo = () => {
<Table aria-label="Expandable Table" variant={compact ? 'compact' : null}>
<Thead>
<Tr>
<Th />
<Th screenReaderText="Row expansion" />
<Th>{columns[0]}</Th>
<Th>{columns[1]}</Th>
<Th>{columns[2]}</Th>
Expand Down
22 changes: 20 additions & 2 deletions packages/react-table/src/components/Table/Th.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ export interface ThProps
stickyRightOffset?: string;
/** Indicates the <th> is part of a subheader of a nested header */
isSubheader?: boolean;
/** Visually hidden text accessible only via assistive technologies. This must be passed in if the
* th is intended to be visually empty, and must be conveyed as a column header text.
*/
screenReaderText?: string;
/** Provides an accessible name to the th. This should only be passed in when the th contains only non-text
* content, such as a "select all" checkbox or "expand all" toggle.
*/
'aria-label'?: string;
}

const ThBase: React.FunctionComponent<ThProps> = ({
Expand Down Expand Up @@ -83,8 +91,17 @@ const ThBase: React.FunctionComponent<ThProps> = ({
stickyLeftOffset,
stickyRightOffset,
isSubheader = false,
screenReaderText,
'aria-label': ariaLabel,
...props
}: ThProps) => {
if (!children && !screenReaderText && !ariaLabel) {
// eslint-disable-next-line no-console
console.warn(
'Th: Table headers must have an accessible name. If the Th is intended to be visually empty, pass in screenReaderText. If the Th contains only non-text, interactive content such as a checkbox or expand toggle, pass in an aria-label.'
);
}

const [showTooltip, setShowTooltip] = React.useState(false);
const [truncated, setTruncated] = React.useState(false);
const cellRef = innerRef ? innerRef : React.createRef();
Expand Down Expand Up @@ -188,8 +205,9 @@ const ThBase: React.FunctionComponent<ThProps> = ({
onBlur={() => setShowTooltip(false)}
data-label={dataLabel}
onMouseEnter={tooltip !== null ? onMouseEnter : onMouseEnterProp}
scope={component === 'th' && children ? scope : null}
scope={component === 'th' ? scope : null}
ref={cellRef}
aria-label={ariaLabel}
className={css(
styles.tableTh,
className,
Expand All @@ -212,7 +230,7 @@ const ThBase: React.FunctionComponent<ThProps> = ({
} as React.CSSProperties
})}
>
{transformedChildren}
{transformedChildren || (screenReaderText && <span className="pf-v5-screen-reader">{screenReaderText}</span>)}
</MergedComponent>
);

Expand Down
31 changes: 31 additions & 0 deletions packages/react-table/src/components/Table/__tests__/Th.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import * as React from 'react';
import { render, screen } from '@testing-library/react';
import { Th } from '../Th';

test('Does not render with aria-label by default', () => {
render(<Th />);
expect(screen.getByRole('columnheader')).not.toHaveAccessibleName();
});

test('Renders with aria-label when passed in', () => {
render(<Th aria-label="Test" />);
expect(screen.getByRole('columnheader')).toHaveAccessibleName('Test');
});

test('Does not render with screen reader text by default', () => {
render(<Th />);

expect(screen.getByRole('columnheader')).toBeEmptyDOMElement();
});

test('Does not render with screen reader text when children are passed in', () => {
render(<Th screenReaderText="Test">Heading label</Th>);

expect(screen.getByRole('columnheader')).not.toHaveTextContent('Test');
});

test('Renders with screen reader text when screenReaderText is passed in', () => {
render(<Th screenReaderText="Test" />);

expect(screen.getByRole('columnheader')).toHaveTextContent('Test');
});
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ export const TableActions: React.FunctionComponent = () => {
<Th>{columnNames.prs}</Th>
<Th>{columnNames.workspaces}</Th>
<Th>{columnNames.lastCommit}</Th>
<Td></Td>
<Td></Td>
<Th screenReaderText="Primary action" />
<Th screenReaderText="Secondary action" />
</Tr>
</Thead>
<Tbody>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export const TableActions: React.FunctionComponent = () => {
<Th>{columnNames.prs}</Th>
<Th>{columnNames.workspaces}</Th>
<Th>{columnNames.lastCommit}</Th>
<Td></Td>
<Th screenReaderText="Actions" />
</Tr>
</Thead>
<Tbody>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export const TableCompoundExpandable: React.FunctionComponent = () => {
<Th>{columnNames.prs}</Th>
<Th>{columnNames.workspaces}</Th>
<Th>{columnNames.lastCommit}</Th>
<Th />
<Th screenReaderText="URL" />
</Tr>
</Thead>
{repositories.map((repo: Repository, rowIndex: number) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,10 @@ export const TableDraggable: React.FunctionComponent = () => {
];

return (
<Table aria-label="Draggable table" className={isDragging && styles.modifiers.dragOver}>
<Table aria-label="Draggable table" className={isDragging ? styles.modifiers.dragOver : ''}>
<Thead>
<Tr>
<Th />
<Th screenReaderText="Drag and drop" />
{columns.map((column, columnIndex) => (
<Th key={columnIndex}>{column}</Th>
))}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export const TableExpandable: React.FunctionComponent = () => {
<Table aria-label="Expandable table" variant={isExampleCompact ? 'compact' : undefined}>
<Thead>
<Tr>
<Th />
<Th screenReaderText="Row expansion" />
<Th width={25}>{columnNames.name}</Th>
<Th width={10}>{columnNames.branches}</Th>
<Th width={15}>{columnNames.prs}</Th>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export const TableNestedExpandable: React.FunctionComponent = () => {
<Table aria-label="Nested column headers with expandable rows table" gridBreakPoint="">
<Thead hasNestedHeader>
<Tr>
<Th rowSpan={2} />
<Th screenReaderText="Row expansion" rowSpan={2} />
<Th width={35} rowSpan={2} hasRightBorder>
{columnNames.team}
</Th>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ export const TableNestedHeaders: React.FunctionComponent = () => {
{columnNames.destination}
</Th>
</Tr>
{/* TODO: Remove the following Tr once Core updates towards https://github.com/patternfly/patternfly/issues/6272
are made for the v5 branch. For v6 branch the row can be removed immediately.
*/}
<Tr isBorderRow aria-hidden="true">
<Td colSpan={9}></Td>
</Tr>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ export const TableExpandable: React.FunctionComponent = () => {
<Table aria-label="Simple table">
<Thead>
<Tr>
<Td />
<Th screenReaderText="Row expansion" />
<Th width={20}>{columnNames.name}</Th>
<Th>{columnNames.branches}</Th>
<Th>{columnNames.prs}</Th>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const TableSelectableRadio: React.FunctionComponent = () => {
<Table aria-label="Selectable table">
<Thead>
<Tr>
<Th />
<Th screenReaderText="Row select" />
<Th>{columnNames.name}</Th>
<Th>{columnNames.branches}</Th>
<Th>{columnNames.prs}</Th>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export const TableStripedExpandable: React.FunctionComponent = () => {
<Table aria-label="Expandable table" variant={isExampleCompact ? 'compact' : undefined} isStriped isExpandable>
<Thead>
<Tr>
<Th />
<Th screenReaderText="Row expansion" />
<Th width={25}>{columnNames.name}</Th>
<Th width={10}>{columnNames.branches}</Th>
<Th width={15}>{columnNames.prs}</Th>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export const selectable: ITransform = (

return {
className: css(styles.tableCheck),
component: 'td',
component: rowId !== -1 ? 'td' : 'th',
isVisible: !rowData || !rowData.fullWidth,
children: (
<SelectColumn
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export const TableBulkSelect: React.FunctionComponent = () => {
<Table aria-label="Selectable table">
<Thead>
<Tr>
<Th />
<Th screenReaderText="Row select" />
<Th key={0}>{columns[0]}</Th>
<Th key={1}>{columns[1]}</Th>
<Th key={2}>{columns[2]}</Th>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export const TableCompoundExpansion: React.FunctionComponent = () => {
<Th>{columnNames.description}</Th>
<Th>{columnNames.date}</Th>
<Th>{columnNames.status}</Th>
<Th />
<Th screenReaderText="Actions" />
</Tr>
</Thead>
<Tbody>
Expand Down Expand Up @@ -205,7 +205,8 @@ export const TableCompoundExpansion: React.FunctionComponent = () => {
<Th>{columnNames.prs}</Th>
<Th>{columnNames.workspaces}</Th>
<Th>{columnNames.lastCommit}</Th>
<Th />
<Th screenReaderText="URL" />
<Th screenReaderText="Actions" />
</Tr>
</Thead>
{repositories.map((repo, rowIndex) => {
Expand Down
Loading
Loading