From 1bbe2a56125a0365fab89c83548fff3a94fd7671 Mon Sep 17 00:00:00 2001
From: Nolan Lawson <nolan@nolanlawson.com>
Date: Tue, 24 Sep 2019 00:50:35 -0700
Subject: [PATCH] fix: improve focus styles (#1526)

* fix: improve focus styles

* fixup
---
 src/routes/_components/NavItem.html           |  3 +--
 src/routes/_components/TabSet.html            |  1 +
 .../dialog/components/GenericDialogList.html  |  2 +-
 .../dialog/components/ModalDialog.html        |  2 +-
 .../settings/SettingsListButton.html          |  2 +-
 src/routes/_components/status/Media.html      |  4 ++++
 .../_components/status/Notification.html      |  2 +-
 src/routes/_components/status/Status.html     |  2 +-
 src/routes/_components/status/StatusPoll.html |  1 +
 src/scss/focus.scss                           | 23 +++++++++++++------
 10 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/src/routes/_components/NavItem.html b/src/routes/_components/NavItem.html
index b0242295..50d3ac58 100644
--- a/src/routes/_components/NavItem.html
+++ b/src/routes/_components/NavItem.html
@@ -1,4 +1,4 @@
-<a class='main-nav-link {selected ? "selected" : ""}'
+<a class='main-nav-link focus-fix {selected ? "selected" : ""}'
    aria-label={ariaLabel}
    aria-current={selected}
    on:click="onClick(event)"
@@ -22,7 +22,6 @@
     align-items: center;
     flex: 1;
     flex-direction: column;
-    outline-offset: calc(-1 * var(--focus-width)); /* TODO: this is hacky, switch to box-sizing:border-box */
   }
 
   .nav-icon-and-label {
diff --git a/src/routes/_components/TabSet.html b/src/routes/_components/TabSet.html
index a475b13b..10d29a74 100644
--- a/src/routes/_components/TabSet.html
+++ b/src/routes/_components/TabSet.html
@@ -3,6 +3,7 @@
     {#each tabs as tab (tab.name)}
     <li class="{currentTabName === tab.name ? 'current' : 'not-current'}">
       <a aria-label="{tab.label} { currentTabName === tab.name ? '(Current)' : ''}"
+         class="focus-fix"
          href={tab.href}
          rel="prefetch">
         {tab.label}
diff --git a/src/routes/_components/dialog/components/GenericDialogList.html b/src/routes/_components/dialog/components/GenericDialogList.html
index 253ede4c..0c163138 100644
--- a/src/routes/_components/dialog/components/GenericDialogList.html
+++ b/src/routes/_components/dialog/components/GenericDialogList.html
@@ -2,7 +2,7 @@
   {#each items as item (item.key)}
   <li class="generic-dialog-list-item">
     <button
-        class="generic-dialog-list-button focus-after"
+        class="generic-dialog-list-button focus-fix"
         on:click="fire('click', item)">
       <!-- Extra wrapper inside button is required for KaiOS. Seems old Firefox does not like flex buttons. -->
       <div class="generic-dialog-list-button-inner">
diff --git a/src/routes/_components/dialog/components/ModalDialog.html b/src/routes/_components/dialog/components/ModalDialog.html
index fe432651..1068b987 100644
--- a/src/routes/_components/dialog/components/ModalDialog.html
+++ b/src/routes/_components/dialog/components/ModalDialog.html
@@ -13,7 +13,7 @@
         <h1 class="modal-dialog-title">{title}</h1>
       {/if}
       <div class="close-dialog-button-wrapper">
-        <button class="close-dialog-button focus-after"
+        <button class="close-dialog-button focus-fix"
                 data-a11y-dialog-hide aria-label="Close dialog">
           <SvgIcon className="close-dialog-button-svg" href="#fa-times" />
         </button>
diff --git a/src/routes/_components/settings/SettingsListButton.html b/src/routes/_components/settings/SettingsListButton.html
index a23ccca1..73092716 100644
--- a/src/routes/_components/settings/SettingsListButton.html
+++ b/src/routes/_components/settings/SettingsListButton.html
@@ -2,7 +2,7 @@
    on:click="fire('click', event)"
    rel="prefetch"
    aria-label={ariaLabel || label}
-   class="settings-list-button focus-after {className ? className : ''}"
+   class="settings-list-button focus-fix {className ? className : ''}"
 >
   <span>
     {label}
diff --git a/src/routes/_components/status/Media.html b/src/routes/_components/status/Media.html
index 22ce7ea6..6902cc30 100644
--- a/src/routes/_components/status/Media.html
+++ b/src/routes/_components/status/Media.html
@@ -85,6 +85,10 @@
     cursor: zoom-in;
   }
 
+  .inline-media {
+    position: relative;
+  }
+
   @media (max-width: 240px) {
     .inline-media {
       min-height: 100px; /* TODO: hack for KaiOS, which renders the grouped-images style as 0 height */
diff --git a/src/routes/_components/status/Notification.html b/src/routes/_components/status/Notification.html
index 0df641c8..7baf038e 100644
--- a/src/routes/_components/status/Notification.html
+++ b/src/routes/_components/status/Notification.html
@@ -70,7 +70,7 @@
       className: ({ $underlineLinks }) => (classname(
         'notification-article',
         'shortcut-list-item',
-        'focus-after',
+        'focus-fix',
         $underlineLinks && 'underline-links'
       ))
     },
diff --git a/src/routes/_components/status/Status.html b/src/routes/_components/status/Status.html
index 32bb78f1..bc309a43 100644
--- a/src/routes/_components/status/Status.html
+++ b/src/routes/_components/status/Status.html
@@ -294,7 +294,7 @@
       className: ({ visibility, timelineType, isStatusInOwnThread, $underlineLinks, $disableTapOnStatus }) => (classname(
         'status-article',
         'shortcut-list-item',
-        'focus-after',
+        'focus-fix',
         timelineType !== 'direct' && visibility === 'direct' && 'status-direct',
         timelineType !== 'search' && 'status-in-timeline',
         isStatusInOwnThread && 'status-in-own-thread',
diff --git a/src/routes/_components/status/StatusPoll.html b/src/routes/_components/status/StatusPoll.html
index 5451e3c3..1bfa0d83 100644
--- a/src/routes/_components/status/StatusPoll.html
+++ b/src/routes/_components/status/StatusPoll.html
@@ -47,6 +47,7 @@
     </li>
     <li class="poll-stat {notification ? 'is-notification' : ''} {expired ? 'poll-expired' : ''}">
       <button id={refreshElementId}
+              class="focus-fix"
               aria-label="Refresh">
         <SvgIcon className="poll-icon" href="#fa-refresh" />
         <span class="poll-stat-text poll-stat-text-refresh" aria-hidden="true">
diff --git a/src/scss/focus.scss b/src/scss/focus.scss
index 5a0b2081..b32e569d 100644
--- a/src/scss/focus.scss
+++ b/src/scss/focus.scss
@@ -1,6 +1,6 @@
 
 
-*:focus, .focus {
+*:focus {
   outline: var(--focus-width) solid var(--focus-outline);
 }
 
@@ -13,6 +13,11 @@
 /* Special class to make focus outlines easier to see in some odd
  * cases where the outline would be clipped. */
 /* TODO: use box-sizing:border-box everywhere so we can get rid of this hack */
+.focus-fix:focus {
+  outline-offset: calc(-1 * var(--focus-width)); /* TODO: this is hacky, switch to box-sizing:border-box */
+}
+
+/* Another hack to make some focus styles appear better */
 .focus-after {
   position: relative;
 }
@@ -35,24 +40,28 @@
 // For KaiOS, do some additional things to improve the focus styles, which don't show up well
 // for some reason
 @media (max-width: 240px) {
-  a:focus, span:focus {
+  a:not(.button):focus, span:focus {
     background: var(--focus-bg) !important;
   }
 
-  button:focus {
+  button:focus, .button:focus {
     opacity: 0.7;
   }
 
-  // add extra "focus-after"-like styles for buttons to increase visibility
-  button.icon-button {
+  button.primary:focus, .button.primary:focus {
+    opacity: 0.8;
+  }
+
+  button, .button {
     position: relative;
   }
 
-  button.icon-button:focus {
+  // add extremely visible styles for buttons, ala .focus-after
+  button:focus, .button:focus {
     outline: none;
   }
 
-  button.icon-button:focus::after {
+  button:focus::after, .button:focus::after {
     position: absolute;
     left: 0;
     right: 0;