Accessibility fixes

- Ensure no duplicate table ids on the About page - Ensure links contain text (setting `title` on the anchor tag isn't enough)
tektoncd · Jul 28, 2020 · e87482d · e87482d
1 parent 74fe8f5
commit e87482d
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 23 deletions.
diff --git a/packages/components/src/components/Table/Table.js b/packages/components/src/components/Table/Table.js
@@ -126,6 +126,7 @@ const Table = props => {
     emptyTextSelectedNamespace,
     filters,
     headers: dataHeaders,
+    id,
     intl,
     isSortable,
     loading,
@@ -150,7 +151,7 @@ const Table = props => {
   });
 
   return (
-    <div className={tableClassNames}>
+    <div className={tableClassNames} id={id}>
       <DataTable
         key={selectedNamespace}
         rows={dataRows}
@@ -255,7 +256,7 @@ const Table = props => {
                         {row.cells.map(cell => (
                           <TableCell
                             key={cell.id}
-                            id={cell.id}
+                            id={id ? `${id}:${cell.id}` : cell.id}
                             className={`cell-${cell.info.header}`}
                             {...(typeof cell.value === 'string' && {
                               title: cell.value
@@ -281,6 +282,7 @@ Table.defaultProps = {
   batchActionButtons: [],
   emptyTextAllNamespaces: null,
   emptyTextSelectedNamespace: null,
+  id: null,
   isSortable: false,
   loading: false,
   selectedNamespace: null,
@@ -293,6 +295,7 @@ Table.propTypes = {
   emptyTextAllNamespaces: PropTypes.string,
   emptyTextSelectedNamespace: PropTypes.string,
   headers: PropTypes.arrayOf(PropTypes.object).isRequired,
+  id: PropTypes.string,
   isSortable: PropTypes.bool,
   loading: PropTypes.bool,
   rows: PropTypes.arrayOf(PropTypes.object).isRequired,

diff --git a/src/containers/About/About.js b/src/containers/About/About.js
@@ -156,7 +156,7 @@ export /* istanbul ignore next */ class About extends Component {
         <div className="tkn--about--content">
           <div className="tkn--about--tables">
             <Table
-              title="Dashboard"
+              id="tkn--about--dashboard-table"
               headers={headers}
               rows={[
                 getRow('Namespace', this.props.dashboardNamespace),
@@ -166,28 +166,28 @@ export /* istanbul ignore next */ class About extends Component {
                 getRow(logoutURLLabel, this.props.logoutURL)
               ].filter(Boolean)}
               size="short"
-              className="tkn--about--dashboard-table"
+              title="Dashboard"
             />
             <Table
-              title="Pipelines"
               headers={headers}
+              id="tkn--about--pipelines-table"
               rows={[
                 getRow('Namespace', this.props.pipelinesNamespace),
                 getRow(versionLabel, this.props.pipelinesVersion)
               ].filter(Boolean)}
               size="short"
-              className="tkn--about--pipelines-table"
+              title="Pipelines"
             />
             {this.props.isTriggersInstalled && (
               <Table
-                title="Triggers"
                 headers={headers}
+                id="tkn--about--triggers-table"
                 rows={[
                   getRow('Namespace', this.props.triggersNamespace),
                   getRow(versionLabel, this.props.triggersVersion)
                 ].filter(Boolean)}
                 size="short"
-                className="tkn--about--triggers-table"
+                title="Triggers"
               />
             )}
           </div>

diff --git a/src/containers/About/About.test.js b/src/containers/About/About.test.js
@@ -19,9 +19,9 @@ import thunk from 'redux-thunk';
 import { renderWithIntl } from '../../utils/test';
 import About from '.';
 
-const dashboardSelector = { selector: '.tkn--about--dashboard-table *' };
-const pipelinesSelector = { selector: '.tkn--about--pipelines-table *' };
-const triggersSelector = { selector: '.tkn--about--triggers-table *' };
+const dashboardSelector = { selector: '#tkn--about--dashboard-table *' };
+const pipelinesSelector = { selector: '#tkn--about--pipelines-table *' };
+const triggersSelector = { selector: '#tkn--about--triggers-table *' };
 const middleware = [thunk];
 const mockStore = configureStore(middleware);
 

diff --git a/src/containers/LogDownloadButton/LogDownloadButton.js b/src/containers/LogDownloadButton/LogDownloadButton.js
@@ -32,26 +32,32 @@ const LogDownloadButton = ({ intl, stepStatus, taskRun }) => {
       <div className="bx--btn-set">
         <a
           className="bx--copy-btn"
-          title={intl.formatMessage({
-            id: 'dashboard.logs.launchButtonTooltip',
-            defaultMessage: 'Open logs in a new window'
-          })}
           href={logURL}
           target="_blank"
           rel="noopener noreferrer"
         >
-          <Launch16 />
+          <Launch16>
+            <title>
+              {intl.formatMessage({
+                id: 'dashboard.logs.launchButtonTooltip',
+                defaultMessage: 'Open logs in a new window'
+              })}
+            </title>
+          </Launch16>
         </a>
         <a
           className="bx--copy-btn"
-          title={intl.formatMessage({
-            id: 'dashboard.logs.downloadButtonTooltip',
-            defaultMessage: 'Download logs'
-          })}
           download={`${pod}__${container}__log.txt`}
           href={logURL}
         >
-          <Download16 />
+          <Download16>
+            <title>
+              {intl.formatMessage({
+                id: 'dashboard.logs.downloadButtonTooltip',
+                defaultMessage: 'Download logs'
+              })}
+            </title>
+          </Download16>
         </a>
       </div>
     </>

diff --git a/src/containers/Pipelines/Pipelines.js b/src/containers/Pipelines/Pipelines.js
@@ -123,7 +123,17 @@ export /* istanbul ignore next */ class Pipelines extends Component {
             name: pipeline.metadata.name
           })}
         >
-          <Information16 className="tkn--resource-info-icon" />
+          <Information16 className="tkn--resource-info-icon">
+            <title>
+              {intl.formatMessage(
+                {
+                  id: 'dashboard.resourceList.viewDetails',
+                  defaultMessage: 'View {resource}'
+                },
+                { resource: pipeline.metadata.name }
+              )}
+            </title>
+          </Information16>
         </Link>
       )
     }));

diff --git a/src/containers/Tasks/Tasks.js b/src/containers/Tasks/Tasks.js
@@ -123,7 +123,17 @@ export /* istanbul ignore next */ class Tasks extends Component {
             name: task.metadata.name
           })}
         >
-          <Information16 className="tkn--resource-info-icon" />
+          <Information16 className="tkn--resource-info-icon">
+            <title>
+              {intl.formatMessage(
+                {
+                  id: 'dashboard.resourceList.viewDetails',
+                  defaultMessage: 'View {resource}'
+                },
+                { resource: task.metadata.name }
+              )}
+            </title>
+          </Information16>
         </Link>
       )
     }));

diff --git a/src/nls/messages_en.json b/src/nls/messages_en.json
@@ -207,6 +207,7 @@
     "dashboard.resourceDetails.errorloading": "Error loading resource",
     "dashboard.resourceList.emptyState": "No resources for type {type}.",
     "dashboard.resourceList.errorLoading": "Error loading {type}",
+    "dashboard.resourceList.viewDetails": "View {resource}",
     "dashboard.secret.errorLoading": "Error loading Secret",
     "dashboard.secret.failed": "Cannot load Secret",
     "dashboard.secret.failedSubtitle": "Secret {secretName} not found",