From fc52bf98f417d56527e25ebb4dd43b1ffe519ff0 Mon Sep 17 00:00:00 2001
From: Mell Y <pong7219@gmail.com>
Date: Wed, 11 Dec 2019 10:52:18 -0300
Subject: [PATCH] Bug 1600752 - changes based on feedback

---
 ui/css/perf.css                           |   8 ++
 ui/perfherder/ProgressBar.jsx             |  31 +++--
 ui/perfherder/alerts/AlertTable.jsx       |  18 +--
 ui/perfherder/alerts/AlertTableRow.jsx    |  19 +--
 ui/perfherder/compare/CompareTable.jsx    | 146 +---------------------
 ui/perfherder/compare/CompareTableRow.jsx |   1 +
 ui/shared/TruncatedText.jsx               |   7 +-
 7 files changed, 51 insertions(+), 179 deletions(-)

diff --git a/ui/css/perf.css b/ui/css/perf.css
index a372835a476..a8dd9d74114 100644
--- a/ui/css/perf.css
+++ b/ui/css/perf.css
@@ -308,6 +308,14 @@ th {
 .compare-table tr .result-links a,
 .compare-table tr .result-links button {
   color: #23527c;
+  opacity: 0.5;
+}
+
+.compare-table tr .result-links a:hover,
+.compare-table tr .result-links button:hover,
+.compare-table tr .result-links a:focus,
+.compare-table tr .result-links button:focus {
+  opacity: 1;
 }
 
 .compare-table tr:hover .retrigger-btn {
diff --git a/ui/perfherder/ProgressBar.jsx b/ui/perfherder/ProgressBar.jsx
index 0372bf3947b..338624ff4f2 100644
--- a/ui/perfherder/ProgressBar.jsx
+++ b/ui/perfherder/ProgressBar.jsx
@@ -7,21 +7,26 @@ import SimpleTooltip from '../shared/SimpleTooltip';
 const ProgressBar = ({ magnitude, regression, color }) => (
   <SimpleTooltip
     text={
-      <Progress multi aria-label="Magnitude of Difference">
+      <Progress
+        multi
+        aria-label={`Magnitude of difference based on lower is better. Metric: ${
+          regression ? 100 - magnitude : magnitude
+        } % regressed`}
+      >
         {/* the % of the bars that are colored and transparent is based on the newIsBetter metric,
         which determines whether the colored bar for magnitude is displayed on the left or right */}
-        <span className="sr-only sr-only-focusable">Magnitude 1</span>
-        <Progress
-          bar
-          value={regression ? 100 - magnitude : magnitude}
-          color={regression ? 'transparent' : color}
-        />
-        <span className="sr-only sr-only-focusable">Magnitude 2</span>
-        <Progress
-          bar
-          value={regression ? magnitude : 100 - magnitude}
-          color={regression ? color : 'transparent'}
-        />
+        <div aria-hidden="true" className="progress w-100">
+          <Progress
+            bar
+            value={regression ? 100 - magnitude : magnitude}
+            color={regression ? 'transparent' : color}
+          />
+          <Progress
+            bar
+            value={regression ? magnitude : 100 - magnitude}
+            color={regression ? color : 'transparent'}
+          />
+        </div>
       </Progress>
     }
     tooltipText="Relative magnitude of change (scale from 0 - 20%+)"
diff --git a/ui/perfherder/alerts/AlertTable.jsx b/ui/perfherder/alerts/AlertTable.jsx
index a330561bf6b..146cbc59996 100644
--- a/ui/perfherder/alerts/AlertTable.jsx
+++ b/ui/perfherder/alerts/AlertTable.jsx
@@ -248,15 +248,15 @@ export default class AlertTable extends React.Component {
                   </tr>
                 </thead>
                 <tbody>
-                  <tr>
-                    <th>&nbsp;</th>
-                    <th>&nbsp;</th>
-                    <th>Alert Name</th>
-                    <th>&nbsp;</th>
-                    <th>&nbsp;</th>
-                    <th>&nbsp;</th>
-                    <th>Detail Hint</th>
-                    <th>Magnitude</th>
+                  <tr className="border">
+                    <th> </th>
+                    <th> </th>
+                    <th>Test and platform</th>
+                    <th>Previous Value</th>
+                    <th> </th>
+                    <th>New Value</th>
+                    <th>Absolute Difference</th>
+                    <th>Magnitude of Change</th>
                     <th>Confidence</th>
                   </tr>
                   {filteredAlerts.map(alert => (
diff --git a/ui/perfherder/alerts/AlertTableRow.jsx b/ui/perfherder/alerts/AlertTableRow.jsx
index 2f9d817f374..48fcaeb966f 100644
--- a/ui/perfherder/alerts/AlertTableRow.jsx
+++ b/ui/perfherder/alerts/AlertTableRow.jsx
@@ -2,7 +2,7 @@
 
 import React from 'react';
 import PropTypes from 'prop-types';
-import { FormGroup, Input, Label } from 'reactstrap';
+import { Button, FormGroup, Input, Label } from 'reactstrap';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
 import {
   faStar as faStarSolid,
@@ -208,13 +208,9 @@ export default class AlertTableRow extends React.Component {
       >
         <td className="table-width-xs px-1">
           <FormGroup check className="ml-2 pl-4">
-            <Label for={alert.id} hidden>
-              alert {alert.id} title
-            </Label>
+            <Label hidden>alert {alert.id} title</Label>
             <Input
-              id={alert.id}
               aria-label={`alert ${alert.id} title`}
-              aria-labelledby={`alert ${alert.id} title`}
               data-testid={`alert ${alert.id} checkbox`}
               type="checkbox"
               disabled={!user.isStaff}
@@ -229,8 +225,13 @@ export default class AlertTableRow extends React.Component {
           </FormGroup>
         </td>
         <td className="px-0">
-          <button
-            type="button"
+          <Button
+            color="black"
+            aria-label={
+              starred
+                ? 'Remove bookmark from this Alert'
+                : 'Bookmark this Alert'
+            }
             className={
               starred
                 ? 'visible border border-0 bg-transparent'
@@ -243,7 +244,7 @@ export default class AlertTableRow extends React.Component {
               title={starred ? 'starred' : 'not starred'}
               icon={starred ? faStarSolid : faStarRegular}
             />
-          </button>
+          </Button>
         </td>
         <td className="text-left">
           {alertStatus !== 'untriaged' ? (
diff --git a/ui/perfherder/compare/CompareTable.jsx b/ui/perfherder/compare/CompareTable.jsx
index d492336bdef..c5346c3cb57 100644
--- a/ui/perfherder/compare/CompareTable.jsx
+++ b/ui/perfherder/compare/CompareTable.jsx
@@ -51,7 +51,8 @@ export default class CompareTable extends React.PureComponent {
                       this.header,
                     )
                   }
-                  title={`Permalink to this test table: ${testName}`}
+                  title="Permalink to this test table"
+                  aria-label={`Permalink to test ${testName} table`}
                 >
                   <FontAwesomeIcon icon={faHashtag} />
                 </Button>
@@ -62,7 +63,6 @@ export default class CompareTable extends React.PureComponent {
             <th className="table-width-sm" />
             <th className="table-width-lg">New</th>
             <th className="table-width-lg">Delta</th>
-            {/* empty for progress bars (magnitude of difference) */}
             <th className="table-width-lg">Magnitude of Difference</th>
             <th className="table-width-lg">Confidence</th>
             <th className="text-right table-width-md">
@@ -87,152 +87,10 @@ export default class CompareTable extends React.PureComponent {
           {data.map(rowLevelResults => (
             <CompareTableRow
               key={rowLevelResults.name}
-<<<<<<< 39891e0e01f1ef9eeed451d1efada0462f91150f
               rowLevelResults={rowLevelResults}
               hashFunction={hashFunction}
               {...this.props}
             />
-=======
-            >
-              <th className="text-left font-weight-normal pl-1">
-                {rowLevelResults.name}
-                <span className="result-links">
-                  {onPermalinkClick && (
-                    <span>
-                      <Button
-                        className="permalink p-0 ml-1"
-                        color="link"
-                        onClick={() =>
-                          onPermalinkClick(
-                            this.getHashBasedId(testName, rowLevelResults.name),
-                          )
-                        }
-                        title={`Permalink to this test: ${rowLevelResults.name}`}
-                      >
-                        <FontAwesomeIcon icon={faHashtag} />
-                      </Button>
-                    </span>
-                  )}
-                  {rowLevelResults.links &&
-                    rowLevelResults.links.map(link => (
-                      <span key={link.title}>
-                        <a href={link.href}>{` ${link.title}`}</a>
-                      </span>
-                    ))}
-                </span>
-              </th>
-              <TableAverage
-                value={rowLevelResults.originalValue}
-                stddev={rowLevelResults.originalStddev}
-                stddevpct={rowLevelResults.originalStddevPct}
-                replicates={rowLevelResults.originalRuns}
-              />
-              <td>
-                {rowLevelResults.originalValue < rowLevelResults.newValue && (
-                  <span>&lt;</span>
-                )}
-                {rowLevelResults.originalValue > rowLevelResults.newValue && (
-                  <span>&gt;</span>
-                )}
-              </td>
-              <TableAverage
-                value={rowLevelResults.newValue}
-                stddev={rowLevelResults.newStddev}
-                stddevpct={rowLevelResults.newStddevPct}
-                replicates={rowLevelResults.newRuns}
-              />
-              <td className={this.getColorClass(rowLevelResults, 'background')}>
-                {rowLevelResults.delta &&
-                Math.abs(displayNumber(rowLevelResults.deltaPercentage)) !==
-                  0 ? (
-                  <SimpleTooltip
-                    textClass="detail-hint"
-                    text={
-                      <React.Fragment>
-                        {(rowLevelResults.isRegression ||
-                          rowLevelResults.isImprovement) && (
-                          <FontAwesomeIcon
-                            icon={
-                              rowLevelResults.isRegression
-                                ? faExclamationTriangle
-                                : faThumbsUp
-                            }
-                            title={
-                              rowLevelResults.isRegression
-                                ? 'regression'
-                                : 'improvement'
-                            }
-                            className={this.getColorClass(
-                              rowLevelResults,
-                              'text',
-                            )}
-                            size="lg"
-                          />
-                        )}
-                        {`  ${displayNumber(rowLevelResults.deltaPercentage)}%`}
-                      </React.Fragment>
-                    }
-                    tooltipText={this.deltaTooltipText(
-                      rowLevelResults.delta,
-                      rowLevelResults.deltaPercentage,
-                      rowLevelResults.newIsBetter,
-                    )}
-                  />
-                ) : null}
-                {rowLevelResults.delta
-                  ? Math.abs(displayNumber(rowLevelResults.deltaPercentage)) ===
-                      0 && (
-                      <span>
-                        {displayNumber(rowLevelResults.deltaPercentage)}%
-                      </span>
-                    )
-                  : null}
-              </td>
-              <td>
-                {rowLevelResults.delta ? (
-                  <ProgressBar
-                    magnitude={rowLevelResults.magnitude}
-                    regression={!rowLevelResults.newIsBetter}
-                    color={this.getColorClass(rowLevelResults, 'bar')}
-                  />
-                ) : null}
-              </td>
-              <td>
-                {rowLevelResults.delta &&
-                rowLevelResults.confidence &&
-                rowLevelResults.confidenceText ? (
-                  <SimpleTooltip
-                    textClass="detail-hint"
-                    text={`${displayNumber(rowLevelResults.confidence)} (${
-                      rowLevelResults.confidenceText
-                    })`}
-                    tooltipText={rowLevelResults.confidenceTextLong}
-                  />
-                ) : null}
-              </td>
-              <td className="text-right">
-                {!hasSubtests &&
-                  !rowLevelResults.isNoiseMetric &&
-                  (rowLevelResults.newRetriggerableJobId || !isBaseAggregate) &&
-                  user.isLoggedIn && (
-                    <Button
-                      className="retrigger-btn btn icon-green mr-1 py-0 px-1"
-                      title={compareTableText.retriggerButtonTitle}
-                      onClick={() => this.retriggerJobs(rowLevelResults, 5)}
-                    >
-                      <FontAwesomeIcon icon={faRedo} />
-                    </Button>
-                  )}
-                {rowLevelResults.originalRuns && (
-                  <SimpleTooltip
-                    textClass="detail-hint"
-                    text={`${rowLevelResults.originalRuns.length} / ${rowLevelResults.newRuns.length}`}
-                    tooltipText={`${rowLevelResults.originalRuns.length} base / ${rowLevelResults.newRuns.length} new`}
-                  />
-                )}
-              </td>
-            </tr>
->>>>>>> Bug 1600752 - Compare View Changes
           ))}
         </tbody>
       </Table>
diff --git a/ui/perfherder/compare/CompareTableRow.jsx b/ui/perfherder/compare/CompareTableRow.jsx
index 66e6a10a192..1480d94af5e 100644
--- a/ui/perfherder/compare/CompareTableRow.jsx
+++ b/ui/perfherder/compare/CompareTableRow.jsx
@@ -77,6 +77,7 @@ export default class CompareTableRow extends React.PureComponent {
                     )
                   }
                   title="Permalink to this test"
+                  aria-label={`Permalink to test ${rowLevelResults.name}`}
                 >
                   <FontAwesomeIcon icon={faHashtag} />
                 </Button>
diff --git a/ui/shared/TruncatedText.jsx b/ui/shared/TruncatedText.jsx
index 2c686a9fb16..9b89e748803 100644
--- a/ui/shared/TruncatedText.jsx
+++ b/ui/shared/TruncatedText.jsx
@@ -1,6 +1,5 @@
-/* eslint-disable jsx-a11y/no-noninteractive-element-interactions */
-
 import React from 'react';
+import { Button } from 'reactstrap';
 import PropTypes from 'prop-types';
 
 export default class TruncatedText extends React.Component {
@@ -22,12 +21,12 @@ export default class TruncatedText extends React.Component {
           {text}
         </p>
         {text.length > maxLength && (
-          <p
+          <Button
             className={`${showMoreClass} mb-0 text-right  pointer`}
             onClick={() => this.setState({ showMoreResults: !showMoreResults })}
           >
             {`show ${showMoreResults ? 'less' : 'more'}`}
-          </p>
+          </Button>
         )}
       </React.Fragment>
     );