Fix (un)select calls re: consecutive root.select()

See code comments for explanation.

1 files changed, 47 insertions, 10 deletions

diff --git a/ui/Root.js b/ui/Root.js
index b2514ce..1933323 100644
--- a/ui/Root.js
+++ b/ui/Root.js
@@ -17,6 +17,8 @@ module.exports = class Root extends DisplayElement {
 
     this.cursorBlinkOffset = Date.now()
 
+    this.oldSelectionStates = []
+
     interfacer.on('inputData', buf => this.handleData(buf))
   }
 
@@ -80,15 +82,36 @@ module.exports = class Root extends DisplayElement {
     const newSelected = el
 
     // Relevant elements that COULD have their "isSelected" state change.
-    // We ignore elements where isSelected is undefined, because they aren't
-    // build to handle being selected, and they break the compare-old-and-new-
-    // state code below.
-    const relevantElements = [
+    const relevantElements = ([
       ...(oldSelected ? [...oldSelected.directAncestors, oldSelected] : []),
       ...(newSelected ? newSelected.directAncestors : [])
-    ].filter(el => typeof el.isSelected !== 'undefined')
-
-    const oldStates = relevantElements.map(el => [el, el.isSelected])
+    ]
+
+      // We ignore elements where isSelected is undefined, because they aren't
+      // built to handle being selected, and they break the compare-old-and-new-
+      // state code below.
+      .filter(el => typeof el.isSelected !== 'undefined')
+
+      // Get rid of duplicates - including any that occurred in the already
+      // existing array of selection states. (We only care about the oldest
+      // selection state, i.e. the one when we did the first .select().)
+      .reduce((acc, el) => {
+        // Duplicates from relevant elements of current .select()
+        if (acc.includes(el)) return acc
+        // Duplicates from already existing selection states
+        if (this.oldSelectionStates.some(x => x[0] === el)) return acc
+        return acc.concat([el])
+      }, []))
+
+    // Keep track of whether those elements were selected before we call the
+    // newly selected element's selected() function. We store these on a
+    // property because we might actually be adding to it from a previous
+    // root.select() call, if that one itself caused this root.select().
+    // One all root.select()s in the "chain" (as it is) have finished, we'll
+    // go through these states and call the appropriate .select/unselect()
+    // functions on each element whose .isSelected changed.
+    const selectionStates = relevantElements.map(el => [el, el.isSelected])
+    this.oldSelectionStates = this.oldSelectionStates.concat(selectionStates)
 
     this.selectedElement = el
 
@@ -96,6 +119,11 @@ module.exports = class Root extends DisplayElement {
     // passed element, even if it was already selected before.
     if (el.selected) el.selected()
     if (typeof el.focused === 'function') el.focused()
+
+    // If the selection changed as a result of the element's selected()
+    // function, stop here. We will leave calling the appropriate functions on
+    // the elements in the oldSelectionStates array to the final .select(),
+    // i.e. the one which caused no change in selected element.
     if (this.selectedElement !== newSelected) return
 
     // Compare the old "isSelected" state of every relevant element with their
@@ -103,11 +131,20 @@ module.exports = class Root extends DisplayElement {
     // functions. (Also call focused and unfocused for some sense of trying to
     // not break old programs, but, like, old programs are going to be broken
     // anyways.)
-    for (const [ el, wasSelected ] of oldStates) {
+    const states = this.oldSelectionStates.slice()
+    for (const [ el, wasSelected ] of states) {
+      // Now that we'll have processed it, we don't want it in the array
+      // anymore.
+      this.oldSelectionStates.shift()
+
       const { isSelected } = el
       if (isSelected && !wasSelected) {
-        if (el.selected) el.selected()
-        if (typeof el.focused === 'function') el.focused()
+        // Don't call these functions if this element is the newly selected
+        // one, because we already called them above!
+        if (el !== newSelected) {
+          if (el.selected) el.selected()
+          if (typeof el.focused === 'function') el.focused()
+        }
       } else if (wasSelected && !isSelected) {
         if (el.unselected) el.unselected()
         if (typeof el.unfocused === 'function') el.unfocused()