« get me outta code hell

Make tab switch between path element and listing, not between listings - 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>2018-08-15 15:40:36 -0300
committerFlorrie <towerofnix@gmail.com>2018-08-15 15:42:19 -0300
commitdd3afa50bd92152c73837c234661fe1d9fcda399 (patch)
treee0eef2fecde6342400b9fd9907ec19b1f556cb22
parent102d8e8e2b456523979e20a77a4f30d11b91688d (diff)
Make tab switch between path element and listing, not between listings
Do note the big fat comment.

I'm actually pretty happy with this commit. I think there's a fair
chance that this is the right behavior, and it simplifies a fair amount
of code, e.g. getting rid of manually setting curIndex on this.form and
restoring selection after Dialog.close() (that's handled by tui-lib
now). It also discards the "hack" I had to do to make the AppElement
form capture keyboard input in the first place. (I had to use addChild
with the container paneLeft/paneRight elements so that keyboard input on
their children, which are the inputs of the AppElement form, could be
captured.)
m---------tui-lib0
-rw-r--r--ui.js55
2 files changed, 35 insertions, 20 deletions
diff --git a/tui-lib b/tui-lib
-Subproject b3fb0187f9c02cb98d98c41c076036ee2b52845
+Subproject 68d5c75dce1c950468d07241261fb9f8c8e3d39
diff --git a/ui.js b/ui.js
index b6d525c..0314fba 100644
--- a/ui.js
+++ b/ui.js
@@ -41,32 +41,27 @@ class AppElement extends FocusElement {
 
     this.rootDirectory = process.env.HOME + '/.mtui'
 
-    this.form = new Form()
-    this.addChild(this.form)
-
     this.paneLeft = new Pane()
-    this.form.addChild(this.paneLeft)
+    this.addChild(this.paneLeft)
 
     this.paneRight = new Pane()
-    this.form.addChild(this.paneRight)
+    this.addChild(this.paneRight)
 
     this.tabber = new Tabber()
     this.paneLeft.addChild(this.tabber)
-    this.form.addInput(this.tabber, false)
 
     this.newGrouplikeListing()
 
     this.queueListingElement = new QueueListingElement(this.recordStore)
     this.queueListingElement.loadGrouplike(this.queueGrouplike)
     this.paneRight.addChild(this.queueListingElement)
-    this.form.addInput(this.queueListingElement, false)
 
     this.queueListingElement.on('queue', item => this.playGrouplikeItem(item))
     this.queueListingElement.on('remove', item => this.unqueueGrouplikeItem(item))
     this.queueListingElement.on('shuffle', () => this.shuffleQueue())
     this.queueListingElement.on('clear', () => this.clearQueue())
     this.queueListingElement.on('select main listing',
-      () => this.form.selectInput(this.tabber))
+      () => this.root.select(this.tabber))
 
     this.playbackPane = new Pane()
     this.addChild(this.playbackPane)
@@ -97,7 +92,7 @@ class AppElement extends FocusElement {
   }
 
   selected() {
-    this.root.select(this.form)
+    this.root.select(this.tabber)
   }
 
   newGrouplikeListing() {
@@ -209,7 +204,7 @@ class AppElement extends FocusElement {
     })
 
     const handleSelectFromPathElement = item => {
-      this.form.selectInput(grouplikeListing)
+      this.root.select(grouplikeListing)
       if (isGroup(item)) {
         grouplikeListing.loadGrouplike(item)
       } else if (item[parentSymbol]) {
@@ -218,7 +213,6 @@ class AppElement extends FocusElement {
       }
     }
 
-    // this.form.addInput(grouplikeListing.pathElement, false)
     grouplikeListing.pathElement.on('select', item => handleSelectFromPathElement(item))
 
     return grouplikeListing
@@ -244,7 +238,6 @@ class AppElement extends FocusElement {
     }
 
     this.alertDialog.close()
-    this.root.select(this.form)
 
     grouplike = await processSmartPlaylist(grouplike)
 
@@ -274,7 +267,6 @@ class AppElement extends FocusElement {
 
     dialog.on('cancelled', () => {
       dialog.close()
-      this.root.select(this.form)
     })
   }
 
@@ -339,11 +331,9 @@ class AppElement extends FocusElement {
     } else if (telc.isShiftDown(keyBuf) || telc.isCaselessLetter(keyBuf, 'n')) {
       this.playNextTrack(this.playingTrack)
     } else if (telc.isCharacter(keyBuf, '1') && this.tabber.selectable) {
-      this.form.curIndex = this.form.inputs.indexOf(this.tabber)
-      this.form.updateSelectedElement()
+      this.root.select(this.tabber)
     } else if (telc.isCharacter(keyBuf, '2') && this.queueListingElement.selectable) {
-      this.form.curIndex = this.form.inputs.indexOf(this.queueListingElement)
-      this.form.updateSelectedElement()
+      this.root.select(this.queueListingElement)
     } else if (keyBuf.equals(Buffer.from([5]))) { // Ctrl-E
       this.editMode = !this.editMode
     } else if (this.editMode && keyBuf.equals(Buffer.from([14]))) { // ctrl-N
@@ -665,7 +655,32 @@ class AppElement extends FocusElement {
   }
 }
 
-class GrouplikeListingElement extends FocusElement {
+class GrouplikeListingElement extends Form {
+  // TODO: This is a Form, which means that it captures the tab key. The result
+  // of this is that you cannot use Tab to navigate the top-level application.
+  // Accordingly, I've made AppElement a FocusElement and not a Form and re-
+  // factored calls of addInput to addChild. However, I'm not sure that this is
+  // the "correct" or most intuitive behavior. Should the tab key be usable to
+  // navigate the entire interface? I don't know. I've gone with the current
+  // behavior (GrouplikeListingElement as a Form) because it feels right at the
+  // moment, but we'll see, I suppose.
+  //
+  // In order to let tab navigate through all UI elements (or rather, the top-
+  // level application as well as GrouplikeListingElements, which are a sort of
+  // nested Form), the AppElement would have to be changed to be a Form again
+  // (replacing addChild with addInput where appropriate). Furthermore, while
+  // the GrouplikeListingElement should stay as a Form subclass, it should be
+  // modified so that it does not capture tab if there is no next element to
+  // select, and vice versa for shift-tab and the previous element. This should
+  // probably be implemented in tui-lib as a flag on Form (captureTabOnEnds,
+  // or something).
+  //
+  // (PS AppElement apparently used a "this.form" property, instead of directly
+  // inheriting from Form, apparently. That's more or less adjacent to the
+  // point. It's removed now. You'll have to add it back, if wanted.)
+  //
+  // August 15th, 2018
+
   constructor(recordStore) {
     super()
 
@@ -675,7 +690,7 @@ class GrouplikeListingElement extends FocusElement {
     this.recordStore = recordStore
 
     this.form = this.getNewForm()
-    this.addChild(this.form)
+    this.addInput(this.form)
 
     this.form.on('selected input', input => {
       if (input && this.pathElement) {
@@ -684,7 +699,7 @@ class GrouplikeListingElement extends FocusElement {
     })
 
     this.pathElement = new PathElement()
-    this.addChild(this.pathElement)
+    this.addInput(this.pathElement)
 
     this.commentLabel = new WrapLabel()
     this.addChild(this.commentLabel)