[EuiBasicTable & EuiInMemoryTable] Fix multiple accessibility/axe err…

…ors on EuiBasicTable and EuiInMemoryTable pages (elastic#5241) * Fix duplicate checkbox `id`s with multiple isSelectable EuiBasicTables on the page - This is possibly only an issue with our docs and the fact that we use the same dataset repeatedly across multiple examples, but solves the error of duplicated IDs and shouldn't negatively affect production users * Fix duplicate popover `id`s with multiple EuiInMemoryTable filters on the page - there's no real need for this popover to have a custom ID instead of a randomized one - remove it - This is possibly only an issue with our docs and the fact that we use the same dataset repeatedly across examples, but solves the error of duplicated IDs and shouldn't negatively affect production users * Fix missing `aria-label` on table pagination - An `aria-label` was being passed to the `PaginationBar` component, but it wasn't actually being correctly used: - `PaginationBar` was never passing an `aria-label` prop down to `EuiTablePagination` or `EuiPagination` - The i18n typing was passing a react element instead of a string via render prop * Fix multiple `landmark-unique` issues on EuiBasicTable & EuiInMemoryTable docs - While we added an aria-label for each table's paginatoin nav, they still need to be unique for multiple tables on the page, which means adding a tableCaption for each demo * Fix `scope-attr-valid` errors on empty table column headings - empty `td`s within a `thead` is valid per https://webaim.org/techniques/tables/data's examples, but should not have the `scope` attr set * Improve demos with empty table headings - I opted to use visually empty headings, populated with EuiScreenReaderOnly text as an example for users looking to implement their own columns + fix odd data-test-subjs caused by either undefined or node column names * Fix various axe failures on custom EuiTable example - missing labels on checkbox elements, duplicate checkbox ID (solved in elastic#5237) - These are all a11y solutions that come OOTB in EuiBasicTable, but need to be added for the custom example * Fix unsemantic headings in EuiBasicTable responsive documentation - these should very likely just be paragraphs, not headings * Re-enable basic table & in-memory table documentations in a11y tests * Add changelog entry * PR feedback: screen reader landmark copy
ym · Oct 29, 2021 · 65ff97c · 65ff97c
1 parent 1766512
commit 65ff97c
Show file tree

Hide file tree

Showing 30 changed files with 533 additions and 509 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,7 +13,7 @@ No public interface changes since `39.0.0`.
 **Bug fixes**
 
 - Fixed tick and level alignment in `Eui[Dual]Range` ([#5181](https://github.com/elastic/eui/pull/5181))
-- Fixed duplicate IDs on mobile/desktop select all checkboxes in `EuiBasicTable` ([#5237](https://github.com/elastic/eui/pull/5237))
+- Fixed multiple accessibility issues in `EuiBasicTable` and `EuiInMemoryTable` ([#5237](https://github.com/elastic/eui/pull/5237), [#5241](https://github.com/elastic/eui/pull/5241))
 - Fixed missing i18n token in `EuiBasicTable`'s no items message ([#5242](https://github.com/elastic/eui/pull/5242))
 
 **Breaking changes**

diff --git a/scripts/a11y-testing.js b/scripts/a11y-testing.js
@@ -6,8 +6,6 @@ const docsPages = async (root, page) => {
   const pagesToSkip = [
     `${root}#/layout/page`, // Has duplicate `<main>` element
     `${root}#/layout/page-header`, // Has duplicate `<header>` element
-    `${root}#/tabular-content/tables`,
-    `${root}#/tabular-content/in-memory-tables`,
     `${root}#/display/aspect-ratio`,
     `${root}#/forms/combo-box`,
     `${root}#/forms/color-selection`,

diff --git a/src-docs/src/views/tables/actions/actions.js b/src-docs/src/views/tables/actions/actions.js
@@ -318,6 +318,7 @@ export const Table = () => {
       <EuiSpacer size="l" />
 
       <EuiBasicTable
+        tableCaption="Demo of EuiBasicTable with actions"
         items={pageOfItems}
         itemId="id"
         columns={columns}

diff --git a/src-docs/src/views/tables/auto/auto.js b/src-docs/src/views/tables/auto/auto.js
@@ -199,6 +199,7 @@ export const Table = () => {
       />
       <EuiSpacer size="m" />
       <EuiBasicTable
+        tableCaption="Demo of EuiBasicTable's table layout options"
         items={items}
         columns={layout === 'custom' ? customColumns : columns}
         tableLayout={layout === 'auto' ? 'auto' : 'fixed'}

diff --git a/src-docs/src/views/tables/basic/basic.js b/src-docs/src/views/tables/basic/basic.js
@@ -120,6 +120,7 @@ export const Table = () => {
 
   return (
     <EuiBasicTable
+      tableCaption="Demo of EuiBasicTable"
       items={items}
       rowHeader="firstName"
       columns={columns}

diff --git a/src-docs/src/views/tables/custom/custom.js b/src-docs/src/views/tables/custom/custom.js
@@ -28,6 +28,7 @@ import {
   EuiTableRowCellCheckbox,
   EuiTableSortMobile,
   EuiTableHeaderMobile,
+  EuiScreenReaderOnly,
 } from '../../../../../src/components';
 
 import {
@@ -248,7 +249,8 @@ export default class extends Component {
       },
       {
         id: 'type',
-        label: '',
+        label: 'Type',
+        isVisuallyHiddenLabel: true,
         alignment: LEFT_ALIGNMENT,
         width: '24px',
         cellProvider: (cell) => <EuiIcon type={cell} size="m" />,
@@ -320,7 +322,8 @@ export default class extends Component {
       },
       {
         id: 'actions',
-        label: '',
+        label: 'Actions',
+        isVisuallyHiddenLabel: true,
         alignment: RIGHT_ALIGNMENT,
         isActionsPopover: true,
         width: '32px',
@@ -437,8 +440,10 @@ export default class extends Component {
   renderSelectAll = (mobile) => {
     return (
       <EuiCheckbox
-        id="selectAllCheckbox"
-        label={mobile ? 'Select all' : null}
+        id={mobile ? 'selectAllCheckboxMobile' : 'selectAllCheckboxDesktop'}
+        label={mobile ? 'Select all rows' : null}
+        aria-label="Select all rows"
+        title="Select all rows"
         checked={this.areAllItemsSelected()}
         onChange={this.toggleAll.bind(this)}
         type={mobile ? null : 'inList'}
@@ -473,6 +478,14 @@ export default class extends Component {
             {this.renderSelectAll()}
           </EuiTableHeaderCellCheckbox>
         );
+      } else if (column.isVisuallyHiddenLabel) {
+        headers.push(
+          <EuiTableHeaderCell key={column.id} width={column.width}>
+            <EuiScreenReaderOnly>
+              <span>{column.label}</span>
+            </EuiScreenReaderOnly>
+          </EuiTableHeaderCell>
+        );
       } else {
         headers.push(
           <EuiTableHeaderCell
@@ -511,6 +524,8 @@ export default class extends Component {
                 checked={this.isItemSelected(item.id)}
                 onChange={this.toggleItem.bind(this, item.id)}
                 type="inList"
+                title="Select this row"
+                aria-label="Select this row"
               />
             </EuiTableRowCellCheckbox>
           );
@@ -741,6 +756,7 @@ export default class extends Component {
         <EuiSpacer size="m" />
 
         <EuiTablePagination
+          tableCaption="Custom EuiTable demo"
           aria-controls={exampleId}
           activePage={this.pager.getCurrentPageIndex()}
           itemsPerPage={this.state.itemsPerPage}

diff --git a/src-docs/src/views/tables/expanding_rows/expanding_rows.js b/src-docs/src/views/tables/expanding_rows/expanding_rows.js
@@ -8,6 +8,7 @@ import {
   EuiHealth,
   EuiButton,
   EuiDescriptionList,
+  EuiScreenReaderOnly,
 } from '../../../../../src/components';
 
 import { RIGHT_ALIGNMENT } from '../../../../../src/services';
@@ -160,6 +161,11 @@ export const Table = () => {
       align: RIGHT_ALIGNMENT,
       width: '40px',
       isExpander: true,
+      name: (
+        <EuiScreenReaderOnly>
+          <span>Expand rows</span>
+        </EuiScreenReaderOnly>
+      ),
       render: (item) => (
         <EuiButtonIcon
           onClick={() => toggleDetails(item)}
@@ -195,6 +201,7 @@ export const Table = () => {
     <Fragment>
       {deleteButton}
       <EuiBasicTable
+        tableCaption="Demo of EuiBasicTable with expanding rows"
         items={pageOfItems}
         itemId="id"
         itemIdToExpandedRowMap={itemIdToExpandedRowMap}

diff --git a/src-docs/src/views/tables/footer/footer.js b/src-docs/src/views/tables/footer/footer.js
@@ -195,6 +195,7 @@ export const Table = () => {
     <Fragment>
       {deleteButton}
       <EuiBasicTable
+        tableCaption="Demo of EuiBasicTable with footer"
         items={pageOfItems}
         itemId="id"
         columns={columns}

diff --git a/src-docs/src/views/tables/in_memory/in_memory.js b/src-docs/src/views/tables/in_memory/in_memory.js
@@ -90,6 +90,7 @@ export const Table = () => {
 
   return (
     <EuiInMemoryTable
+      tableCaption="Demo of EuiInMemoryTable"
       items={store.users}
       columns={columns}
       pagination={true}

diff --git a/src-docs/src/views/tables/in_memory/in_memory_controlled_pagination.js b/src-docs/src/views/tables/in_memory/in_memory_controlled_pagination.js
@@ -107,6 +107,7 @@ export const Table = () => {
 
   return (
     <EuiInMemoryTable
+      tableCaption="Demo of EuiInMemoryTable with controlled pagination"
       onTableChange={({ page: { index } }) =>
         setPagination({ pageIndex: index })
       }

diff --git a/src-docs/src/views/tables/in_memory/in_memory_custom_sorting.js b/src-docs/src/views/tables/in_memory/in_memory_custom_sorting.js
@@ -38,6 +38,7 @@ export const Table = () => {
 
   return (
     <EuiInMemoryTable
+      tableCaption="Demo of EuiInMemoryTable with custom sorting"
       items={data}
       columns={columns}
       pagination={false}

diff --git a/src-docs/src/views/tables/in_memory/in_memory_search.js b/src-docs/src/views/tables/in_memory/in_memory_search.js
@@ -145,6 +145,7 @@ export const Table = () => {
       </EuiFlexGroup>
       <EuiSpacer size="l" />
       <EuiInMemoryTable
+        tableCaption="Demo of EuiInMemoryTable with search"
         items={store.users}
         columns={columns}
         search={search}

diff --git a/src-docs/src/views/tables/in_memory/in_memory_search_callback.js b/src-docs/src/views/tables/in_memory/in_memory_search_callback.js
@@ -66,6 +66,7 @@ export const Table = () => {
 
   return (
     <EuiInMemoryTable
+      tableCaption="Demo of EuiInMemoryTable with search callback"
       items={items}
       loading={isLoading}
       columns={[

diff --git a/src-docs/src/views/tables/in_memory/in_memory_search_external.js b/src-docs/src/views/tables/in_memory/in_memory_search_external.js
@@ -199,6 +199,7 @@ export const Table = () => {
         </EuiFlexItem>
         <EuiFlexItem grow={3}>
           <EuiInMemoryTable
+            tableCaption="Demo of EuiInMemoryTable with search and external state"
             items={store.users}
             columns={columns}
             search={search}

diff --git a/src-docs/src/views/tables/in_memory/in_memory_selection.js b/src-docs/src/views/tables/in_memory/in_memory_selection.js
@@ -241,6 +241,7 @@ export const Table = () => {
       <EuiSpacer size="l" />
 
       <EuiInMemoryTable
+        tableCaption="Demo of EuiInMemoryTable with selection"
         ref={tableRef}
         items={users}
         itemId="id"

diff --git a/src-docs/src/views/tables/mobile/mobile.js b/src-docs/src/views/tables/mobile/mobile.js
@@ -216,6 +216,7 @@ export const Table = () => {
       <EuiSpacer size="l" />
 
       <EuiBasicTable
+        tableCaption="Demo for responsive EuiBasicTable with mobile options"
         items={pageOfItems}
         itemId="id"
         columns={columns}

diff --git a/src-docs/src/views/tables/mobile/mobile_section.js b/src-docs/src/views/tables/mobile/mobile_section.js
@@ -44,12 +44,12 @@ export const section = {
         the table does not break down easily into this format. For these use
         cases, you may set <EuiCode language="js">responsive=false</EuiCode>.
       </p>
-      <h4>
+      <p>
         To make your table work responsively, please make sure you add the
         following <EuiTextColor color="danger">additional</EuiTextColor> props
         to the top level table component (<strong>EuiBasicTable</strong> or{' '}
         <strong>EuiInMemoryTable</strong>):
-      </h4>
+      </p>
       <ul>
         <li>
           <EuiCode>isSelectable</EuiCode>: if the table has a single column of
@@ -63,16 +63,15 @@ export const section = {
           which may/may not be hidden in hover
         </li>
       </ul>
-      <h4>
+      <p>
         The <EuiCode>mobileOptions</EuiCode> object can be passed to the{' '}
         <strong>EuiTableRowCell</strong> directly or with each column item
         provided to <strong>EuiBasicTable</strong>.
-      </h4>
+      </p>
       <EuiCodeBlock language="js">{exampleItem}</EuiCodeBlock>
-      <h4>Note:</h4>
       <p>
-        You can also change basic table row cell props like{' '}
-        <EuiCode>truncateText</EuiCode> and <EuiCode>textOnly</EuiCode> for
+        <strong>Note:</strong> You can also change basic table row cell props
+        like <EuiCode>truncateText</EuiCode> and <EuiCode>textOnly</EuiCode> for
         mobile layouts, though you must also be passing a mobile specific render
         function.
       </p>

diff --git a/src-docs/src/views/tables/paginated/paginated.js b/src-docs/src/views/tables/paginated/paginated.js
@@ -148,6 +148,7 @@ export const Table = () => {
       />
       <EuiSpacer size="xl" />
       <EuiBasicTable
+        tableCaption="Demo for EuiBasicTable with pagination"
         items={pageOfItems}
         columns={columns}
         pagination={pagination}

diff --git a/src-docs/src/views/tables/selection/selection.js b/src-docs/src/views/tables/selection/selection.js
@@ -205,6 +205,7 @@ export const Table = () => {
       <EuiSpacer size="l" />
 
       <EuiBasicTable
+        tableCaption="Demo for EuiBasicTable with selection"
         ref={tableRef}
         items={pageOfItems}
         itemId="id"

diff --git a/src-docs/src/views/tables/sorting/sorting.js b/src-docs/src/views/tables/sorting/sorting.js
@@ -211,6 +211,7 @@ export const Table = () => {
       </EuiFlexGroup>
       <EuiSpacer />
       <EuiBasicTable
+        tableCaption="Demo for EuiBasicTable with sorting"
         items={pageOfItems}
         columns={columns}
         pagination={pagination}