« get me outta code hell

Make menubar select first item, like context menus - mtui - Music Text User Interface - user-friendly command line music player
about summary refs log tree commit diff
diff options
context:
space:
mode:
authorFlorrie <towerofnix@gmail.com>2019-09-30 16:28:23 -0300
committerFlorrie <towerofnix@gmail.com>2019-09-30 16:34:06 -0300
commit79b7d2a133a566fbc10666ea5b74b4d15d28e481 (patch)
treead334daadbd58534f33f20be3cdcc8f017c3037f
parent1418f06eda87edd99ae5bce4f29247e486930bcb (diff)
Make menubar select first item, like context menus
This also tidies up the logic for changing the selected element from
context menus to the menubar, fixing a bug where the menubar forgets
which element was selected before it, and not re-introducing the bug
which the complex logic fixed in the first place (which was the menubar
seeing the context menu as the previously selected element, when the
menu will be destroyed by the time the menubar restores its selection).
-rw-r--r--todo.txt1
-rw-r--r--ui.js32
2 files changed, 15 insertions, 18 deletions
diff --git a/todo.txt b/todo.txt
index 8a86e5f..e95a8e7 100644
--- a/todo.txt
+++ b/todo.txt
@@ -420,3 +420,4 @@ TODO: Investigate menubar UX - now that it uses KeyboardSelector, it's very
       comparable in interaction to ContextMenus. To match context menus,
       should we make its selection index reset to zero (i.e. the 'mtui' option)
       whenever it's selected (via the keyboard)?
+      (Done! Decided yes.)
diff --git a/ui.js b/ui.js
index 7037e87..674541d 100644
--- a/ui.js
+++ b/ui.js
@@ -1144,6 +1144,14 @@ class AppElement extends FocusElement {
       if (this.menubar.isSelected) {
         this.menubar.restoreSelection()
       } else {
+        // If we've got a menu open, close it, restoring selection to the
+        // element selected before the menu was opened, so the menubar will
+        // see that as the previously selected element (instead of the context
+        // menu - which will be closed irregardless and gone when the menubar
+        // tries to restore the selection).
+        if (this.menuLayer.children[0]) {
+          this.menuLayer.children[0].close()
+        }
         this.menubar.select()
       }
     } else if (keyBuf.equals(Buffer.from([5]))) { // Ctrl-E
@@ -3499,17 +3507,7 @@ class ContextMenu extends FocusElement {
     if (this.visible) {
       const newlySelected = this.root.selectedElement
       this.close()
-
-      // Bad hack: if the menubar is the selected thing, directly select it
-      // (hence setting its previously selected element to whatever we just
-      // restored the selection to as the context menu).
-      const menubar = newlySelected.directAncestors.find(
-        el => el instanceof Menubar)
-      if (menubar) {
-        menubar.select()
-      } else {
-        this.selectedBefore.root.select(newlySelected)
-      }
+      this.selectedBefore.root.select(newlySelected)
     }
   }
 
@@ -3694,18 +3692,16 @@ class Menubar extends ListScrollForm {
   }
 
   select() {
-    // The context menu disappears when it's deselected, so we really want to
-    // use whatever element was selected before the menu was opened.
+    // When the menubar is selected from the menubar's context menu, the UI
+    // looks like it's "popping" a state, so don't reset the selected index to
+    // the start - something we only do when we "newly" select the menubar.
     if (this.contextMenu && this.contextMenu.isSelected) {
-      // ...Unless it was the menubar that was already selected.
-      if (!this.contextMenu.selectedBefore.directAncestors.includes(this)) {
-        this.selectedBefore = this.contextMenu.selectedBefore
-      }
+      this.root.select(this)
     } else {
       this.selectedBefore = this.root.selectedElement
+      this.firstInput()
     }
 
-    this.root.select(this)
     this.keyboardSelector.reset()
   }