From f2f59552641b6ef7c253d159ab01e7be73e13b19 Mon Sep 17 00:00:00 2001
From: Benedict Aas <x+g@shou.io>
Date: Fri, 27 Oct 2017 19:41:47 +0100
Subject: [PATCH] feat(GUI): use tabindex and focus to navigate (#1745)

* feat(GUI): use tabindex and focus to navigate

We make navigating with the tab key easier by highlighting focused
elements more visibly, adding `tabindex` attributes to elements, and
making `open-external` links respond to keyboard events.

Change-Type: minor
Changelog-Entry: Improve tab-key navigation through tabindex and visual improvements.
Connects-To: https://github.com/resin-io/etcher/issues/1734

* outline with 10s timeout

* use orange "warning colour" as outline

* smaller outline on settings buttons, fix order on settings page

* allow selection in drive-selector

* fix typo, better tabindexes
---
 .../controllers/drive-selector.js             | 25 ++++++++++++++
 .../templates/drive-selector-modal.tpl.html   | 15 +++++---
 .../templates/warning-modal.tpl.html          |  6 +++-
 lib/gui/css/desktop.css                       |  6 ----
 lib/gui/css/main.css                          | 34 +++++++++++--------
 lib/gui/index.html                            | 31 ++++++++++++-----
 .../open-external/directives/open-external.js | 13 +++++++
 lib/gui/pages/main/templates/main.tpl.html    |  5 +++
 .../settings/templates/settings.tpl.html      |  5 +++
 lib/gui/scss/components/_button.scss          | 11 ++----
 lib/gui/scss/main.scss                        | 16 +++++++++
 11 files changed, 123 insertions(+), 44 deletions(-)

diff --git a/lib/gui/components/drive-selector/controllers/drive-selector.js b/lib/gui/components/drive-selector/controllers/drive-selector.js
index 09cd5e54..2cc61249 100644
--- a/lib/gui/components/drive-selector/controllers/drive-selector.js
+++ b/lib/gui/components/drive-selector/controllers/drive-selector.js
@@ -251,4 +251,29 @@ module.exports = function (
   this.getDriveStatuses = this.memoizeImmutableListReference((drive) => {
     return this.constraints.getDriveImageCompatibilityStatuses(drive, this.state.getImage())
   })
+
+  /**
+   * @summary Keyboard event drive toggling
+   * @function
+   * @public
+   *
+   * @description
+   * Keyboard-event specific entry to the toggleDrive function.
+   *
+   * @param {Object} drive - drive
+   * @param {Object} $event - event
+   *
+   * @example
+   * <div tabindex="1" ng-keypress="this.keyboardToggleDrive(drive, $event)">
+   *   Tab-select me and press enter or space!
+   * </div>
+   */
+  this.keyboardToggleDrive = (drive, $event) => {
+    console.log($event.keyCode)
+    const ENTER = 13
+    const SPACE = 32
+    if (_.includes([ ENTER, SPACE ], $event.keyCode)) {
+      this.toggleDrive(drive)
+    }
+  }
 }
diff --git a/lib/gui/components/drive-selector/templates/drive-selector-modal.tpl.html b/lib/gui/components/drive-selector/templates/drive-selector-modal.tpl.html
index ce8a1a07..7507df61 100644
--- a/lib/gui/components/drive-selector/templates/drive-selector-modal.tpl.html
+++ b/lib/gui/components/drive-selector/templates/drive-selector-modal.tpl.html
@@ -1,6 +1,6 @@
 <div class="modal-header">
   <h4 class="modal-title">Select a Drive</h4>
-  <button class="close" ng-click="modal.closeModal()">&times;</button>
+  <button tabindex="14" class="close" ng-click="modal.closeModal()">&times;</button>
 </div>
 
 <div class="modal-body">
@@ -14,10 +14,14 @@
           ng-src="./assets/{{drive.icon}}.svg"
           width="25"
           height="30">
-        <div class="list-group-item-section list-group-item-section-expanded">
-          <h4 class="list-group-item-heading">
-            {{ drive.description }} <span class="word-keep"
-              ng-show="drive.size">- {{ drive.size | closestUnit }}</span>
+        <div
+          class="list-group-item-section list-group-item-section-expanded"
+          tabindex="{{ 15 + $index }}"
+          ng-keypress="modal.keyboardToggleDrive(drive, $event)">
+
+          <h4 class="list-group-item-heading">{{ drive.description }} -
+            <span class="word-keep"
+              ng-show="drive.size">{{ drive.size | closestUnit }}</span>
           </h4>
           <p class="list-group-item-text">{{ drive.displayName }}</p>
 
@@ -50,6 +54,7 @@
 
 <div class="modal-footer">
   <button class="button button-primary button-block"
+    tabindex="{{ 15 + modal.getDrives().length }}"
     ng-click="modal.closeModal()"
     ng-disabled="!modal.state.hasDrive()">Continue</button>
 </div>
diff --git a/lib/gui/components/warning-modal/templates/warning-modal.tpl.html b/lib/gui/components/warning-modal/templates/warning-modal.tpl.html
index 1d00fdd2..fdbdfef5 100644
--- a/lib/gui/components/warning-modal/templates/warning-modal.tpl.html
+++ b/lib/gui/components/warning-modal/templates/warning-modal.tpl.html
@@ -3,7 +3,9 @@
     <span class="glyphicon glyphicon-exclamation-sign"></span>
     <span>Attention</span>
   </h4>
-  <button class="close" ng-click="modal.reject()">&times;</button>
+  <button class="close"
+    tabindex="11"
+    ng-click="modal.reject()">&times;</button>
 </div>
 
 <div class="modal-body">
@@ -13,8 +15,10 @@
 <div class="modal-footer">
   <div class="modal-menu">
     <button class="button button-danger button-block"
+      tabindex="13"
       ng-click="modal.accept()">{{ ::modal.options.confirmationLabel }}</button>
     <button ng-if="modal.options.rejectionLabel" class="button button-block"
+      tabindex="12"
       ng-click="modal.reject()">{{ ::modal.options.rejectionLabel }}</button>
   </div>
 </div>
diff --git a/lib/gui/css/desktop.css b/lib/gui/css/desktop.css
index 8450f62a..d04d6540 100644
--- a/lib/gui/css/desktop.css
+++ b/lib/gui/css/desktop.css
@@ -47,12 +47,6 @@ body {
   -webkit-overflow-scrolling: touch;
 }
 
-/* Prevent blue outline */
-input:focus,
-button:focus {
-  outline: none !important;
-}
-
 /* Titles don't have margins on desktop apps */
 h1, h2, h3, h4, h5, h6 {
   margin: 0;
diff --git a/lib/gui/css/main.css b/lib/gui/css/main.css
index c26d8fee..9a400032 100644
--- a/lib/gui/css/main.css
+++ b/lib/gui/css/main.css
@@ -6132,33 +6132,30 @@ body {
 .button-no-hover, .button[disabled], [disabled].progress-button, .progress-button[active="true"] {
   pointer-events: none; }
 
-.button-default,
-.button-default:focus {
+.button-default {
   background-color: #ececec;
-  color: #b3b3b3;
-  outline: none; }
+  color: #b3b3b3; }
 
+.button-default:focus,
 .button-default:hover {
   background-color: lightgray;
   color: #b3b3b3; }
 
-.button-primary, .progress-button,
-.button-primary:focus,
-.progress-button:focus {
+.button-primary, .progress-button {
   background-color: #5793db;
-  color: #fff;
-  outline: none; }
+  color: #fff; }
 
-.button-primary:hover, .progress-button:hover {
+.button-primary:focus, .progress-button:focus,
+.button-primary:hover,
+.progress-button:hover {
   background-color: #2d78d2;
   color: #fff; }
 
-.button-danger,
-.button-danger:focus {
+.button-danger {
   background-color: #d9534f;
-  color: #fff;
-  outline: none; }
+  color: #fff; }
 
+.button-danger:focus,
 .button-danger:hover {
   background-color: #c9302c;
   color: #fff; }
@@ -6741,3 +6738,12 @@ body {
   .section-header > .button, .section-header > .progress-button {
     padding-left: 3px;
     padding-right: 3px; }
+
+@keyframes focus-highlight {
+  from {
+    outline: 2px solid #ff912f; }
+  to {
+    outline: none; } }
+
+[tabindex]:focus {
+  animation: focus-highlight 10s steps(2, end) forwards; }
diff --git a/lib/gui/index.html b/lib/gui/index.html
index 1c80db98..7dd88d0a 100644
--- a/lib/gui/index.html
+++ b/lib/gui/index.html
@@ -20,16 +20,24 @@
   </head>
   <body ng-app="Etcher">
     <header class="section-header" ng-controller="HeaderController as header">
-      <button class="button button-link" ng-click="header.openHelpPage()">
-        <span class="glyphicon glyphicon-question-sign"></span>
+      <button class="button button-link"
+        ng-click="header.openHelpPage()"
+        tabindex="-1">
+        <span tabindex="4" class="glyphicon glyphicon-question-sign"></span>
       </button>
 
-      <button class="button button-link" ui-sref="settings" hide-if-state="settings">
-        <span class="glyphicon glyphicon-cog"></span>
+      <button class="button button-link"
+        ui-sref="settings"
+        hide-if-state="settings"
+        tabindex="-1">
+        <span tabindex="5" class="glyphicon glyphicon-cog"></span>
       </button>
 
-      <button class="button button-link" ui-sref="main" show-if-state="settings">
-        <span class="glyphicon glyphicon-chevron-left"></span> Back
+      <button class="button button-link"
+        tabindex="-1"
+        ui-sref="main"
+        show-if-state="settings">
+        <span tabindex="5" class="glyphicon glyphicon-chevron-left"></span> Back
       </button>
     </header>
 
@@ -37,23 +45,28 @@
 
     <footer class="section-footer" ng-controller="StateController as state"
       ng-hide="state.currentName === 'success'">
-      <span os-open-external="https://etcher.io?ref=etcher_footer">
+      <span os-open-external="https://etcher.io?ref=etcher_footer"
+        tabindex="100">
         <svg-icon path="'../assets/etcher.svg'"
           width="'83px'"
           height="'13px'"></svg-icon>
       </span>
 
       <span class="caption">
-        is <span class="caption" os-open-external="https://github.com/resin-io/etcher">an open source project</span> by
+        is <span class="caption"
+          tabindex="101"
+          os-open-external="https://github.com/resin-io/etcher">an open source project</span> by
       </span>
 
-      <span os-open-external="https://resin.io?ref=etcher">
+      <span os-open-external="https://resin.io?ref=etcher"
+        tabindex="102">
         <svg-icon path="'../assets/resin.svg'"
           width="'79px'"
           height="'23px'"></svg-icon>
       </span>
 
       <span class="caption footer-right"
+        tabindex="103"
         manifest-bind="version"
         os-open-external="https://github.com/resin-io/etcher/blob/master/CHANGELOG.md"></span>
     </footer>
diff --git a/lib/gui/os/open-external/directives/open-external.js b/lib/gui/os/open-external/directives/open-external.js
index c56329f9..7149be98 100644
--- a/lib/gui/os/open-external/directives/open-external.js
+++ b/lib/gui/os/open-external/directives/open-external.js
@@ -16,6 +16,8 @@
 
 'use strict'
 
+const _ = require('lodash')
+
 /**
  * @summary OsOpenExternal directive
  * @function
@@ -43,6 +45,17 @@ module.exports = (OSOpenExternalService) => {
       element.on('click', () => {
         OSOpenExternalService.open(attributes.osOpenExternal)
       })
+
+      const ENTER_KEY = 13
+      const SPACE_KEY = 32
+      element.on('keypress', (event) => {
+        if (_.includes([ ENTER_KEY, SPACE_KEY ], event.keyCode)) {
+          // Don't spam the user with several tabs if the key is being held
+          if (!event.repeat) {
+            OSOpenExternalService.open(attributes.osOpenExternal)
+          }
+        }
+      })
     }
   }
 }
diff --git a/lib/gui/pages/main/templates/main.tpl.html b/lib/gui/pages/main/templates/main.tpl.html
index 24908deb..71b2cce9 100644
--- a/lib/gui/pages/main/templates/main.tpl.html
+++ b/lib/gui/pages/main/templates/main.tpl.html
@@ -10,6 +10,7 @@
         <div ng-hide="main.selection.hasImage()">
           <button
             class="button button-primary button-brick"
+            tabindex="{{ main.selection.hasImage() ? -1 : 1 }}"
             ng-click="image.openImageSelector()">Select image</button>
 
           <p class="step-footer">
@@ -29,6 +30,7 @@
           </div>
 
           <button class="button button-link step-footer"
+            tabindex="1"
             ng-click="image.reselectImage()"
             ng-hide="main.state.isFlashing()">Change</button>
         </div>
@@ -51,6 +53,7 @@
 
           <div>
             <button class="button button-primary button-brick"
+              tabindex="{{ main.selection.hasDrive() ? -1 : 2 }}"
               ng-disabled="main.shouldDriveStepBeDisabled()"
               ng-click="drive.openDriveSelector()">Select drive</button>
           </div>
@@ -70,6 +73,7 @@
             <span class="step-drive step-size">{{ main.selection.getDrive().size | closestUnit }}</span>
           </div>
           <button class="button button-link step-footer"
+            tabindex="{{ main.selection.hasDrive() ? 2 : -1 }}"
             ng-click="drive.reselectDrive()"
             ng-hide="main.state.isFlashing()">Change</button>
         </div>
@@ -86,6 +90,7 @@
 
       <div class="space-vertical-large">
         <progress-button class="button-brick"
+          tabindex="3"
           percentage="main.state.getFlashState().percentage"
           striped="{{ main.state.getFlashState().type == 'check' }}"
           ng-attr-active="{{ main.state.isFlashing() }}"
diff --git a/lib/gui/pages/settings/templates/settings.tpl.html b/lib/gui/pages/settings/templates/settings.tpl.html
index 153938c4..b6b095c8 100644
--- a/lib/gui/pages/settings/templates/settings.tpl.html
+++ b/lib/gui/pages/settings/templates/settings.tpl.html
@@ -4,6 +4,7 @@
   <div class="checkbox">
     <label>
       <input type="checkbox"
+        tabindex="6"
         ng-model="settings.currentData.errorReporting"
         ng-change="settings.toggle('errorReporting')">
 
@@ -14,6 +15,7 @@
   <div class="checkbox">
     <label>
       <input type="checkbox"
+        tabindex="7"
         ng-model="settings.currentData.unmountOnSuccess"
         ng-change="settings.toggle('unmountOnSuccess')">
 
@@ -33,6 +35,7 @@
   <div class="checkbox">
     <label>
       <input type="checkbox"
+        tabindex="8"
         ng-model="settings.currentData.validateWriteOnSuccess"
         ng-change="settings.toggle('validateWriteOnSuccess')">
 
@@ -43,6 +46,7 @@
   <div class="checkbox" ng-show="settings.model.get('updatesEnabled')">
     <label>
       <input type="checkbox"
+        tabindex="9"
         ng-model="settings.currentData.includeUnstableUpdateChannel"
         ng-change="settings.toggle('includeUnstableUpdateChannel')">
 
@@ -53,6 +57,7 @@
   <div class="checkbox">
     <label>
       <input type="checkbox"
+        tabindex="10"
         ng-model="settings.currentData.unsafeMode"
         ng-change="settings.toggle('unsafeMode', {
           description: 'Are you sure you want to turn this on? You will be able to overwrite your system drives if you\'re not careful.',
diff --git a/lib/gui/scss/components/_button.scss b/lib/gui/scss/components/_button.scss
index 2b317749..aba5856c 100644
--- a/lib/gui/scss/components/_button.scss
+++ b/lib/gui/scss/components/_button.scss
@@ -75,19 +75,12 @@ $button-types-styles: (
 @each $style in map-keys($button-types-styles) {
   $button-styles: map-get($button-types-styles, $style);
 
-  .button-#{$style},
-
-  // Undo `:focus` styles from Bootstrap.
-  // On Electron, the user can click and press over a button,
-  // then move the mouse away from the button and release,
-  // and the button will erroneusly keep the `:focus` state style.
-  .button-#{$style}:focus {
-
+  .button-#{$style} {
     background-color: map-get($button-styles, "bg");
     color: map-get($button-styles, "color");
-    outline: none;
   }
 
+  .button-#{$style}:focus,
   .button-#{$style}:hover {
     background-color: darken(map-get($button-styles, "bg"), 10%);
     color: map-get($button-styles, "color");
diff --git a/lib/gui/scss/main.scss b/lib/gui/scss/main.scss
index 983ddafd..a11ca4ca 100644
--- a/lib/gui/scss/main.scss
+++ b/lib/gui/scss/main.scss
@@ -123,3 +123,19 @@ body {
     padding-right: 3px;
   }
 }
+
+@keyframes focus-highlight {
+  from {
+    outline: 2px solid $palette-theme-warning-background;
+  }
+
+  to {
+    outline: none;
+  }
+}
+
+[tabindex] {
+  &:focus {
+    animation: focus-highlight 10s steps(2, end) forwards;
+  }
+}