Fix keyboard navigation with Form having SelectList and SelectListMul…

…ti (#142), and some 'focus' event issues
cronvel · Jan 11, 2021 · 12a0776 · 12a0776
1 parent 48ff6d0
commit 12a0776
Show file tree

Hide file tree

Showing 10 changed files with 53 additions and 27 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,4 +1,10 @@
 
+v1.45.0
+-------
+
+Fix keyboard navigation with Form having SelectList and SelectListMulti (#142), and some 'focus' event issues
+
+
 v1.44.3
 -------
 

diff --git a/lib/document/BaseMenu.js b/lib/document/BaseMenu.js
@@ -201,7 +201,7 @@ BaseMenu.prototype.onKey = function( key , trash , data ) {
 		case 'previous' :
 			this.focusChild = this.focusPreviousChild( ! this.maxPage ) ;
 			if ( this.focusChild === this.children[ 0 ] && this.maxPage && this.page > 0 ) {
-				this.previousPage( 'cycle' ) ;
+				this.previousPage( 'backCycle' ) ;
 			}
 			break ;
 		case 'next' :
@@ -212,7 +212,7 @@ BaseMenu.prototype.onKey = function( key , trash , data ) {
 			break ;
 		case 'previousPage' :
 			if ( this.maxPage && this.page > 0 ) {
-				this.previousPage( 'cycle' ) ;
+				this.previousPage( 'backCycle' ) ;
 			}
 			break ;
 		case 'nextPage' :
@@ -222,7 +222,7 @@ BaseMenu.prototype.onKey = function( key , trash , data ) {
 			break ;
 		case 'firstPage' :
 			if ( this.maxPage && this.page !== 0 ) {
-				this.toPage( 0 , 'cycle' ) ;
+				this.toPage( 0 , 'backCycle' ) ;
 			}
 			break ;
 		case 'lastPage' :
@@ -240,18 +240,22 @@ BaseMenu.prototype.onKey = function( key , trash , data ) {
 
 
 BaseMenu.prototype.onWheel = function( data ) {
-	if ( data.yDirection < 0 ) { this.previousPage( 'cycle' ) ; }
+	if ( data.yDirection < 0 ) { this.previousPage( 'backCycle' ) ; }
 	else if ( data.yDirection > 0 ) { this.nextPage( 'cycle' ) ; }
 } ;
 
 
 
 BaseMenu.prototype.onFocus = function( focus , type ) {
-	if ( type === 'cycle' ) { return ; }
+	if ( type === 'cycle' || type === 'backCycle' ) { return ; }
+	//if ( type === 'backCycle' ) { return ; }
 
 	if ( focus ) {
-		if ( this.focusChild ) { this.document.giveFocusTo( this.focusChild , 'delegate' ) ; }
-		else { this.focusChild = this.focusNextChild() ; }
+		// Defer to the next tick to avoid recursive events producing wrong listener order
+		process.nextTick( () => {
+			if ( this.focusChild && ! this.focusChild.destroyed ) { this.document.giveFocusTo( this.focusChild , 'delegate' ) ; }
+			else { this.focusChild = this.focusNextChild() ; }
+		} ) ;
 	}
 } ;
 

diff --git a/lib/document/ColumnMenuMulti.js b/lib/document/ColumnMenuMulti.js
@@ -110,7 +110,7 @@ ColumnMenuMulti.prototype.onKey = function( key , altKeys , data ) {
 		case 'previous' :
 			this.focusChild = this.focusPreviousChild( ! this.maxPage ) ;
 			if ( this.focusChild === this.children[ 0 ] && this.maxPage && this.page > 0 ) {
-				this.previousPage( 'cycle' ) ;
+				this.previousPage( 'backCycle' ) ;
 			}
 			break ;
 		case 'next' :
@@ -121,7 +121,7 @@ ColumnMenuMulti.prototype.onKey = function( key , altKeys , data ) {
 			break ;
 		case 'previousPage' :
 			if ( this.maxPage && this.page > 0 ) {
-				this.previousPage( 'cycle' ) ;
+				this.previousPage( 'backCycle' ) ;
 			}
 			break ;
 		case 'nextPage' :
@@ -131,7 +131,7 @@ ColumnMenuMulti.prototype.onKey = function( key , altKeys , data ) {
 			break ;
 		case 'firstPage' :
 			if ( this.maxPage && this.page !== 0 ) {
-				this.toPage( 0 , 'cycle' ) ;
+				this.toPage( 0 , 'backCycle' ) ;
 			}
 			break ;
 		case 'lastPage' :

diff --git a/lib/document/Document.js b/lib/document/Document.js
@@ -295,11 +295,11 @@ Document.prototype.focusPrevious = function() {
 					currentElement = currentElement.children[ currentElement.children.length - 1 ] ;
 				}
 
-				if ( ! currentElement.hidden ) { focusAware = this.giveFocusTo_( currentElement , 'cycle' ) ; }
+				if ( ! currentElement.hidden ) { focusAware = this.giveFocusTo_( currentElement , 'backCycle' ) ; }
 			}
 			else if ( currentElement.parent.parent ) {
 				currentElement = currentElement.parent ;
-				if ( ! currentElement.hidden ) { focusAware = this.giveFocusTo_( currentElement , 'cycle' ) ; }
+				if ( ! currentElement.hidden ) { focusAware = this.giveFocusTo_( currentElement , 'backCycle' ) ; }
 			}
 			else {
 				// We are at the top-level, just below the document, so cycle again to the last child of the last child
@@ -314,7 +314,7 @@ Document.prototype.focusPrevious = function() {
 					currentElement = currentElement.children[ currentElement.children.length - 1 ] ;
 				}
 
-				if ( ! currentElement.hidden ) { focusAware = this.giveFocusTo_( currentElement , 'cycle' ) ; }
+				if ( ! currentElement.hidden ) { focusAware = this.giveFocusTo_( currentElement , 'backCycle' ) ; }
 			}
 		}
 		else if ( currentElement.children.length ) {
@@ -325,7 +325,7 @@ Document.prototype.focusPrevious = function() {
 				currentElement = currentElement.children[ currentElement.children.length - 1 ] ;
 			}
 
-			if ( ! currentElement.hidden ) { focusAware = this.giveFocusTo_( currentElement , 'cycle' ) ; }
+			if ( ! currentElement.hidden ) { focusAware = this.giveFocusTo_( currentElement , 'backCycle' ) ; }
 		}
 		else {
 			// Nothing to do: no children, no parent, nothing...

diff --git a/lib/document/Element.js b/lib/document/Element.js
@@ -508,7 +508,7 @@ Element.prototype.focusPreviousChild = function( loop = true ) {
 			else { index = 0 ; break ; }
 		}
 
-		focusAware = this.document.giveFocusTo_( this.children[ index ] , 'cycle' ) ;
+		focusAware = this.document.giveFocusTo_( this.children[ index ] , 'backCycle' ) ;
 
 		// Exit if the focus was given to a focus-aware element or if we have done a full loop already
 		if ( focusAware || startingIndex === index ) { break ; }

diff --git a/lib/document/Form.js b/lib/document/Form.js
@@ -303,11 +303,14 @@ Form.prototype.onKey = function( key , altKeys , data ) {
 
 
 Form.prototype.onFocus = function( focus , type ) {
-	if ( type === 'cycle' ) { return ; }
+	if ( type === 'cycle' || type === 'backCycle' ) { return ; }
 
 	if ( focus ) {
-		if ( this.focusChild ) { this.document.giveFocusTo( this.focusChild , 'delegate' ) ; }
-		else { this.focusChild = this.focusNextChild() ; }
+		// Defer to the next tick to avoid recursive events producing wrong listener order
+		process.nextTick( () => {
+			if ( this.focusChild ) { this.document.giveFocusTo( this.focusChild , 'delegate' ) ; }
+			else { this.focusChild = this.focusNextChild() ; }
+		} ) ;
 	}
 } ;
 

diff --git a/lib/document/LabeledInput.js b/lib/document/LabeledInput.js
@@ -49,6 +49,7 @@ function LabeledInput( options ) {
 
 	// For text-input only
 	this.hiddenContent = options.hiddenContent ;
+	this.hasInputFocus = false ;
 
 	// For SelectList, this apply temp zIndex manipulation for the children to this element
 	this.interceptTempZIndex = true ;
@@ -303,7 +304,7 @@ LabeledInput.prototype.updateStatus = function() {
 		this.labelText.rightPadding = this.labelSubmittedRightPadding ;
 	}
 	else */
-	if ( this.hasFocus ) {
+	if ( this.hasFocus || this.hasInputFocus ) {
 		if ( this.labelText ) {
 			this.labelText.attr = this.labelFocusAttr ;
 			this.labelText.leftPadding = this.labelFocusLeftPadding ;
@@ -359,14 +360,27 @@ LabeledInput.prototype.onInputSubmit = function( data ) {
 
 LabeledInput.prototype.onFocus = function( focus , type ) {
 	this.hasFocus = focus ;
-	this.updateStatus() ;
-	this.draw() ;
+
+	if ( type === 'delegate' ) { return ; }
+
+	if ( focus && type !== 'backCycle' && this.input ) {
+		// Defer to the next tick to avoid recursive events producing wrong listener order
+		process.nextTick( () => {
+			this.document.giveFocusTo( this.input , 'delegate' ) ;
+		} ) ;
+	}
+	else {
+		// This is done by .onChildFocus() if there is an attached input
+		this.updateStatus() ;
+		//this.draw() ;
+		if ( this.labelText ) { this.labelText.draw() ; }
+	}
 } ;
 
 
 
 LabeledInput.prototype.onChildFocus = function( focus , type ) {
-	this.hasFocus = focus ;
+	this.hasInputFocus = focus ;
 	this.updateStatus() ;
 	if ( this.labelText ) { this.labelText.draw() ; }
 } ;

diff --git a/lib/document/SelectList.js b/lib/document/SelectList.js
@@ -117,7 +117,6 @@ SelectList.prototype.defaultOptions = {
 
 
 
-
 SelectList.prototype.destroy = function( isSubDestroy , noDraw = false ) {
 	if ( this.destroyed ) { return ; }
 
@@ -167,7 +166,7 @@ SelectList.prototype.toggle = function( show = null , noDraw = false ) {
 // selectOnly: don't draw, don't toggle
 SelectList.prototype.select = function( button , selectOnly ) {
 	// /!\ Warning! button can be a button (called from .onButtonSubmit()) or a buttonDef (called from .setValue()) /!\
-	// This is nasty an should be fixed!
+	// This is nasty and should be fixed!
 	// Ideally, a button should retain a 'def' pointer
 	var width ,
 		extraWidth = 1 + this.buttonSymbolWidth ,

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "terminal-kit",
-  "version": "1.44.3",
+  "version": "1.45.0",
   "description": "256 colors, keys and mouse, input field, progress bars, screen buffer (including 32-bit composition and image loading), text buffer, and many more... Whether you just need colors and styles, build a simple interactive command line tool or a complexe terminal app: this is the absolute terminal lib for Node.js!",
   "main": "lib/termkit.js",
   "directories": {

diff --git a/sample/document/labeled-input-test.js b/sample/document/labeled-input-test.js
@@ -40,7 +40,7 @@ var document = term.createDocument() ;
 
 var labeledInput ;
 
-//*
+/*
 labeledInput = new termkit.LabeledInput( {
 	parent: document ,
 	label: 'name: ' ,
@@ -50,7 +50,7 @@ labeledInput = new termkit.LabeledInput( {
 } ) ;
 //*/
 
-/*
+//*
 labeledInput = new termkit.LabeledInput( {
 	parent: document ,
 	label: 'name: ' ,