Several cosmetic fixes for off-by-one in padding and display issues when resetting lists

art/themes/luciano_blocktronics/theme.hjson

@@ -495,8 +495,8 @@
 						VM1: {
 							height: 15
 							width: 30
-							itemFormat: "|00|03{subject}|00 {statusIndicator}"
+							itemFormat: "|00{statusIndicator} |00|03{subject}"
-							focusItemFormat: "|00|19|15{subject}|00 {statusIndicator}"
+							focusItemFormat: "|00{statusIndicator} |00|19|15{subject}"
 						}
 						MT2: {
 							height: 15

core/full_menu_view.js

@@ -59,7 +59,7 @@ function FullMenuView(options) {
         }
 
         for (let i = 0; i < this.dimens.height; i++) {
-            const text = `${strUtil.pad(this.fillChar, width, this.fillChar, 'left')}`;
+            const text = strUtil.pad('', width, this.fillChar);
             this.client.term.write(
                 `${ansi.goto(
                     this.position.row + i,
@@ -77,6 +77,7 @@ function FullMenuView(options) {
             this.autoAdjustHeightIfEnabled();
 
             this.pages = []; // reset
+            this.currentPage = 0; // reset currentPage when pages reset
 
             // Calculate number of items visible per column
             this.itemsPerRow = Math.floor(this.dimens.height / (this.itemSpacing + 1));
@@ -240,14 +241,25 @@ function FullMenuView(options) {
             sgr = index === this.focusedItemIndex ? this.getFocusSGR() : this.getSGR();
         }
 
-        let renderLength = strUtil.renderStringLength(text);
+        const renderLength = strUtil.renderStringLength(text);
-        if (this.hasTextOverflow() && item.col + renderLength > this.dimens.width) {
+
+        let relativeColumn = item.col - this.position.col;
+        if (relativeColumn < 0) {
+            relativeColumn = 0;
+            this.client.log.warn(
+                { itemCol: item.col, positionColumn: this.position.col },
+                'Invalid item column detected in full menu'
+            );
+        }
+
+        if (relativeColumn + renderLength > this.dimens.width) {
+            const overflow = this.hasTextOverflow() ? this.textOverflow : '';
             text =
                 strUtil.renderSubstr(
                     text,
                     0,
-                    this.dimens.width - (item.col + this.textOverflow.length)
+                    this.dimens.width - (relativeColumn + overflow.length)
-                ) + this.textOverflow;
+                ) + overflow;
         }
 
         let padLength = Math.min(item.fixedLength + 1, this.dimens.width);
@@ -270,7 +282,21 @@ FullMenuView.prototype.redraw = function () {
 
     this.cachePositions();
 
+    // In case we get in a bad state, try to recover
+    if (this.currentPage < 0) {
+        this.currentPage = 0;
+    }
+
     if (this.items.length) {
+        if (
+            this.currentPage > this.pages.length ||
+            !_.isObject(this.pages[this.currentPage])
+        ) {
+            this.client.log.warn(
+                { currentPage: this.currentPage, pagesLength: this.pages.length },
+                'Invalid state! in full menu redraw'
+            );
+        } else {
             for (
                 let i = this.pages[this.currentPage].start;
                 i <= this.pages[this.currentPage].end;
@@ -280,6 +306,7 @@ FullMenuView.prototype.redraw = function () {
                 this.drawItem(i);
             }
         }
+    }
 };
 
 FullMenuView.prototype.setHeight = function (height) {
@@ -358,6 +385,10 @@ FullMenuView.prototype.setItems = function (items) {
         this.oldDimens = Object.assign({}, this.dimens);
     }
 
+    // Reset the page on new items
+    this.currentPage = 0;
+    this.focusedItemIndex = 0;
+
     FullMenuView.super_.prototype.setItems.call(this, items);
 
     this.positionCacheExpired = true;
@@ -377,6 +408,15 @@ FullMenuView.prototype.focusNext = function () {
         this.clearPage();
         this.focusedItemIndex = 0;
         this.currentPage = 0;
+    } else {
+        if (
+            this.currentPage > this.pages.length ||
+            !_.isObject(this.pages[this.currentPage])
+        ) {
+            this.client.log.warn(
+                { currentPage: this.currentPage, pagesLength: this.pages.length },
+                'Invalid state in focusNext for full menu view'
+            );
         } else {
             this.focusedItemIndex++;
             if (this.focusedItemIndex > this.pages[this.currentPage].end) {
@@ -384,6 +424,7 @@ FullMenuView.prototype.focusNext = function () {
                 this.currentPage++;
             }
         }
+    }
 
     this.redraw();
 
@@ -397,11 +438,21 @@ FullMenuView.prototype.focusPrevious = function () {
         this.currentPage = this.pages.length - 1;
     } else {
         this.focusedItemIndex--;
+        if (
+            this.currentPage > this.pages.length ||
+            !_.isObject(this.pages[this.currentPage])
+        ) {
+            this.client.log.warn(
+                { currentPage: this.currentPage, pagesLength: this.pages.length },
+                'Bad focus state, ignoring call to focusPrevious.'
+            );
+        } else {
             if (this.focusedItemIndex < this.pages[this.currentPage].start) {
                 this.clearPage();
                 this.currentPage--;
             }
         }
+    }
 
     this.redraw();
 

core/string_util.js

@@ -145,29 +145,29 @@ function pad(s, len, padChar, justify, stringSGR, padSGR, useRenderLen) {
     useRenderLen = _.isUndefined(useRenderLen) ? true : useRenderLen;
 
     const renderLen = useRenderLen ? renderStringLength(s) : s.length;
-    const padlen = len >= renderLen ? len - renderLen : 0;
+    const padLen = len > renderLen ? len - renderLen : 0;
 
     switch (justify) {
         case 'L':
         case 'left':
-            s = `${stringSGR}${s}${padSGR}${Array(padlen).join(padChar)}`;
+            s = `${stringSGR}${s}${padSGR}${padChar.repeat(padLen)}`;
             break;
 
         case 'C':
         case 'center':
         case 'both':
             {
-                const right = Math.ceil(padlen / 2);
+                const right = Math.ceil(padLen / 2);
-                const left = padlen - right;
+                const left = padLen - right;
                 s = `${padSGR}${Array(left + 1).join(
                     padChar
-                )}${stringSGR}${s}${padSGR}${Array(right + 1).join(padChar)}`;
+                )}${stringSGR}${s}${padSGR}${padChar.repeat(right)}`;
             }
             break;
 
         case 'R':
         case 'right':
-            s = `${padSGR}${Array(padlen).join(padChar)}${stringSGR}${s}`;
+            s = `${padSGR}${padChar.repeat(padLen)}${stringSGR}${s}`;
             break;
 
         default:

core/vertical_menu_view.js

@@ -98,6 +98,11 @@ function VerticalMenuView(options) {
             sgr = index === self.focusedItemIndex ? self.getFocusSGR() : self.getSGR();
         }
 
+        const renderLength = strUtil.renderStringLength(text);
+        if (renderLength > this.dimens.width) {
+            text = strUtil.renderSubstr(text, 0, this.dimens.width);
+        }
+
         text = `${sgr}${strUtil.pad(
             `${text}${this.styleSGR1}`,
             this.dimens.width,
@@ -137,18 +142,16 @@ VerticalMenuView.prototype.redraw = function () {
     //  erase old items
     //  :TODO: optimize this: only needed if a item is removed or new max width < old.
     if (this.oldDimens) {
-        const blank = new Array(Math.max(this.oldDimens.width, this.dimens.width)).join(
+        const blank = ' '.repeat(Math.max(this.oldDimens.width, this.dimens.width));
-            ' '
+        let row = this.position.row;
-        );
-        let seq = ansi.goto(this.position.row, this.position.col) + this.getSGR() + blank;
-        let row = this.position.row + 1;
         const endRow = row + this.oldDimens.height - 2;
 
         while (row <= endRow) {
-            seq += ansi.goto(row, this.position.col) + blank;
+            this.client.term.write(
+                ansi.goto(row, this.position.col) + this.getSGR() + blank
+            );
             row += 1;
         }
-        this.client.term.write(seq);
         delete this.oldDimens;
     }
 
@@ -241,6 +244,7 @@ VerticalMenuView.prototype.setItems = function (items) {
     if (this.items && this.items.length) {
         this.oldDimens = Object.assign({}, this.dimens);
     }
+    this.focusedItemIndex = 0;
 
     VerticalMenuView.super_.prototype.setItems.call(this, items);