Debounce showing FilePatch when navigating with keyboard

lib/views/staging-view.js

@@ -9,9 +9,28 @@ import MergeConflictListItemView from './merge-conflict-list-item-view'
 import CompositeListSelection from './composite-list-selection'
 import {shortenSha} from '../helpers'
 
+const debounce = (fn, wait) => {
+  let timeout
+  return (...args) => {
+    return new Promise(resolve => {
+      clearTimeout(timeout)
+      timeout = setTimeout(() => {
+        resolve(fn(...args))
+      }, wait)
+    })
+  }
+}
+
 export default class StagingView {
   constructor (props) {
     this.props = props
+    atom.config.observe("github.keyboardNavigationDelay", (value) => {
+      if (value === 0) {
+        this.debouncedDidChangeSelectedItem = this.didChangeSelectedItem.bind(this)
+      } else {
+        this.debouncedDidChangeSelectedItem = debounce(this.didChangeSelectedItem.bind(this), value)
+      }
+    })
     this.mouseSelectionInProgress = false
     this.listElementsByItem = new WeakMap()
     this.registerItemElement = this.registerItemElement.bind(this)
@@ -26,7 +45,6 @@ export default class StagingView {
       },
       idForItem: item => item.filePath
     })
-    this.onDidChangeSelectedItem()
     etch.initialize(this)
 
     this.subscriptions = new CompositeDisposable()
