From 68343a7a0999f63f048df971a2200643e37d2a1f Mon Sep 17 00:00:00 2001
From: Nolan Lawson <nolan@nolanlawson.com>
Date: Mon, 8 Jul 2019 09:42:45 -0700
Subject: [PATCH] fix: use pure CSS for focus effect (#1323)

* fix: use pure CSS for focus effect

fixes #1322

* apply elsewhere too

* fix comment
---
 .../dialog/components/GenericDialogList.html  | 10 +--------
 src/routes/_components/status/Media.html      | 20 ++---------------
 .../_components/status/Notification.html      |  9 +-------
 src/routes/_components/status/Status.html     | 10 +--------
 src/routes/_utils/events.js                   | 22 -------------------
 src/scss/global.scss                          | 21 ++++++++++++++++++
 6 files changed, 26 insertions(+), 66 deletions(-)

diff --git a/src/routes/_components/dialog/components/GenericDialogList.html b/src/routes/_components/dialog/components/GenericDialogList.html
index 768aa757..8554c965 100644
--- a/src/routes/_components/dialog/components/GenericDialogList.html
+++ b/src/routes/_components/dialog/components/GenericDialogList.html
@@ -2,8 +2,7 @@
   {#each items as item (item.key)}
   <li class="generic-dialog-list-item">
     <button
-        class="generic-dialog-list-button"
-        on:applyFocusStylesToParent="noop()"
+        class="generic-dialog-list-button focus-after"
         on:click="fire('click', item)">
       <SvgIcon className="generic-dialog-list-item-svg" href={item.icon} />
       <span class="generic-dialog-list-button-span">
@@ -45,9 +44,6 @@
     display: flex;
     flex-direction: row;
   }
-  .generic-dialog-list-button:focus {
-    outline: none; /* focus style is on parent instead */
-  }
   .generic-dialog-list-button-span {
     flex: 1;
     margin-left: 20px;
@@ -93,13 +89,9 @@
 </style>
 <script>
   import SvgIcon from '../../SvgIcon.html'
-  import { applyFocusStylesToParent } from '../../../_utils/events'
   import noop from 'lodash-es/noop'
 
   export default {
-    events: {
-      applyFocusStylesToParent
-    },
     methods: {
       noop
     },
diff --git a/src/routes/_components/status/Media.html b/src/routes/_components/status/Media.html
index 395d44bb..f01ba78b 100644
--- a/src/routes/_components/status/Media.html
+++ b/src/routes/_components/status/Media.html
@@ -1,7 +1,7 @@
 {#if type === 'video' || type === 'audio'}
   <button id={elementId}
           type="button"
-          class="play-video-button {$largeInlineMedia ? '' : 'fixed-size'} {type === 'audio' ? 'play-audio-button' : ''}"
+          class="play-video-button focus-after {$largeInlineMedia ? '' : 'fixed-size'} {type === 'audio' ? 'play-audio-button' : ''}"
           aria-label="Play video: {description}"
           style="width: {inlineWidth}px; height: {inlineHeight}px;">
     <PlayVideoIcon />
@@ -21,7 +21,7 @@
 {:else}
   <button id={elementId}
           type="button"
-          class="show-image-button {$largeInlineMedia ? '' : 'fixed-size'}"
+          class="show-image-button focus-after {$largeInlineMedia ? '' : 'fixed-size'}"
           aria-label="Show image: {description}"
           title={description}
           on:mouseover="set({mouseover: event})"
@@ -72,27 +72,11 @@
     border-radius: 0;
     border: none;
     background: none;
-    position: relative;
   }
   .play-audio-button {
     background: var(--audio-bg);
   }
 
-  /* the actual focus outline is not very visible, so use an ::after pseudo-element */
-  .play-video-button:focus, .show-image-button:focus {
-    outline: none;
-  }
-
-  .play-video-button:focus::after, .show-image-button:focus::after {
-    content: '';
-    position: absolute;
-    left: 0;
-    right: 0;
-    bottom: 0;
-    top: 0;
-    border: 2px solid var(--focus-outline);
-  }
-
   .show-image-button {
     cursor: zoom-in;
   }
diff --git a/src/routes/_components/status/Notification.html b/src/routes/_components/status/Notification.html
index 855d287b..12d801f2 100644
--- a/src/routes/_components/status/Notification.html
+++ b/src/routes/_components/status/Notification.html
@@ -9,7 +9,6 @@
            aria-posinset={index + 1}
            aria-setsize={length}
            aria-label={ariaLabel}
-           on:applyFocusStylesToParent="noop()"
   >
     <StatusHeader {notification} {notificationId} {status} {statusId} {timelineType}
                   {account} {accountId} {uuid} isStatusInNotification="true" />
@@ -26,9 +25,6 @@
     padding: 10px 20px;
     border-bottom: 1px solid var(--main-border);
   }
-  .notification-article:focus {
-    outline: none; /* focus is on the parent instead */
-  }
   @media (max-width: 767px) {
     .notification-article {
       padding: 10px 10px;
@@ -46,7 +42,6 @@
   import { goto } from '../../../../__sapper__/client'
   import { composeNewStatusMentioning } from '../../_actions/mention'
   import { classname } from '../../_utils/classname'
-  import { applyFocusStylesToParent } from '../../_utils/events'
   import noop from 'lodash-es/noop'
   import { createStatusOrNotificationUuid } from '../../_utils/createStatusOrNotificationUuid'
 
@@ -77,6 +72,7 @@
       className: ({ $underlineLinks }) => (classname(
         'notification-article',
         'shortcut-list-item',
+        'focus-after',
         $underlineLinks && 'underline-links'
       ))
     },
@@ -90,9 +86,6 @@
         let { account } = this.get()
         await composeNewStatusMentioning(account)
       }
-    },
-    events: {
-      applyFocusStylesToParent
     }
   }
 </script>
