From b2d7fad435e40fa8736edc37dae9ba743c7cdbec Mon Sep 17 00:00:00 2001
From: Nolan Lawson <nolan@nolanlawson.com>
Date: Wed, 7 Aug 2019 20:38:01 -0700
Subject: [PATCH] perf: only update draggable x/y state at end of drag (#1379)

* perf: only update draggable x/y state at end of drag

This is more intelligent and more performant than using requestIdleCallback willy-nilly. We can just update the store when the user is actually done dragging the button.

* remove console.log

* consistent syntax
---
 src/routes/_components/Draggable.html         | 17 ++++++-----
 .../components/MediaFocalPointDialog.html     | 17 +++++------
 src/routes/_utils/throttleTimer.js            | 20 +++++++++++++
 src/routes/_utils/throttleTimers.js           | 28 -------------------
 4 files changed, 37 insertions(+), 45 deletions(-)
 create mode 100644 src/routes/_utils/throttleTimer.js
 delete mode 100644 src/routes/_utils/throttleTimers.js

diff --git a/src/routes/_components/Draggable.html b/src/routes/_components/Draggable.html
index 0b360b1b..59f6eff2 100644
--- a/src/routes/_components/Draggable.html
+++ b/src/routes/_components/Draggable.html
@@ -31,19 +31,17 @@
 </style>
 <script>
   import { observe } from 'svelte-extras'
-  import {
-    throttleRequestAnimationFrame,
-    throttleRequestPostAnimationFrame
-  } from '../_utils/throttleTimers'
+  import { throttleTimer } from '../_utils/throttleTimer'
   import { pointerUp, pointerDown, pointerLeave, pointerMove } from '../_utils/pointerEvents'
+  import { requestPostAnimationFrame } from '../_utils/requestPostAnimationFrame'
 
   // ensure DOM writes only happen once after a rAF
-  const updateIndicatorStyle = throttleRequestAnimationFrame()
-  const updateIndicatorClass = throttleRequestAnimationFrame()
-  const updateDraggableClass = throttleRequestAnimationFrame()
+  const updateIndicatorStyle = throttleTimer(requestAnimationFrame)
+  const updateIndicatorClass = throttleTimer(requestAnimationFrame)
+  const updateDraggableClass = throttleTimer(requestAnimationFrame)
 
   // ensure DOM reads only happen once after a rPAF
-  const calculateGBCR = throttleRequestPostAnimationFrame()
+  const calculateGBCR = throttleTimer(requestPostAnimationFrame)
 
   const clamp = x => Math.max(0, Math.min(1, x))
 
@@ -53,7 +51,9 @@
         if (dragging) {
           this.fire('dragStart')
         } else {
+          const { x, y } = this.get()
           this.fire('dragEnd')
+          this.fire('change', { x, y })
         }
       }, { init: false })
       this.observe('indicatorStyle', indicatorStyle => {
@@ -115,7 +115,6 @@
             const x = clamp((e.clientX - rect.left - offsetX) / rect.width)
             const y = clamp((e.clientY - rect.top - offsetY) / rect.height)
             this.set({ x, y })
-            this.fire('change', { x, y })
           })
         }
       },
diff --git a/src/routes/_components/dialog/components/MediaFocalPointDialog.html b/src/routes/_components/dialog/components/MediaFocalPointDialog.html
index 762a0623..1ad71f31 100644
--- a/src/routes/_components/dialog/components/MediaFocalPointDialog.html
+++ b/src/routes/_components/dialog/components/MediaFocalPointDialog.html
@@ -30,9 +30,9 @@
           indicatorHeight={52}
           x={indicatorX}
           y={indicatorY}
+          on:dragStart="onDragStart()"
+          on:dragEnd="onDragEnd()"
           on:change="onDraggableChange(event)"
-          on:dragStart="set({dragging: true})"
-          on:dragEnd="set({dragging: false})"
         >
           <SvgIcon
             className="media-focal-point-indicator-svg"
@@ -193,11 +193,6 @@
   import { intrinsicScale } from '../../../_thirdparty/intrinsic-scale/intrinsicScale'
   import { resize } from '../../../_utils/events'
   import Draggable from '../../Draggable.html'
-  import { throttleScheduleIdleTask } from '../../../_utils/throttleTimers'
-
-  // Updating the focal points in the store causes a lot of computations (extra JS work),
-  // so we really don't want to do it for every drag event.
-  const updateFocalPointsInStore = throttleScheduleIdleTask()
 
   const parseAndValidateFloat = rawText => {
     let float = parseFloat(rawText)
@@ -307,7 +302,7 @@
         observeAndSync('rawFocusY', 'focusY')
       },
       onDraggableChange ({ x, y }) {
-        updateFocalPointsInStore(() => {
+        scheduleIdleTask(() => {
           const focusX = parseAndValidateFloat(percentToCoords(x * 100))
           const focusY = parseAndValidateFloat(percentToCoords(100 - (y * 100)))
           const { realm, index, media } = this.get()
@@ -319,6 +314,12 @@
           }
         })
       },
+      onDragStart () {
+        this.set({ dragging: true })
+      },
+      onDragEnd () {
+        this.set({ dragging: false })
+      },
       measure () {
         requestAnimationFrame(() => {
           if (!this.refs.container) {
diff --git a/src/routes/_utils/throttleTimer.js b/src/routes/_utils/throttleTimer.js
new file mode 100644
index 00000000..2dfbc011
--- /dev/null
+++ b/src/routes/_utils/throttleTimer.js
@@ -0,0 +1,20 @@
+// Sometimes we want to queue multiple requestAnimationFrames but only run the last one.
+// It's tedious to do this using cancelAnimationFrame, so this is a utility to throttle
+// a timer such that it only runs the last callback when it fires.
+
+export const throttleTimer = timer => {
+  let queuedCallback
+
+  const flush = () => {
+    const callback = queuedCallback
+    queuedCallback = null
+    callback()
+  }
+
+  return callback => {
+    if (!queuedCallback) {
+      timer(flush)
+    }
+    queuedCallback = callback
+  }
+}
diff --git a/src/routes/_utils/throttleTimers.js b/src/routes/_utils/throttleTimers.js
deleted file mode 100644
index bf0aff43..00000000
--- a/src/routes/_utils/throttleTimers.js
+++ /dev/null
@@ -1,28 +0,0 @@
-// Sometimes we want to queue multiple requestAnimationFrames but only run the last one.
-// It's tedious to do this using cancelAnimationFrame, so this is a utility to throttle
-// a timer such that it only runs the last callback when it fires.
-
-import { requestPostAnimationFrame } from './requestPostAnimationFrame'
-import { scheduleIdleTask } from './scheduleIdleTask'
-
-const throttle = (timer) => {
-  return () => {
-    let queuedCallback
-
-    return function throttledRaf (callback) {
-      const alreadyQueued = !!queuedCallback
-      queuedCallback = callback
-      if (!alreadyQueued) {
-        timer(() => {
-          const cb = queuedCallback
-          queuedCallback = null
-          cb()
-        })
-      }
-    }
-  }
-}
-
-export const throttleRequestAnimationFrame = throttle(requestAnimationFrame)
-export const throttleRequestPostAnimationFrame = throttle(requestPostAnimationFrame)
-export const throttleScheduleIdleTask = throttle(scheduleIdleTask)