« get me outta code hell

mtui - Music Text User Interface - user-friendly command line music player
about summary refs log tree commit diff
diff options
context:
space:
mode:
-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()
   }