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

thatblindgeye · web-flow · commit 48d8140d5ecd · 2024-03-26T13:12:09.000-04:00
* fix(Table): updated a11y for empty/nontext Th components

* Updated additional examples and demos

* Added initial Th test file

* Updated deprecated snapshots, removed outdated integration test
diff --git a/packages/react-core/src/demos/Filters/examples/FilterAttributeSearch.tsx b/packages/react-core/src/demos/Filters/examples/FilterAttributeSearch.tsx
@@ -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>
diff --git a/packages/react-core/src/demos/Filters/examples/FilterCheckboxSelect.tsx b/packages/react-core/src/demos/Filters/examples/FilterCheckboxSelect.tsx
@@ -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>
diff --git a/packages/react-core/src/demos/Filters/examples/FilterFaceted.tsx b/packages/react-core/src/demos/Filters/examples/FilterFaceted.tsx
@@ -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>
diff --git a/packages/react-core/src/demos/Filters/examples/FilterMixedSelectGroup.tsx b/packages/react-core/src/demos/Filters/examples/FilterMixedSelectGroup.tsx
@@ -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>
diff --git a/packages/react-core/src/demos/Filters/examples/FilterSameSelectGroup.tsx b/packages/react-core/src/demos/Filters/examples/FilterSameSelectGroup.tsx
@@ -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>
diff --git a/packages/react-core/src/demos/Filters/examples/FilterSearchInput.tsx b/packages/react-core/src/demos/Filters/examples/FilterSearchInput.tsx
@@ -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>
diff --git a/packages/react-core/src/demos/Filters/examples/FilterSingleSelect.tsx b/packages/react-core/src/demos/Filters/examples/FilterSingleSelect.tsx
@@ -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>
diff --git a/packages/react-core/src/demos/examples/Card/CardStatus.tsx b/packages/react-core/src/demos/examples/Card/CardStatus.tsx
@@ -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}
diff --git a/packages/react-core/src/demos/examples/Tabs/TabsAndTable.tsx b/packages/react-core/src/demos/examples/Tabs/TabsAndTable.tsx
@@ -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>
diff --git a/packages/react-integration/cypress/integration/tableselectable.spec.ts b/packages/react-integration/cypress/integration/tableselectable.spec.ts
@@ -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', () => {
diff --git a/packages/react-integration/demo-app-ts/src/components/demos/TableDemo/TableComposableDemo.tsx b/packages/react-integration/demo-app-ts/src/components/demos/TableDemo/TableComposableDemo.tsx
@@ -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>
@@ -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>
@@ -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>
diff --git a/packages/react-table/src/components/Table/Th.tsx b/packages/react-table/src/components/Table/Th.tsx
@@ -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> = ({
@@ -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();
@@ -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,
@@ -212,7 +230,7 @@ const ThBase: React.FunctionComponent<ThProps> = ({
         } as React.CSSProperties
       })}
     >
-      {transformedChildren}
+      {transformedChildren || (screenReaderText && <span className="pf-v5-screen-reader">{screenReaderText}</span>)}
     </MergedComponent>
   );
 
diff --git a/packages/react-table/src/components/Table/__tests__/Th.test.tsx b/packages/react-table/src/components/Table/__tests__/Th.test.tsx
@@ -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');
+});
diff --git a/packages/react-table/src/components/Table/examples/TableActions.tsx b/packages/react-table/src/components/Table/examples/TableActions.tsx
@@ -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>
diff --git a/packages/react-table/src/components/Table/examples/TableActionsOverflow.tsx b/packages/react-table/src/components/Table/examples/TableActionsOverflow.tsx
@@ -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>
diff --git a/packages/react-table/src/components/Table/examples/TableCompoundExpandable.tsx b/packages/react-table/src/components/Table/examples/TableCompoundExpandable.tsx
@@ -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) => {
diff --git a/packages/react-table/src/components/Table/examples/TableDraggable.tsx b/packages/react-table/src/components/Table/examples/TableDraggable.tsx
@@ -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>
           ))}
diff --git a/packages/react-table/src/components/Table/examples/TableExpandable.tsx b/packages/react-table/src/components/Table/examples/TableExpandable.tsx
@@ -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>
diff --git a/packages/react-table/src/components/Table/examples/TableNestedExpandable.tsx b/packages/react-table/src/components/Table/examples/TableNestedExpandable.tsx
@@ -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>
diff --git a/packages/react-table/src/components/Table/examples/TableNestedStickyHeader.tsx b/packages/react-table/src/components/Table/examples/TableNestedStickyHeader.tsx
@@ -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>
diff --git a/packages/react-table/src/components/Table/examples/TableNestedTableExpandable.tsx b/packages/react-table/src/components/Table/examples/TableNestedTableExpandable.tsx
@@ -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>
diff --git a/packages/react-table/src/components/Table/examples/TableSelectableRadio.tsx b/packages/react-table/src/components/Table/examples/TableSelectableRadio.tsx
@@ -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>
diff --git a/packages/react-table/src/components/Table/examples/TableStripedExpandable.tsx b/packages/react-table/src/components/Table/examples/TableStripedExpandable.tsx
@@ -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>
diff --git a/packages/react-table/src/components/Table/utils/decorators/selectable.tsx b/packages/react-table/src/components/Table/utils/decorators/selectable.tsx
@@ -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
diff --git a/packages/react-table/src/demos/examples/TableBulkSelect.tsx b/packages/react-table/src/demos/examples/TableBulkSelect.tsx
@@ -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>
diff --git a/packages/react-table/src/demos/examples/TableCompoundExpansion.tsx b/packages/react-table/src/demos/examples/TableCompoundExpansion.tsx
@@ -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>
@@ -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) => {
diff --git a/packages/react-table/src/deprecated/components/Table/__tests__/__snapshots__/Table.test.tsx.snap b/packages/react-table/src/deprecated/components/Table/__tests__/__snapshots__/Table.test.tsx.snap
