Fix/readd patternfly a11y (patternfly#4879)

* add patternfly-a11y * fix(a11y): fix existing a11y issues in fullscreen pages * fix ci * fix webpack cache * remove store artifacts * address feedback * update datalist snapshots
arnthor-agustsson · Sep 28, 2020 · 525ffb2 · 525ffb2
1 parent 2006c88
commit 525ffb2
Show file tree

Hide file tree

Showing 18 changed files with 288 additions and 73 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -40,8 +40,8 @@ references:
     js-deps-v{{.Environment.CACHE_VERSION}}-{{checksum "yarn.lock"}}
   cypress_cache_key: &cypress_cache_key
     cypress-v{{.Environment.CACHE_VERSION}}-{{checksum "yarn.lock"}}
-  gatsby_cache_key: &gatsby_cache_key
-    gatsby-v{{.Environment.CACHE_VERSION}}-{{checksum "yarn.lock"}}
+  webpack_cache_key: &webpack_cache_key
+    webpack-v{{.Environment.CACHE_VERSION}}-{{checksum "yarn.lock"}}
   build_cache_key: &build_cache_key
     build-cache-v{{.Environment.CACHE_VERSION}}-{{checksum "yarn.lock"}}
   lint_cache_key: &lint_cache_key
@@ -69,9 +69,9 @@ workflows:
     - test_integration:
         requires:
         - build_demo_app
-    # - test_a11y_pf4:
-    #     requires:
-    #     - build_docs_pf4
+    - test_a11y_pf4:
+        requires:
+        - build_docs_pf4
     - deploy_prerelease:
         requires:
         - lint_pf4
@@ -219,19 +219,21 @@ jobs:
         - *js_deps_cache_key
     - restore_cache:
         keys:
-        - *gatsby_cache_key
+        - *webpack_cache_key
     - run:
         name: Build patternfly-react docs
         command: yarn build:docs
     - save_cache:
         paths: *webpack_cache_paths
-        key: *gatsby_cache_key
+        key: *webpack_cache_key
     - run:
         name: Upload docs to surge.sh
         command: node .circleci/upload-preview.js packages/react-docs/public
     - persist_to_workspace:
         root: ~/project
-        paths: *webpack_cache_paths
+        paths:
+        - packages/react-docs/public
+        - packages/react-docs/src/generated
   test_a11y_pf4:
     docker:
     - image: circleci/node:12-browsers
@@ -249,10 +251,8 @@ jobs:
         command: yarn test:a11y
     - run:
         name: Upload a11y report
-        command: node .circleci/upload-preview.js packages/react-docs/coverage
+        command: node .circleci/upload-preview.js packages/react-docs/coverage/dist
         when: always
-    - store_artifacts:
-        path: packages/react-docs/coverage/
   deploy_prerelease:
     docker:
     - image: circleci/node:12

diff --git a/.circleci/upload-preview.js b/.circleci/upload-preview.js
@@ -1,6 +1,5 @@
-const fs = require('fs');
 const path = require('path');
-const Octokit = require('@octokit/rest');
+const { Octokit } = require('@octokit/rest');
 const octokit = new Octokit({ auth: process.env.GH_PR_TOKEN });
 const surge = require('surge');
 const publishFn = surge().publish();
@@ -21,15 +20,12 @@ let uploadURL = `${repo}-${prnum ? `pr-${prnum}` : prbranch}`
   .replace(/[\/|\.]/g, '-')
   .replace('-master', '');
 
-if (uploadFolderName === 'coverage') {
-  fs.copyFileSync(
-    path.join(uploadFolder, 'report.html'),
-    path.join(uploadFolder, 'index.html')
-  );
-}
 if (uploadFolderName === '.out') {
   uploadURL += '-pf3';
 }
+else if (uploadFolderName === 'dist') {
+  uploadURL += `-a11y`;
+}
 else if (uploadFolderName === 'results') {
   uploadURL += `-cypress`;
 }
@@ -76,7 +72,7 @@ if (prnum) {
       else if (uploadFolderName === 'public') {
         commentBody += tryAddComment(`PF4 preview: https://${uploadURL}`, commentBody);
       }
-      else if (uploadFolderName === 'coverage') {
+      else if (uploadFolderName === 'dist') {
         commentBody += tryAddComment(`A11y report: https://${uploadURL}`, commentBody);
       }
       else if (uploadFolderName === 'report') {

diff --git a/package.json b/package.json
@@ -87,7 +87,7 @@
     "lint:ts": "yarn lint packages",
     "lint:versions": "node scripts/verifyPatternflyVersions.js",
     "prettier": "node node_modules/.bin/prettier --write \"packages/**/*.{js,jsx,ts,tsx}\" \"scripts/**/*.{js,jsx,ts,tsx}\"",
-    "serve:docs": "lerna run serve",
+    "serve:docs": "yarn workspace @patternfly/react-docs serve",
     "serve:integration": "lerna run serve:demo-app",
     "start": "yarn build && concurrently --kill-others \"yarn watch\" \"yarn workspace @patternfly/react-docs develop\"",
     "start:cypress": "lerna run cypress:open",

diff --git a/packages/react-core/src/components/ChipGroup/ChipGroup.tsx b/packages/react-core/src/components/ChipGroup/ChipGroup.tsx
@@ -88,8 +88,14 @@ export class ChipGroup extends React.Component<ChipGroupProps, ChipGroupState> {
     const { isTooltipVisible } = this.state;
     return isTooltipVisible ? (
       <Tooltip position={tooltipPosition} content={categoryName}>
-        <span tabIndex={0} ref={this.headingRef} className={css(styles.chipGroupLabel)} aria-hidden="true" id={id}>
-          {categoryName}
+        <span
+          tabIndex={0}
+          ref={this.headingRef}
+          className={css(styles.chipGroupLabel)}
+          id={id}
+          aria-label={categoryName}
+        >
+          <span aria-hidden="true">{categoryName}</span>
         </span>
       </Tooltip>
     ) : (

diff --git a/packages/react-core/src/components/DataList/DataList.tsx b/packages/react-core/src/components/DataList/DataList.tsx
@@ -54,6 +54,7 @@ export const DataList: React.FunctionComponent<DataListProps> = ({
       <ul
         className={css(styles.dataList, isCompact && styles.modifiers.compact, className)}
         aria-label={ariaLabel}
+        role="listbox"
         {...props}
       >
         {children}

diff --git a/packages/react-core/src/components/DataList/DataListItem.tsx b/packages/react-core/src/components/DataList/DataListItem.tsx
@@ -66,6 +66,7 @@ export const DataListItem: React.FunctionComponent<DataListItemProps> = ({
             className
           )}
           aria-labelledby={ariaLabelledBy}
+          role="option"
           {...(isSelectable && { tabIndex: 0, onClick: selectDataListItem, onKeyDown })}
           {...(isSelectable && selectedDataListItemId === id && { 'aria-selected': true })}
           {...props}

diff --git a/...act-core/src/components/DataList/__tests__/Generated/__snapshots__/DataList.test.tsx.snap b/...act-core/src/components/DataList/__tests__/Generated/__snapshots__/DataList.test.tsx.snap
@@ -13,6 +13,7 @@ exports[`DataList should match snapshot (auto-generated) 1`] = `
   <ul
     aria-label="string"
     className="pf-c-data-list ''"
+    role="listbox"
   >
     ReactNode
   </ul>

diff --git a/packages/react-core/src/components/DataList/__tests__/__snapshots__/DataList.test.tsx.snap b/packages/react-core/src/components/DataList/__tests__/__snapshots__/DataList.test.tsx.snap
@@ -128,6 +128,7 @@ exports[`DataList List 1`] = `
   <ul
     aria-label="this is a simple list"
     className="pf-c-data-list data-list-custom"
+    role="listbox"
   />
 </ContextProvider>
 `;
@@ -145,6 +146,7 @@ exports[`DataList List compact 1`] = `
   <ul
     aria-label="this is a simple list"
     className="pf-c-data-list pf-m-compact"
+    role="listbox"
   />
 </ContextProvider>
 `;
@@ -162,6 +164,7 @@ exports[`DataList List default 1`] = `
   <ul
     aria-label="this is a simple list"
     className="pf-c-data-list"
+    role="listbox"
   />
 </ContextProvider>
 `;

diff --git a/packages/react-core/src/components/LabelGroup/LabelGroup.tsx b/packages/react-core/src/components/LabelGroup/LabelGroup.tsx
@@ -90,8 +90,14 @@ export class LabelGroup extends React.Component<LabelGroupProps, LabelGroupState
     const { isTooltipVisible } = this.state;
     return isTooltipVisible ? (
       <Tooltip position={tooltipPosition} content={categoryName}>
-        <span tabIndex={0} ref={this.headingRef} className={css(styles.labelGroupLabel)} aria-hidden="true" id={id}>
-          {categoryName}
+        <span
+          tabIndex={0}
+          ref={this.headingRef}
+          className={css(styles.labelGroupLabel)}
+          id={id}
+          aria-label={categoryName}
+        >
+          <span aria-hidden="true">{categoryName}</span>
         </span>
       </Tooltip>
     ) : (

diff --git a/packages/react-core/src/components/Page/PageHeader.tsx b/packages/react-core/src/components/Page/PageHeader.tsx
@@ -46,6 +46,7 @@ export const PageHeader: React.FunctionComponent<PageHeaderProps> = ({
   showNavToggle = false,
   onNavToggle = () => undefined as any,
   'aria-label': ariaLabel = 'Global navigation',
+  'aria-controls': ariaControls = null,
   ...props
 }: PageHeaderProps) => {
   const LogoComponent = logoComponent as any;
@@ -70,7 +71,7 @@ export const PageHeader: React.FunctionComponent<PageHeaderProps> = ({
                       id="nav-toggle"
                       onClick={navToggle}
                       aria-label={ariaLabel}
-                      aria-controls="page-sidebar"
+                      aria-controls={ariaControls}
                       aria-expanded={navOpen ? 'true' : 'false'}
                       variant={ButtonVariant.plain}
                     >
@@ -85,10 +86,6 @@ export const PageHeader: React.FunctionComponent<PageHeaderProps> = ({
                 )}
               </div>
             )}
-            {/* Hide for now until we have the context selector component */}
-            {/* <div className={css(styles.pageHeaderSelector)}>
-            pf-c-context-selector
-          </div> */}
             {topNav && <div className={css(styles.pageHeaderNav)}>{topNav}</div>}
             {headerTools}
           </header>

diff --git a/packages/react-core/src/demos/PrimaryDetail.md b/packages/react-core/src/demos/PrimaryDetail.md
@@ -401,7 +401,7 @@ class PrimaryDetailFullPage extends React.Component {
           selectedDataListItemId={selectedDataListItemId}
           onSelectDataListItem={this.onSelectDataListItem}
         >
-          <DataListItem aria-labelledby="selectable-action-item1" id="full-page-item1">
+          <DataListItem id="full-page-item1">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[
@@ -441,7 +441,7 @@ class PrimaryDetailFullPage extends React.Component {
               />
             </DataListItemRow>
           </DataListItem>
-          <DataListItem aria-labelledby="selectable-action-item2" id="full-page-item2">
+          <DataListItem id="full-page-item2">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[
@@ -488,7 +488,7 @@ class PrimaryDetailFullPage extends React.Component {
               />
             </DataListItemRow>
           </DataListItem>
-          <DataListItem aria-labelledby="selectable-action-item3" id="full-page-item3">
+          <DataListItem id="full-page-item3">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[
@@ -528,7 +528,7 @@ class PrimaryDetailFullPage extends React.Component {
               />
             </DataListItemRow>
           </DataListItem>
-          <DataListItem aria-labelledby="selectable-action-item4" id="full-page-item4">
+          <DataListItem id="full-page-item4">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[
@@ -986,7 +986,7 @@ class PrimaryDetailContentPadding extends React.Component {
           selectedDataListItemId={selectedDataListItemId}
           onSelectDataListItem={this.onSelectDataListItem}
         >
-          <DataListItem aria-labelledby="selectable-action-item1" id="content-padding-item1">
+          <DataListItem id="content-padding-item1">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[
@@ -1026,7 +1026,7 @@ class PrimaryDetailContentPadding extends React.Component {
               />
             </DataListItemRow>
           </DataListItem>
-          <DataListItem aria-labelledby="selectable-action-item2" id="content-padding-item2">
+          <DataListItem id="content-padding-item2">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[
@@ -1073,7 +1073,7 @@ class PrimaryDetailContentPadding extends React.Component {
               />
             </DataListItemRow>
           </DataListItem>
-          <DataListItem aria-labelledby="selectable-action-item3" id="content-padding-item3">
+          <DataListItem id="content-padding-item3">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[
@@ -1113,7 +1113,7 @@ class PrimaryDetailContentPadding extends React.Component {
               />
             </DataListItemRow>
           </DataListItem>
-          <DataListItem aria-labelledby="selectable-action-item4" id="content-padding-item4">
+          <DataListItem id="content-padding-item4">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[
@@ -2131,7 +2131,7 @@ class PrimaryDetailDataListInCard extends React.Component {
           selectedDataListItemId={selectedDataListItemId}
           onSelectDataListItem={this.onSelectDataListItem}
         >
-          <DataListItem aria-labelledby="data-list-item1-in-card" id="data-list-item1">
+          <DataListItem aria-label="data-list-item1-in-card" id="data-list-item1">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[
@@ -2143,7 +2143,7 @@ class PrimaryDetailDataListInCard extends React.Component {
               />
             </DataListItemRow>
           </DataListItem>
-          <DataListItem aria-labelledby="data-list-item2-in-card" id="data-list-item2">
+          <DataListItem aria-label="data-list-item2-in-card" id="data-list-item2">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[
@@ -2155,7 +2155,7 @@ class PrimaryDetailDataListInCard extends React.Component {
               />
             </DataListItemRow>
           </DataListItem>
-          <DataListItem aria-labelledby="data-list-item3-in-card" id="data-list-item3">
+          <DataListItem aria-label="data-list-item3-in-card" id="data-list-item3">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[
@@ -2167,7 +2167,7 @@ class PrimaryDetailDataListInCard extends React.Component {
               />
             </DataListItemRow>
           </DataListItem>
-          <DataListItem aria-labelledby="data-list-item4-in-card" id="data-list-item4">
+          <DataListItem aria-label="data-list-item4-in-card" id="data-list-item4">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[
@@ -2179,7 +2179,7 @@ class PrimaryDetailDataListInCard extends React.Component {
               />
             </DataListItemRow>
           </DataListItem>
-          <DataListItem aria-labelledby="data-list-item5-in-card" id="data-list-item5">
+          <DataListItem aria-label="data-list-item5-in-card" id="data-list-item5">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[
@@ -2603,7 +2603,7 @@ class PrimaryDetailInlineModifier extends React.Component {
           selectedDataListItemId={selectedDataListItemId}
           onSelectDataListItem={this.onSelectDataListItem}
         >
-          <DataListItem aria-labelledby="selectable-action-item1" id="inline-modifier-item1">
+          <DataListItem id="inline-modifier-item1">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[
@@ -2643,7 +2643,7 @@ class PrimaryDetailInlineModifier extends React.Component {
               />
             </DataListItemRow>
           </DataListItem>
-          <DataListItem aria-labelledby="selectable-action-item2" id="inline-modifier-item2">
+          <DataListItem id="inline-modifier-item2">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[
@@ -2690,7 +2690,7 @@ class PrimaryDetailInlineModifier extends React.Component {
               />
             </DataListItemRow>
           </DataListItem>
-          <DataListItem aria-labelledby="selectable-action-item3" id="inline-modifier-item3">
+          <DataListItem id="inline-modifier-item3">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[
@@ -2730,7 +2730,7 @@ class PrimaryDetailInlineModifier extends React.Component {
               />
             </DataListItemRow>
           </DataListItem>
-          <DataListItem aria-labelledby="selectable-action-item4" id="inline-modifier-item4">
+          <DataListItem id="inline-modifier-item4">
             <DataListItemRow>
               <DataListItemCells
                 dataListCells={[

diff --git a/packages/react-docs/package.json b/packages/react-docs/package.json
@@ -18,7 +18,8 @@
     "clean": "rimraf .cache public static/assets static/base.css src/generated/**/*.js",
     "develop": "theme-patternfly-org start",
     "serve": "serve public",
-    "screenshots": "theme-patternfly-org screenshots --urlPrefix http://localhost:5000"
+    "screenshots": "theme-patternfly-org screenshots --urlPrefix http://localhost:5000",
+    "test:a11y": "patternfly-a11y --config patternfly-a11y.config"
   },
   "dependencies": {
     "@patternfly/patternfly": "4.42.2",
@@ -34,6 +35,7 @@
     "@patternfly/react-virtualized-extension": "^4.5.87"
   },
   "devDependencies": {
+    "@patternfly/patternfly-a11y": "^2.0.10",
     "@types/react": "^16.8.0",
     "@types/react-dom": "^16.8.0",
     "react": "^16.8.0",