From 0c9992c0e16700b466e9977164519b6461f526ff Mon Sep 17 00:00:00 2001
From: Nolan Lawson <nolan@nolanlawson.com>
Date: Tue, 20 Mar 2018 20:28:53 -0700
Subject: [PATCH] change scheduling to focus on requestIdleCallback

---
 routes/_components/status/Notification.html   | 11 +++++--
 routes/_components/status/Status.html         |  8 ++++-
 .../timeline/NotificationVirtualListItem.html |  1 +
 .../timeline/StatusVirtualListItem.html       |  1 +
 routes/_components/timeline/Timeline.html     | 33 +++++++------------
 .../_components/virtualList/VirtualList.html  |  2 +-
 .../virtualList/VirtualListContainer.html     | 21 +++++-------
 .../virtualList/VirtualListLazyItem.html      |  3 +-
 .../virtualList/virtualListStore.js           |  2 +-
 routes/_utils/restoreFocus.js                 |  9 +++++
 tests/spec/010-focus.js                       | 24 ++++++++++++++
 11 files changed, 76 insertions(+), 39 deletions(-)
 create mode 100644 routes/_utils/restoreFocus.js

diff --git a/routes/_components/status/Notification.html b/routes/_components/status/Notification.html
index 64e57b98..10b8bf1b 100644
--- a/routes/_components/status/Notification.html
+++ b/routes/_components/status/Notification.html
@@ -1,5 +1,5 @@
 {{#if notification.type === 'mention' || notification.type === 'reblog' || notification.type === 'favourite'}}
-  <Status :index :length :timelineType :timelineValue
+  <Status :index :length :timelineType :timelineValue :focusSelector
           status="{{notification.status}}"
           :notification
           on:recalculateHeight
@@ -8,7 +8,7 @@
   <article class="notification-article"
            tabindex="0"
            aria-posinset="{{index}}" aria-setsize="{{length}}"
-  >
+           ref:node >
     <div class="follow-notification-offset">
       <StatusHeader :notification :notificationId :status :statusId :timelineType
                     :account :accountId :uuid isStatusInNotification="true" />
@@ -37,8 +37,15 @@
   import Status from './Status.html'
   import StatusHeader from './StatusHeader.html'
   import { store } from '../../_store/store'
+  import { restoreFocus } from '../../_utils/restoreFocus'
 
   export default {
+    oncreate() {
+      let focusSelector = this.get('focusSelector')
+      if (this.refs.node && focusSelector) {
+        restoreFocus(this.refs.node, focusSelector)
+      }
+    },
     components: {
       Status,
       StatusHeader
diff --git a/routes/_components/status/Status.html b/routes/_components/status/Status.html
index 135b892e..0d1f2527 100644
--- a/routes/_components/status/Status.html
+++ b/routes/_components/status/Status.html
@@ -5,7 +5,8 @@
          aria-posinset="{{index}}"
          aria-setsize="{{length}}"
          aria-label="{{ariaLabel}}"
-         on:recalculateHeight>
+         on:recalculateHeight
+         ref:node >
   {{#if showHeader}}
     <StatusHeader :notification :notificationId :status :statusId :timelineType
                   :account :accountId :uuid :isStatusInNotification />
@@ -102,6 +103,7 @@
   import { goto } from 'sapper/runtime.js'
   import { registerClickDelegate, unregisterClickDelegate } from '../../_utils/delegate'
   import { classname } from '../../_utils/classname'
+  import { restoreFocus } from '../../_utils/restoreFocus'
 
   export default {
     oncreate() {
@@ -110,6 +112,10 @@
         // the whole <article> is clickable in this case
         registerClickDelegate(delegateKey, (e) => this.onClickOrKeydown(e))
       }
+      let focusSelector = this.get('focusSelector')
+      if (this.refs.node && focusSelector) {
+        restoreFocus(this.refs.node, focusSelector)
+      }
     },
     ondestroy() {
       let delegateKey = this.get('delegateKey')
diff --git a/routes/_components/timeline/NotificationVirtualListItem.html b/routes/_components/timeline/NotificationVirtualListItem.html
index 05493614..04bd5b8b 100644
--- a/routes/_components/timeline/NotificationVirtualListItem.html
+++ b/routes/_components/timeline/NotificationVirtualListItem.html
@@ -2,6 +2,7 @@
   notification="{{virtualProps.notification}}"
   timelineType="{{virtualProps.timelineType}}"
   timelineValue="{{virtualProps.timelineValue}}"
+  focusSelector="{{virtualProps.focusSelector}}"
   index="{{virtualIndex}}"
   length="{{virtualLength}}"
   on:recalculateHeight />
diff --git a/routes/_components/timeline/StatusVirtualListItem.html b/routes/_components/timeline/StatusVirtualListItem.html
index 6b9635ec..64a14a52 100644
--- a/routes/_components/timeline/StatusVirtualListItem.html
+++ b/routes/_components/timeline/StatusVirtualListItem.html
@@ -1,6 +1,7 @@
 <Status status="{{virtualProps.status}}"
         timelineType="{{virtualProps.timelineType}}"
         timelineValue="{{virtualProps.timelineValue}}"
+        focusSelector="{{virtualProps.focusSelector}}"
         index="{{virtualIndex}}"
         length="{{virtualLength}}"
         on:recalculateHeight />
diff --git a/routes/_components/timeline/Timeline.html b/routes/_components/timeline/Timeline.html
index 12535550..c9782d48 100644
--- a/routes/_components/timeline/Timeline.html
+++ b/routes/_components/timeline/Timeline.html
@@ -79,9 +79,6 @@
       console.log('timeline oncreate()')
       this.setupFocus()
       setupTimeline()
-      if (this.store.get('initialized')) {
-        this.restoreFocus()
-      }
       this.setupStreaming()
     },
     ondestroy() {
@@ -101,13 +98,22 @@
       VirtualListComponent: (timelineType) => {
         return timelineType === 'notifications' ? NotificationVirtualListItem : StatusVirtualListItem
       },
-      makeProps: ($currentInstance, timelineType, timelineValue) => async (itemId) => {
-        let res = { timelineType, timelineValue }
+      makeProps: ($currentInstance, timelineType, timelineValue, $lastFocusedElementSelector) => async (itemId) => {
+        let res = {
+          timelineType,
+          timelineValue
+        }
         if (timelineType === 'notifications') {
           res.notification = await database.getNotification($currentInstance, itemId)
         } else {
           res.status = await database.getStatus($currentInstance, itemId)
         }
+        if ($lastFocusedElementSelector && $lastFocusedElementSelector.includes(itemId)) {
+          // this selector is guaranteed to contain the statusId. false positives
+          // (e.g. notification id "1" matches notification id "11") are okay
+          // because Status.html won't be able to find the selector which is fine.
+          res.focusSelector = $lastFocusedElementSelector
+        }
         return res
       },
       label: (timeline, $currentInstance, timelineType, timelineValue) => {
@@ -262,22 +268,7 @@
         this.store.setForTimeline(instanceName, timelineName, {
           lastFocusedElementSelector: null
         })
-      },
-      restoreFocus() {
-        let lastFocusedElementSelector = this.store.get('lastFocusedElementSelector')
-        console.log('lastFocused', lastFocusedElementSelector)
-        if (lastFocusedElementSelector) {
-          requestAnimationFrame(() => {
-            requestAnimationFrame(() => {
-              let element = document.querySelector(lastFocusedElementSelector)
-              console.log('el', element)
-              if (element) {
-                element.focus()
-              }
-            })
-          })
-        }
-      },
+      }
     }
   }
 </script>
\ No newline at end of file
diff --git a/routes/_components/virtualList/VirtualList.html b/routes/_components/virtualList/VirtualList.html
index 8cf0b021..1061b761 100644
--- a/routes/_components/virtualList/VirtualList.html
+++ b/routes/_components/virtualList/VirtualList.html
@@ -32,7 +32,7 @@
   import { mark, stop } from '../../_utils/marks'
   import isEqual from 'lodash/isEqual'
 
-  const DISTANCE_FROM_BOTTOM_TO_FIRE = 400
+  const DISTANCE_FROM_BOTTOM_TO_FIRE = 800
   const SCROLL_EVENT_THROTTLE = 1000
 
   export default {
diff --git a/routes/_components/virtualList/VirtualListContainer.html b/routes/_components/virtualList/VirtualListContainer.html
index a10e95cb..ddeefebf 100644
--- a/routes/_components/virtualList/VirtualListContainer.html
+++ b/routes/_components/virtualList/VirtualListContainer.html
@@ -4,6 +4,7 @@
   import throttle from 'lodash/throttle'
   import { isFullscreen, attachFullscreenListener, detachFullscreenListener } from '../../_utils/fullscreen'
   import { mark, stop } from '../../_utils/marks'
+  import { scheduleIdleTask } from '../../_utils/scheduleIdleTask'
 
   const SCROLL_EVENT_DELAY = 300
 
@@ -20,20 +21,16 @@
           console.log('allVisibleItemsHaveHeight', allVisibleItemsHaveHeight)
           if (!this.get('initializedScrollTop') && allVisibleItemsHaveHeight && node) {
             this.set({'initializedScrollTop': true})
-            requestAnimationFrame(() => {
-              mark('set scrollTop')
-              console.log('forcing scroll top to ', scrollTop)
-              node.scrollTop = scrollTop
-              stop('set scrollTop')
-            })
+            mark('set scrollTop')
+            console.log('forcing scroll top to ', scrollTop)
+            node.scrollTop = scrollTop
+            stop('set scrollTop')
           }
         })
       } else {
-        requestAnimationFrame(() => {
-          this.store.setForRealm({
-            scrollHeight: node.scrollHeight,
-            offsetHeight: node.offsetHeight
-          })
+        this.store.setForRealm({
+          scrollHeight: node.scrollHeight,
+          offsetHeight: node.offsetHeight
         })
       }
       stop('onCreate VirtualListContainer')
@@ -75,7 +72,7 @@
       },
       onScroll(event) {
         let { scrollTop, scrollHeight } = event.target
-        requestAnimationFrame(() => { // delay slightly to improve scroll perf
+        scheduleIdleTask(() => { // delay slightly to improve scroll perf
           mark('onScroll -> setForRealm()')
           this.store.setForRealm({scrollTop, scrollHeight})
           stop('onScroll -> setForRealm()')
diff --git a/routes/_components/virtualList/VirtualListLazyItem.html b/routes/_components/virtualList/VirtualListLazyItem.html
index 873db8bf..828796a1 100644
--- a/routes/_components/virtualList/VirtualListLazyItem.html
+++ b/routes/_components/virtualList/VirtualListLazyItem.html
@@ -9,6 +9,7 @@
 <script>
   import VirtualListItem from './VirtualListItem'
   import { mark, stop } from '../../_utils/marks'
+  import { scheduleIdleTask } from '../../_utils/scheduleIdleTask'
 
   export default {
     async oncreate() {
@@ -18,7 +19,7 @@
       let key = this.get('key')
       if (makeProps) {
         let props = await makeProps(key)
-        requestAnimationFrame(() => { // delay slightly to avoid slow scrolling
+        scheduleIdleTask(() => { // delay slightly to avoid slow scrolling
           mark('VirtualListLazyItem set props')
           this.set({props: props})
           stop('VirtualListLazyItem set props')
diff --git a/routes/_components/virtualList/virtualListStore.js b/routes/_components/virtualList/virtualListStore.js
index 10e2b119..cc44dd2d 100644
--- a/routes/_components/virtualList/virtualListStore.js
+++ b/routes/_components/virtualList/virtualListStore.js
@@ -2,7 +2,7 @@ import { mark, stop } from '../../_utils/marks'
 import { RealmStore } from '../../_utils/RealmStore'
 import { reselect } from '../../_utils/reselect'
 
-const VIEWPORT_RENDER_FACTOR = 1.5
+const VIEWPORT_RENDER_FACTOR = 5
 
 class VirtualListStore extends RealmStore {
   constructor (state) {
diff --git a/routes/_utils/restoreFocus.js b/routes/_utils/restoreFocus.js
new file mode 100644
index 00000000..962031ea
--- /dev/null
+++ b/routes/_utils/restoreFocus.js
@@ -0,0 +1,9 @@
+export function restoreFocus (element, selector) {
+  // Have to check from the parent because otherwise this element itself wouldn't match.
+  // This is fine for <article class=status> elements because they already have a div wrapper.
+  let elementToFocus = element.parentElement.querySelector(selector)
+  console.log('restoreFocus', selector, elementToFocus)
+  if (elementToFocus) {
+    elementToFocus.focus()
+  }
+}
diff --git a/tests/spec/010-focus.js b/tests/spec/010-focus.js
index 3b2e8d00..a1adb1a6 100644
--- a/tests/spec/010-focus.js
+++ b/tests/spec/010-focus.js
@@ -34,6 +34,7 @@ test('timeline link preserves focus', async t => {
     .expect(getUrl()).contains('/accounts/')
     .click(goBackButton)
     .expect(getUrl()).eql('http://localhost:4002/')
+    .expect(getNthStatus(0).exists).ok()
     .expect(getActiveElementInnerText()).eql('admin')
     .click(getNthStatus(0).find('.status-sidebar'))
     .expect(getUrl()).contains('/accounts/')
@@ -51,10 +52,32 @@ test('notification timeline preserves focus', async t => {
     .expect(getUrl()).contains('/accounts/')
     .click(goBackButton)
     .expect(getUrl()).eql('http://localhost:4002/notifications')
+    .expect(getNthStatus(0).exists).ok()
     .expect(getActiveElementInnerText()).eql('quux')
     .expect(getActiveElementInsideNthStatus()).eql('5')
 })
 
+test('thread preserves focus', async t => {
+  await t.useRole(foobarRole)
+    .navigateTo('/accounts/3')
+  await scrollToStatus(t, 2)
+  await t.click(getNthStatus(2))
+    .expect(getUrl()).contains('/statuses/')
+    .click(getNthStatus(24).find('.status-sidebar'))
+    .expect(getUrl()).contains('/accounts/')
+    .click(goBackButton)
+    .expect(getUrl()).contains('/statuses/')
+    .expect(getNthStatus(24).exists).ok()
+    .expect(getActiveElementClass()).contains('status-sidebar')
+    .expect(getActiveElementInsideNthStatus()).eql('24')
+    .click(getNthStatus(23))
+    .expect(getNthStatus(23).find('.status-absolute-date').exists).ok()
+  await goBack()
+  await t.expect(getNthStatus(24).find('.status-absolute-date').exists).ok()
+    .expect(getActiveElementClass()).contains('status-article status-in-timeline')
+    .expect(getActiveElementInsideNthStatus()).eql('23')
+})
+
 test('reply preserves focus and moves focus to the text input', async t => {
   await t.useRole(foobarRole)
     .click(getNthReplyButton(1))
@@ -62,6 +85,7 @@ test('reply preserves focus and moves focus to the text input', async t => {
     .expect(getActiveElementClass()).contains('compose-box-input')
     .click(goBackButton)
     .expect(getUrl()).eql('http://localhost:4002/')
+    .expect(getNthStatus(0).exists).ok()
     .expect(getActiveElementClass()).contains('icon-button')
     .expect(getActiveElementAriaLabel()).eql('Reply')
     .expect(getActiveElementInsideNthStatus()).eql('1')