fix(accessibility): upgrade axe-core and fix new errors (uber#5081)

* fix(accessibility): upgrade axe-core and fix new errors * fix(a11y): update unit tests * fix(data-table): correct spacing * test(vrt): update visual snapshots for bf5bcb7 (uber#5082) Co-authored-by: UberOpenSourceBot <[email protected]> Co-authored-by: UberOpenSourceBot <[email protected]> Co-authored-by: UberOpenSourceBot <[email protected]>
imnotjames · Aug 10, 2022 · 0a7fa59 · 0a7fa59
1 parent 5e96e57
commit 0a7fa59
Show file tree

Hide file tree

Showing 34 changed files with 175 additions and 181 deletions.
diff --git a/package.json b/package.json
@@ -117,7 +117,7 @@
     "@typescript-eslint/parser": "^5.26.0",
     "@zeit/next-css": "^1.0.1",
     "@zeit/next-mdx": "^1.2.0",
-    "axe-core": "^4.2.2",
+    "axe-core": "^4.4.3",
     "babel-core": "^7.0.0-bridge.0",
     "babel-eslint": "10.1.0",
     "babel-jest": "^24.0.0",

diff --git a/src/data-table/column.tsx b/src/data-table/column.tsx
@@ -7,6 +7,7 @@ LICENSE file in the root directory of this source tree.
 import * as React from 'react';
 
 import { Checkbox } from '../checkbox';
+import { LocaleContext } from '../locale';
 import { useStyletron } from '../styles';
 
 import type { ColumnOptions } from './types';
@@ -27,6 +28,7 @@ function Column<Value, FilterParams>(
     // todo(flow->ts) add proper type annotation
     // eslint-disable-next-line @typescript-eslint/no-explicit-any,react/display-name
     renderCell: React.forwardRef((props, ref: any) => {
+      const locale = React.useContext(LocaleContext);
       const [css, theme] = useStyletron();
       const ProvidedCell = options.renderCell;
 
@@ -63,6 +65,7 @@ function Column<Value, FilterParams>(
             {Boolean(props.onSelect) && (
               <span className={css({ paddingRight: theme.sizing.scale300 })}>
                 <Checkbox
+                  aria-label={locale.datatable.selectRow}
                   onChange={props.onSelect}
                   checked={props.isSelected}
                   overrides={{

diff --git a/src/data-table/header-cell.tsx b/src/data-table/header-cell.tsx
@@ -7,6 +7,7 @@ LICENSE file in the root directory of this source tree.
 import * as React from 'react';
 
 import { Checkbox } from '../checkbox';
+import { LocaleContext } from '../locale';
 import { useStyletron } from '../styles';
 import ChevronDown from '../icon/chevron-down';
 import ChevronUp from '../icon/chevron-up';
@@ -37,6 +38,7 @@ type HeaderCellProps = {
 };
 
 const HeaderCell = React.forwardRef<HTMLDivElement, HeaderCellProps>((props, ref) => {
+  const locale = React.useContext(LocaleContext);
   const [css, theme] = useStyletron();
   const [focusVisible, setFocusVisible] = React.useState(false);
   const checkboxRef = React.useRef(null);
@@ -47,8 +49,7 @@ const HeaderCell = React.forwardRef<HTMLDivElement, HeaderCellProps>((props, ref
     }
   };
 
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars
-  const handleBlur = (event: SyntheticEvent) => {
+  const handleBlur = () => {
     if (focusVisible !== false) {
       setFocusVisible(false);
     }
@@ -61,95 +62,111 @@ const HeaderCell = React.forwardRef<HTMLDivElement, HeaderCellProps>((props, ref
   return (
     <div
       ref={ref}
-      role="button"
-      tabIndex={0}
       className={css({
         ...theme.typography.font350,
         alignItems: 'center',
         backgroundColor,
-        boxSizing: 'border-box',
         color: theme.colors.contentPrimary,
-        cursor: props.sortable ? 'pointer' : null,
         display: props.isMeasured ? 'inline-flex' : 'flex',
         flexGrow: 1,
         height: '100%',
         paddingLeft: theme.sizing.scale500,
-        paddingRight: theme.sizing.scale500,
-        flexWrap: 'nowrap',
-        whiteSpace: 'nowrap',
-        outline: focusVisible ? `3px solid ${theme.colors.accent}` : 'none',
-        outlineOffset: '-3px',
       })}
-      onMouseEnter={props.onMouseEnter}
-      onMouseLeave={props.onMouseLeave}
-      onKeyUp={(event) => {
-        if (event.key === 'Enter') {
-          props.onSort(props.index);
-        }
-      }}
-      onClick={(event) => {
-        // Avoid column sort if select-all checkbox click.
-        if (checkboxRef.current && checkboxRef.current.contains(event.target)) {
-          return;
-        }
-        if (props.sortable) {
-          props.onSort(props.index);
-        }
-      }}
-      onFocus={handleFocus}
-      onBlur={handleBlur}
     >
       {props.isSelectable && (
-        <span className={css({ paddingRight: theme.sizing.scale300 })} ref={checkboxRef}>
+        <span
+          className={css({
+            paddingRight: theme.sizing.scale300,
+          })}
+          ref={checkboxRef}
+        >
           <Checkbox
-            // eslint-disable-next-line @typescript-eslint/no-unused-vars
-            onChange={(e) => {
+            onChange={() => {
               if (props.isSelectedAll || props.isSelectedIndeterminate) {
                 props.onSelectNone();
               } else {
                 props.onSelectAll();
               }
             }}
+            aria-label={locale.datatable.selectAllRows}
             checked={props.isSelectedAll || props.isSelectedIndeterminate}
             isIndeterminate={props.isSelectedIndeterminate}
           />
         </span>
       )}
-      {props.title}
       <div
         className={css({
-          position: 'relative',
-          width: '100%',
-          display: 'flex',
           alignItems: 'center',
+          backgroundColor: 'transparent',
+          border: 'none',
+          boxSizing: 'border-box',
+          cursor: props.sortable ? 'pointer' : null,
+          display: 'flex',
+          flexGrow: 1,
+          height: '100%',
+          outline: focusVisible ? `3px solid ${theme.colors.accent}` : 'none',
+          outlineOffset: '-3px',
+          // paddingLeft: theme.sizing.scale500,
+          paddingRight: theme.sizing.scale500,
+          whiteSpace: 'nowrap',
         })}
+        onMouseEnter={props.onMouseEnter}
+        onMouseLeave={props.onMouseLeave}
+        onKeyUp={(event) => {
+          if (event.key === 'Enter') {
+            props.onSort(props.index);
+          }
+        }}
+        onClick={() => {
+          if (props.sortable) {
+            props.onSort(props.index);
+          }
+        }}
+        onFocus={handleFocus}
+        onBlur={handleBlur}
+        role="button"
+        tabIndex={0}
       >
-        {(props.isHovered || props.sortDirection) && props.sortable && (
-          <div
-            style={{
-              backgroundColor,
-              display: 'flex',
-              alignItems: 'center',
-              position: 'absolute',
-              right: -4,
-            }}
-          >
-            {props.sortDirection === SORT_DIRECTIONS.DESC && (
-              <ChevronDown
-                color={
-                  props.sortDirection ? theme.colors.contentPrimary : theme.colors.contentSecondary
-                }
-              />
-            )}
-            {(props.sortDirection === SORT_DIRECTIONS.ASC || !props.sortDirection) && (
-              <ChevronUp
-                color={
-                  props.sortDirection ? theme.colors.contentPrimary : theme.colors.contentSecondary
-                }
-              />
-            )}
-          </div>
-        )}
+        {props.title}
+        <div
+          className={css({
+            position: 'relative',
+            width: '100%',
+            display: 'flex',
+            alignItems: 'center',
+          })}
+        >
+          {(props.isHovered || props.sortDirection) && props.sortable && (
+            <div
+              style={{
+                backgroundColor,
+                display: 'flex',
+                alignItems: 'center',
+                position: 'absolute',
+                right: -4,
+              }}
+            >
+              {props.sortDirection === SORT_DIRECTIONS.DESC && (
+                <ChevronDown
+                  color={
+                    props.sortDirection
+                      ? theme.colors.contentPrimary
+                      : theme.colors.contentSecondary
+                  }
+                />
+              )}
+              {(props.sortDirection === SORT_DIRECTIONS.ASC || !props.sortDirection) && (
+                <ChevronUp
+                  color={
+                    props.sortDirection
+                      ? theme.colors.contentPrimary
+                      : theme.colors.contentSecondary
+                  }
+                />
+              )}
+            </div>
+          )}
+        </div>
       </div>
     </div>
   );

diff --git a/src/data-table/locale.js.flow b/src/data-table/locale.js.flow
@@ -40,6 +40,8 @@ export type DataTableLocaleT = {|
   booleanFilterFalse: string,
   booleanColumnTrueShort: string,
   booleanColumnFalseShort: string,
+  selectRow: string,
+  selectAllRows: string,
 |};
 
 const locale = {
@@ -77,6 +79,9 @@ const locale = {
   booleanFilterFalse: 'false',
   booleanColumnTrueShort: 'T',
   booleanColumnFalseShort: 'F',
+  selectRow: 'Select row',
+  selectAllRows: 'Select all rows',
+
 };
 
 export default locale;
diff --git a/src/data-table/locale.ts b/src/data-table/locale.ts
@@ -38,6 +38,8 @@ export type DataTableLocale = {
   booleanFilterFalse: string;
   booleanColumnTrueShort: string;
   booleanColumnFalseShort: string;
+  selectRow: string;
+  selectAllRows: string;
 };
 
 const locale = {
@@ -75,6 +77,8 @@ const locale = {
   booleanFilterFalse: 'false',
   booleanColumnTrueShort: 'T',
   booleanColumnFalseShort: 'F',
+  selectRow: 'Select row',
+  selectAllRows: 'Select all rows',
 };
 
 export default locale;
diff --git a/src/data-table/stateful-data-table.tsx b/src/data-table/stateful-data-table.tsx
@@ -232,12 +232,12 @@ export function StatefulDataTable(props: StatefulDataTableProps) {
 
               {Boolean(selectedRowIds.size) && props.batchActions && (
                 <div
-                  style={{
+                  className={css({
                     display: 'flex',
                     alignItems: 'center',
-                    paddingTop: theme.sizing.scale400,
-                    paddingBottom: theme.sizing.scale400,
-                  }}
+                    paddingTop: theme.sizing.scale300,
+                    paddingBottom: theme.sizing.scale300,
+                  })}
                 >
                   {props.batchActions.map((action) => {
                     function onClick(event) {

diff --git a/src/datepicker/datepicker.tsx b/src/datepicker/datepicker.tsx
@@ -7,7 +7,7 @@ LICENSE file in the root directory of this source tree.
 import * as React from 'react';
 
 import { MaskedInput } from '../input';
-import { Popover, PLACEMENT } from '../popover';
+import { Popover, PLACEMENT, ACCESSIBILITY_TYPE } from '../popover';
 import Calendar from './calendar';
 import { getOverrides } from '../helpers/overrides';
 import getInterpolatedString from '../helpers/i18n-interpolation';
@@ -503,6 +503,7 @@ export default class Datepicker<T = Date> extends React.Component<
         {(locale) => (
           <React.Fragment>
             <PopoverComponent
+              accessibilityType={ACCESSIBILITY_TYPE.none}
               focusLock={false}
               autoFocus={false}
               mountNode={this.props.mountNode}

diff --git a/src/locale/es_AR.ts b/src/locale/es_AR.ts
@@ -85,6 +85,8 @@ const es_AR: Locale = {
     booleanFilterFalse: 'falso',
     booleanColumnTrueShort: 'V',
     booleanColumnFalseShort: 'F',
+    selectRow: 'Seleccionar fila',
+    selectAllRows: 'Seleccionar todas las filas',
   },
 
   buttongroup: {

diff --git a/src/locale/tr_TR.ts b/src/locale/tr_TR.ts
@@ -85,6 +85,8 @@ const tr_TR: Locale = {
     booleanFilterFalse: 'yanlış',
     booleanColumnTrueShort: 'D',
     booleanColumnFalseShort: 'Y',
+    selectRow: 'satır seç',
+    selectAllRows: 'Tüm satırları seç',
   },
 
   buttongroup: {

diff --git a/src/pagination/__tests__/pagination.e2e.ts b/src/pagination/__tests__/pagination.e2e.ts
@@ -8,57 +8,37 @@ LICENSE file in the root directory of this source tree.
 import { expect, test } from '@playwright/test';
 import { mount, analyzeAccessibility } from '../../test/integration';
 
-const selectors = {
-  prevButton: 'button[data-test="prev-button"]',
-  nextButton: 'button[data-test="next-button"]',
-  dropDownButton: '[data-baseweb="select"] [aria-selected]',
-};
-
 test.describe('pagination', () => {
   test('passes basic accessibility tests', async ({ page }) => {
     await mount(page, 'pagination--pagination');
-    await page.waitForSelector(selectors.prevButton);
-    const accessibilityReport = await analyzeAccessibility(page, [
-      {
-        //indicates listbox element should contain option elements (options are hoisted in the popover)
-        id: 'aria-required-children',
-        enabled: false,
-      },
-    ]);
+    const prev = page.locator('text=Prev').first();
+    await prev.waitFor();
 
+    const accessibilityReport = await analyzeAccessibility(page);
     expect(accessibilityReport).toHaveNoAccessibilityIssues();
   });
 
   test('can be navigated using the prev and next buttons', async ({ page }) => {
     await mount(page, 'pagination--pagination');
-    await page.waitForSelector(selectors.prevButton);
-    // assert initial state
-    const initialValue = await page.$eval(selectors.dropDownButton, (input) => input.textContent);
-    expect(initialValue).toBe('1');
-
-    // paginate to the next page
-    await page.click(selectors.nextButton);
-    let value = await page.$eval(selectors.dropDownButton, (input) => input.textContent);
-    expect(value).toBe('2');
-
-    // paginate to the previous page
-    await page.click(selectors.prevButton);
-    value = await page.$eval(selectors.dropDownButton, (input) => input.textContent);
-    expect(value).toBe('1');
+    const select = page.locator('[data-baseweb="select"]').first();
+    const prev = page.locator('text=Prev').first();
+    const next = page.locator('text=Next').first();
+
+    await expect(select).toContainText('1');
+    await next.click();
+    await expect(select).toContainText('2');
+    await prev.click();
+    await expect(select).toContainText('1');
   });
 
   test('can be navigated using the dropdown menu', async ({ page }) => {
     await mount(page, 'pagination--pagination');
-    await page.waitForSelector(selectors.prevButton);
-    // assert initial state
-    const initialValue = await page.$eval(selectors.dropDownButton, (input) => input.textContent);
-    expect(initialValue).toBe('1');
-
-    // paginate using the dropdown menu
-    await page.click(selectors.dropDownButton);
-    await page.click('ul li:nth-child(3)');
+    const select = page.locator('[data-baseweb="select"]').first();
+    const options = page.locator('[role="option"]');
 
-    let value = await page.$eval(selectors.dropDownButton, (input) => input.textContent);
-    expect(value).toBe('3');
+    await expect(select).toContainText('1');
+    await select.click();
+    await options.nth(2).click();
+    await expect(select).toContainText('3');
   });
 });