diff --git a/src/routes/_components/status/Status.html b/src/routes/_components/status/Status.html
index 3e992708..a1ed5519 100644
--- a/src/routes/_components/status/Status.html
+++ b/src/routes/_components/status/Status.html
@@ -5,7 +5,6 @@
          aria-setsize={length}
          aria-label={ariaLabel}
          on:recalculateHeight
-         on:applyFocusStylesToParent="noop()"
 >
   {#if showHeader}
     <StatusHeader {...params} />
@@ -81,10 +80,6 @@
     background-color: var(--status-direct-background);
   }
 
-  .status-article:focus {
-    outline: none; /* focus is on the parent instead */
-  }
-
   .status-article.status-in-own-thread {
     grid-template-areas:
       "sidebar     author-name"
@@ -139,7 +134,6 @@
   import { LONG_POST_LENGTH, LONG_POST_TEXT } from '../../_static/statuses'
   import { absoluteDateFormatter } from '../../_utils/formatters'
   import { composeNewStatusMentioning } from '../../_actions/mention'
-  import { applyFocusStylesToParent } from '../../_utils/events'
   import noop from 'lodash-es/noop'
   import { createStatusOrNotificationUuid } from '../../_utils/createStatusOrNotificationUuid'
   import { statusHtmlToPlainText } from '../../_utils/statusHtmlToPlainText'
@@ -300,6 +294,7 @@
       className: ({ visibility, timelineType, isStatusInOwnThread, $underlineLinks, $disableTapOnStatus }) => (classname(
         'status-article',
         'shortcut-list-item',
+        'focus-after',
         timelineType !== 'direct' && visibility === 'direct' && 'status-direct',
         timelineType !== 'search' && 'status-in-timeline',
         isStatusInOwnThread && 'status-in-own-thread',
@@ -346,9 +341,6 @@
         shortcutScope,
         originalStatusEmojis
       })
-    },
-    events: {
-      applyFocusStylesToParent
     }
   }
 </script>
diff --git a/src/routes/_utils/events.js b/src/routes/_utils/events.js
index 05c3d8c9..b8c0a199 100644
--- a/src/routes/_utils/events.js
+++ b/src/routes/_utils/events.js
@@ -34,28 +34,6 @@ export function selectionChange (node, callback) {
   }
 }
 
-// in some cases we apply focus styles to parent rather
-// than the node itself because it shows up better
-export function applyFocusStylesToParent (node) {
-  function onFocus () {
-    node.parentElement.classList.toggle('focus', true)
-  }
-
-  function onBlur () {
-    node.parentElement.classList.toggle('focus', false)
-  }
-
-  node.addEventListener('focus', onFocus)
-  node.addEventListener('blur', onBlur)
-
-  return {
-    destroy () {
-      node.removeEventListener('focus', onFocus)
-      node.removeEventListener('blur', onBlur)
-    }
-  }
-}
-
 export function resize (node, callback) {
   registerResizeListener(callback)
 
diff --git a/src/scss/global.scss b/src/scss/global.scss
index 7af0ff9e..e5434b6a 100644
--- a/src/scss/global.scss
+++ b/src/scss/global.scss
@@ -251,3 +251,24 @@ textarea {
 .inline-emoji {
   font-family: PinaforeEmoji, sans-serif;
 }
+
+/* Special class to make focus outlines easier to see in some odd
+ * cases where the outline would be clipped. */
+.focus-after {
+  position: relative;
+}
+
+.focus-after:focus {
+  outline: none;
+}
+
+.focus-after:focus::after {
+  position: absolute;
+  left: 0;
+  right: 0;
+  bottom: 0;
+  top: 0;
+  content: '';
+  border: 2px solid var(--focus-outline);
+  pointer-events: none;
+}