@@ -81,35 +99,34 @@ export default class StagingView {
   selectPrevious (preserveTail = false) {
     this.selection.selectPreviousItem(preserveTail)
     this.selection.coalesce()
-    this.onDidChangeSelectedItem()
+    if (!preserveTail) this.debouncedDidChangeSelectedItem()
     return etch.update(this)
   }
 
   selectNext (preserveTail = false) {
     this.selection.selectNextItem(preserveTail)
     this.selection.coalesce()
-    this.onDidChangeSelectedItem()
+    if (!preserveTail) this.debouncedDidChangeSelectedItem()
     return etch.update(this)
   }
 
   selectAll () {
     this.selection.selectAllItems()
     this.selection.coalesce()
-    this.onDidChangeSelectedItem()
     return etch.update(this)
   }
 
   selectFirst (preserveTail = false) {
     this.selection.selectFirstItem(preserveTail)
     this.selection.coalesce()
-    this.onDidChangeSelectedItem()
+    if (!preserveTail) this.debouncedDidChangeSelectedItem()
     return etch.update(this)
   }
 
   selectLast (preserveTail = false) {
     this.selection.selectLastItem(preserveTail)
     this.selection.coalesce()
-    this.onDidChangeSelectedItem()
+    if (!preserveTail) this.debouncedDidChangeSelectedItem()
     return etch.update(this)
   }
 
@@ -118,7 +135,7 @@ export default class StagingView {
     if (headItem) this.listElementsByItem.get(headItem).scrollIntoViewIfNeeded()
   }
 
-  onDidChangeSelectedItem () {
+  didChangeSelectedItem () {
     const selectedItem = this.selection.getHeadItem()
     if (!this.isFocused() || !selectedItem) return
 
@@ -271,7 +288,6 @@ export default class StagingView {
 
   focus () {
     this.element.focus()
-    this.onDidChangeSelectedItem()
   }
 
   isFocused () {

package.json

@@ -53,5 +53,12 @@
         "1.1.0": "consumeStatusBar_1_1"
       }
     }
+  },
+  "configSchema": {
+    "keyboardNavigationDelay": {
+      "type": "number",
+      "default": 300,
+      "description": "How long to wait before showing a diff view after navigating to an entry with the keyboard"
+    }
   }
 }

test/views/staging-view.test.js

@@ -68,45 +68,101 @@ describe('StagingView', () => {
   })
 
   describe('when the selection changes', function () {
-    it('notifies the parent component via the appropriate callback', async function () {
-      const filePatches = [
-        { filePath: 'a.txt', status: 'modified' },
-        { filePath: 'b.txt', status: 'deleted' }
-      ]
-      const mergeConflicts = [{
-        filePath: 'conflicted-path',
-        status: {
-          file: 'modified',
-          ours: 'deleted',
-          theirs: 'modified'
-        }
-      }]
-
-      const didSelectFilePath = sinon.spy()
-      const didSelectMergeConflictFile = sinon.spy()
-
-      const view = new StagingView({
-        didSelectFilePath, didSelectMergeConflictFile,
-        unstagedChanges: filePatches, mergeConflicts, stagedChanges: []
+    describe("when github.keyboardNavigationDelay is 0", () => {
+      beforeEach(() => {
+        atom.config.set("github.keyboardNavigationDelay", 0)
       })
-      document.body.appendChild(view.element)
-      assert.equal(didSelectFilePath.callCount, 0)
 
-      view.focus()
-      assert.isTrue(didSelectFilePath.calledWith(filePatches[0].filePath))
-      await view.selectNext()
-      assert.isTrue(didSelectFilePath.calledWith(filePatches[1].filePath))
-      await view.selectNext()
-      assert.isTrue(didSelectMergeConflictFile.calledWith(mergeConflicts[0].filePath))
+      it('synchronously notifies the parent component via the appropriate callback', async function () {
+        const filePatches = [
+          { filePath: 'a.txt', status: 'modified' },
+          { filePath: 'b.txt', status: 'deleted' }
+        ]
+        const mergeConflicts = [{
+          filePath: 'conflicted-path',
+          status: {
+            file: 'modified',
+            ours: 'deleted',
+            theirs: 'modified'
+          }
+        }]
+
+        const didSelectFilePath = sinon.spy()
+        const didSelectMergeConflictFile = sinon.spy()
+
+        const view = new StagingView({
+          didSelectFilePath, didSelectMergeConflictFile,
+          unstagedChanges: filePatches, mergeConflicts, stagedChanges: []
+        })
+        document.body.appendChild(view.element)
+        assert.equal(didSelectFilePath.callCount, 0)
+
+        view.focus()
+        await view.selectNext()
+        assert.isTrue(didSelectFilePath.calledWith(filePatches[1].filePath))
+        await view.selectNext()
+        assert.isTrue(didSelectMergeConflictFile.calledWith(mergeConflicts[0].filePath))
+
+        document.body.focus()
+        assert.isFalse(view.isFocused())
+        didSelectFilePath.reset()
+        didSelectMergeConflictFile.reset()
+        await view.selectNext()
+        assert.equal(didSelectMergeConflictFile.callCount, 0)
+
+        view.element.remove()
+      })
+    })
 
-      document.body.focus()
-      assert.isFalse(view.isFocused())
-      didSelectFilePath.reset()
-      didSelectMergeConflictFile.reset()
-      await view.selectNext()
-      assert.equal(didSelectMergeConflictFile.callCount, 0)
+    describe("when github.keyboardNavigationDelay is greater than 0", () => {
+      beforeEach(() => {
+        atom.config.set("github.keyboardNavigationDelay", 50)
+      })
 
-      view.element.remove()
+      it('asynchronously notifies the parent component via the appropriate callback', async function () {
+        const filePatches = [
+          { filePath: 'a.txt', status: 'modified' },
+          { filePath: 'b.txt', status: 'deleted' }
+        ]
+        const mergeConflicts = [{
+          filePath: 'conflicted-path',
+          status: {
+            file: 'modified',
+            ours: 'deleted',
+            theirs: 'modified'
+          }
+        }]
+
+        const didSelectFilePath = sinon.spy()
+        const didSelectMergeConflictFile = sinon.spy()
+
+        const view = new StagingView({
+          didSelectFilePath, didSelectMergeConflictFile,
+          unstagedChanges: filePatches, mergeConflicts, stagedChanges: []
+        })
+        document.body.appendChild(view.element)
+        assert.equal(didSelectFilePath.callCount, 0)
+
+        view.focus()
+        await view.selectNext()
+        assert.isFalse(didSelectFilePath.calledWith(filePatches[1].filePath))
+        await new Promise(resolve => setTimeout(resolve, 100))
+        assert.isTrue(didSelectFilePath.calledWith(filePatches[1].filePath))
+        await view.selectNext()
+        assert.isFalse(didSelectMergeConflictFile.calledWith(mergeConflicts[0].filePath))
+        await new Promise(resolve => setTimeout(resolve, 100))
+        assert.isTrue(didSelectMergeConflictFile.calledWith(mergeConflicts[0].filePath))
+
+        document.body.focus()
+        assert.isFalse(view.isFocused())
+        didSelectFilePath.reset()
+        didSelectMergeConflictFile.reset()
+        await view.selectNext()
+        await new Promise(resolve => setTimeout(resolve, 100))
+        assert.equal(didSelectMergeConflictFile.callCount, 0)
+
+        view.element.remove()
+      })
     })
 
     it('autoscroll to the selected item if it is out of view', async function () {