From cfbcc12398fd38f0d410af69d05a8cd01bb4d48b Mon Sep 17 00:00:00 2001
From: David Walsh <davidwalsh83@gmail.com>
Date: Thu, 5 Nov 2020 12:05:41 -0600
Subject: [PATCH] Make all UI tabs accessible via keyboard (#9518)

---
 test/e2e/metamask-ui.spec.js                  | 18 ++++++++-----
 .../app/asset-list-item/asset-list-item.js    | 14 ++++++++---
 .../app/asset-list-item/asset-list-item.scss  | 25 +++++++++++++++----
 .../confirm-page-container-content/index.scss |  6 +++++
 .../gas-modal-page-container/index.scss       | 11 ++++++--
 .../app/selected-account/index.scss           |  2 ++
 .../selected-account.component.js             |  4 +--
 ui/app/components/app/tab-bar/index.scss      |  2 ++
 ui/app/components/app/tab-bar/tab-bar.js      |  4 +--
 ui/app/components/ui/list-item/index.scss     |  9 ++++++-
 .../ui/list-item/list-item.component.js       |  4 ++-
 ui/app/components/ui/tabs/tab/index.scss      | 15 ++++++++---
 .../components/ui/tabs/tab/tab.component.js   |  2 +-
 ui/app/pages/add-token/token-list/index.scss  |  3 ++-
 .../token-list/token-list.component.js        |  8 +++---
 ui/app/pages/home/index.scss                  |  5 +++-
 16 files changed, 99 insertions(+), 33 deletions(-)

diff --git a/test/e2e/metamask-ui.spec.js b/test/e2e/metamask-ui.spec.js
index 32ad702b3..b98cf9f0b 100644
--- a/test/e2e/metamask-ui.spec.js
+++ b/test/e2e/metamask-ui.spec.js
@@ -846,7 +846,7 @@ describe('MetaMask', function () {
     })
 
     it('displays the contract creation data', async function () {
-      await driver.clickElement(By.xpath(`//li[contains(text(), 'Data')]`))
+      await driver.clickElement(By.xpath(`//button[contains(text(), 'Data')]`))
       await driver.delay(regularDelayMs)
 
       await driver.findElement(By.xpath(`//div[contains(text(), '127.0.0.1')]`))
@@ -860,7 +860,9 @@ describe('MetaMask', function () {
       assert.ok(confirmDataText.includes('Bytes:'))
       assert.ok(confirmDataText.includes('675'))
 
-      await driver.clickElement(By.xpath(`//li[contains(text(), 'Details')]`))
+      await driver.clickElement(
+        By.xpath(`//button[contains(text(), 'Details')]`),
+      )
       await driver.delay(regularDelayMs)
     })
 
@@ -1055,7 +1057,9 @@ describe('MetaMask', function () {
       )
       await driver.delay(regularDelayMs)
 
-      await driver.clickElement(By.xpath(`//li[contains(text(), 'Advanced')]`))
+      await driver.clickElement(
+        By.xpath(`//button[contains(text(), 'Advanced')]`),
+      )
       await driver.delay(tinyDelayMs)
 
       const [gasPriceInput, gasLimitInput] = await driver.findElements(
@@ -1098,7 +1102,7 @@ describe('MetaMask', function () {
 
     it('picks the newly created Test token', async function () {
       await driver.clickElement(
-        By.xpath("//li[contains(text(), 'Custom Token')]"),
+        By.xpath("//button[contains(text(), 'Custom Token')]"),
       )
       await driver.delay(regularDelayMs)
 
@@ -1165,7 +1169,7 @@ describe('MetaMask', function () {
     })
 
     it('displays the token transfer data', async function () {
-      await driver.clickElement(By.xpath(`//li[contains(text(), 'Data')]`))
+      await driver.clickElement(By.xpath(`//button[contains(text(), 'Data')]`))
       await driver.delay(regularDelayMs)
 
       const functionType = await driver.findElement(
@@ -1192,7 +1196,9 @@ describe('MetaMask', function () {
         ),
       )
 
-      await driver.clickElement(By.xpath(`//li[contains(text(), 'Details')]`))
+      await driver.clickElement(
+        By.xpath(`//button[contains(text(), 'Details')]`),
+      )
       await driver.delay(regularDelayMs)
     })
 
diff --git a/ui/app/components/app/asset-list-item/asset-list-item.js b/ui/app/components/app/asset-list-item/asset-list-item.js
index 27e7eab34..dfaf77db7 100644
--- a/ui/app/components/app/asset-list-item/asset-list-item.js
+++ b/ui/app/components/app/asset-list-item/asset-list-item.js
@@ -93,10 +93,16 @@ const AssetListItem = ({
       className={classnames('asset-list-item', className)}
       data-testid={dataTestId}
       title={
-        <>
-          <h2 className="asset-list-item__token-value">{primary}</h2>
-          <h2 className="asset-list-item__token-symbol">{tokenSymbol}</h2>
-        </>
+        <button
+          className="asset-list-item__token-button"
+          onClick={onClick}
+          title={`${primary} ${tokenSymbol || ''}`}
+        >
+          <h2>
+            <span className="asset-list-item__token-value">{primary}</span>
+            <span className="asset-list-item__token-symbol">{tokenSymbol}</span>
+          </h2>
+        </button>
       }
       titleIcon={titleIcon}
       subtitle={<h3 title={secondary}>{secondary}</h3>}
diff --git a/ui/app/components/app/asset-list-item/asset-list-item.scss b/ui/app/components/app/asset-list-item/asset-list-item.scss
index 89047462b..9ad041e7a 100644
--- a/ui/app/components/app/asset-list-item/asset-list-item.scss
+++ b/ui/app/components/app/asset-list-item/asset-list-item.scss
@@ -1,9 +1,24 @@
 .asset-list-item {
-  &__token-value {
-    padding: 0 5px 0 0;
-    overflow: hidden;
-    white-space: nowrap;
-    text-overflow: ellipsis;
+  & .list-item__heading {
+    max-width: 100%;
+  }
+
+  &__token-button {
+    padding-inline-start: 0;
+    min-width: 0;
+    min-height: 0;
+    text-align: start;
+
+    & h2 {
+      overflow: hidden;
+      white-space: nowrap;
+      text-overflow: ellipsis;
+      max-width: 100%;
+    }
+
+    & span {
+      padding-right: 5px;
+    }
   }
 
   &__chevron-right {
diff --git a/ui/app/components/app/confirm-page-container/confirm-page-container-content/index.scss b/ui/app/components/app/confirm-page-container/confirm-page-container-content/index.scss
index 94ff2b711..c45f5d11f 100644
--- a/ui/app/components/app/confirm-page-container/confirm-page-container-content/index.scss
+++ b/ui/app/components/app/confirm-page-container/confirm-page-container-content/index.scss
@@ -77,6 +77,12 @@
     color: #8c8e94;
     text-transform: uppercase;
     margin: 0 8px;
+
+    & button {
+      font-size: unset;
+      color: #8c8e94;
+      text-transform: uppercase;
+    }
   }
 
   .page-container__footer {
diff --git a/ui/app/components/app/gas-customization/gas-modal-page-container/index.scss b/ui/app/components/app/gas-customization/gas-modal-page-container/index.scss
index 3c255dc6b..fd09c01ee 100644
--- a/ui/app/components/app/gas-customization/gas-modal-page-container/index.scss
+++ b/ui/app/components/app/gas-customization/gas-modal-page-container/index.scss
@@ -57,6 +57,10 @@
       display: none;
     }
 
+    &__tab {
+      margin-right: 0;
+    }
+
     &__tabs {
       margin-top: 0;
     }
@@ -70,9 +74,12 @@
         margin-right: 0;
       }
 
-      &--selected {
+      & button {
+        font-size: unset;
+      }
+
+      &.tab--active button {
         color: $primary-blue;
-        border-bottom: 2px solid $primary-blue;
       }
     }
   }
diff --git a/ui/app/components/app/selected-account/index.scss b/ui/app/components/app/selected-account/index.scss
index 3f1df50b2..05757b73e 100644
--- a/ui/app/components/app/selected-account/index.scss
+++ b/ui/app/components/app/selected-account/index.scss
@@ -37,6 +37,8 @@
     padding: 6px 1px;
     border-radius: 10px;
     cursor: pointer;
+    width: 100%;
+    background-color: unset;
 
     &:hover {
       background-color: $Grey-000;
diff --git a/ui/app/components/app/selected-account/selected-account.component.js b/ui/app/components/app/selected-account/selected-account.component.js
index 36ce7c46c..b3b04b5d0 100644
--- a/ui/app/components/app/selected-account/selected-account.component.js
+++ b/ui/app/components/app/selected-account/selected-account.component.js
@@ -43,7 +43,7 @@ class SelectedAccount extends Component {
             this.state.copied ? t('copiedExclamation') : t('copyToClipboard')
           }
         >
-          <div
+          <button
             className="selected-account__clickable"
             onClick={() => {
               this.setState({ copied: true })
@@ -60,7 +60,7 @@ class SelectedAccount extends Component {
             <div className="selected-account__address">
               {shortenAddress(checksummedAddress)}
             </div>
-          </div>
+          </button>
         </Tooltip>
       </div>
     )
diff --git a/ui/app/components/app/tab-bar/index.scss b/ui/app/components/app/tab-bar/index.scss
index f05cea784..1e20d1fed 100644
--- a/ui/app/components/app/tab-bar/index.scss
+++ b/ui/app/components/app/tab-bar/index.scss
@@ -16,6 +16,8 @@
     padding: 16px 24px;
     opacity: 0.5;
     transition: opacity 200ms ease-in-out;
+    background-color: unset;
+    text-align: start;
 
     @media screen and (min-width: 576px) {
       &:hover {
diff --git a/ui/app/components/app/tab-bar/tab-bar.js b/ui/app/components/app/tab-bar/tab-bar.js
index 15083cb85..e4f38feac 100644
--- a/ui/app/components/app/tab-bar/tab-bar.js
+++ b/ui/app/components/app/tab-bar/tab-bar.js
@@ -8,7 +8,7 @@ const TabBar = (props) => {
   return (
     <div className="tab-bar">
       {tabs.map(({ key, content, description }) => (
-        <div
+        <button
           key={key}
           className={classnames('tab-bar__tab pointer', {
             'tab-bar__tab--active': isActive(key, content),
@@ -22,7 +22,7 @@ const TabBar = (props) => {
             </div>
           </div>
           <div className="tab-bar__tab__caret" />
-        </div>
+        </button>
       ))}
     </div>
   )
diff --git a/ui/app/components/ui/list-item/index.scss b/ui/app/components/ui/list-item/index.scss
index 670d25748..5470a692c 100644
--- a/ui/app/components/ui/list-item/index.scss
+++ b/ui/app/components/ui/list-item/index.scss
@@ -19,7 +19,8 @@
   align-items: start;
   cursor: pointer;
 
-  &:hover {
+  &:hover,
+  &:focus-within {
     background-color: $Grey-000;
   }
 
@@ -44,6 +45,12 @@
     display: flex;
     align-items: center;
 
+    & button {
+      background: unset;
+      font-size: unset;
+      padding-inline-start: 0;
+    }
+
     &-wrap {
       display: inline-block;
       margin-left: 8px;
diff --git a/ui/app/components/ui/list-item/list-item.component.js b/ui/app/components/ui/list-item/list-item.component.js
index 46ab65ac9..8ac979dd5 100644
--- a/ui/app/components/ui/list-item/list-item.component.js
+++ b/ui/app/components/ui/list-item/list-item.component.js
@@ -27,7 +27,9 @@ export default function ListItem({
         {React.isValidElement(title) ? (
           title
         ) : (
-          <h2 className="list-item__title">{title}</h2>
+          <button onClick={onClick}>
+            <h2 className="list-item__title">{title}</h2>
+          </button>
         )}
         {titleIcon && (
           <div className="list-item__heading-wrap">{titleIcon}</div>
diff --git a/ui/app/components/ui/tabs/tab/index.scss b/ui/app/components/ui/tabs/tab/index.scss
index b0fb0d30e..0753ba9dd 100644
--- a/ui/app/components/ui/tabs/tab/index.scss
+++ b/ui/app/components/ui/tabs/tab/index.scss
@@ -1,8 +1,15 @@
 .tab {
-  cursor: pointer;
-  padding: 8px;
-  min-width: 50px;
-  text-align: center;
+  button {
+    cursor: pointer;
+    padding: 8px;
+    min-width: 50px;
+    text-align: center;
+    display: block;
+    width: 100%;
+    background-color: unset;
+    font-size: unset;
+    color: unset;
+  }
 
   &--active {
     color: $black;
diff --git a/ui/app/components/ui/tabs/tab/tab.component.js b/ui/app/components/ui/tabs/tab/tab.component.js
index 02c9c0b9b..e88deaca4 100644
--- a/ui/app/components/ui/tabs/tab/tab.component.js
+++ b/ui/app/components/ui/tabs/tab/tab.component.js
@@ -25,7 +25,7 @@ const Tab = (props) => {
         onClick(tabIndex)
       }}
     >
-      {name}
+      <button>{name}</button>
     </li>
   )
 }
diff --git a/ui/app/pages/add-token/token-list/index.scss b/ui/app/pages/add-token/token-list/index.scss
index 185f91d53..f9f296098 100644
--- a/ui/app/pages/add-token/token-list/index.scss
+++ b/ui/app/pages/add-token/token-list/index.scss
@@ -23,7 +23,8 @@
     border: 1px solid transparent;
     position: relative;
 
-    &:hover {
+    &:hover,
+    &:focus {
       border-color: rgba($malibu-blue, 0.5);
     }
 
diff --git a/ui/app/pages/add-token/token-list/token-list.component.js b/ui/app/pages/add-token/token-list/token-list.component.js
index e8c71ad07..83e9c352d 100644
--- a/ui/app/pages/add-token/token-list/token-list.component.js
+++ b/ui/app/pages/add-token/token-list/token-list.component.js
@@ -37,6 +37,8 @@ export default class InfoBox extends Component {
             .map((_, i) => {
               const { logo, symbol, name, address } = results[i] || {}
               const tokenAlreadyAdded = checkExistingAddresses(address, tokens)
+              const onClick = () =>
+                !tokenAlreadyAdded && onToggleToken(results[i])
 
               return (
                 Boolean(logo || symbol || name) && (
@@ -45,10 +47,10 @@ export default class InfoBox extends Component {
                       'token-list__token--selected': selectedTokens[address],
                       'token-list__token--disabled': tokenAlreadyAdded,
                     })}
-                    onClick={() =>
-                      !tokenAlreadyAdded && onToggleToken(results[i])
-                    }
+                    onClick={onClick}
+                    onKeyPress={(event) => event.key === 'Enter' && onClick()}
                     key={i}
+                    tabIndex="0"
                   >
                     <div
                       className="token-list__token-icon"
diff --git a/ui/app/pages/home/index.scss b/ui/app/pages/home/index.scss
index 2934c920d..2841155dd 100644
--- a/ui/app/pages/home/index.scss
+++ b/ui/app/pages/home/index.scss
@@ -31,7 +31,6 @@
 
     flex-grow: 1;
     color: $Grey-500;
-    padding: 16px 8px;
     font-weight: 500;
   }
 
@@ -39,6 +38,10 @@
     color: $Blue-500;
   }
 
+  &__main-view &__tab button {
+    padding: 16px 8px;
+  }
+
   &__connect-status-text {
     display: flex;
     flex-direction: column;