From e963f18ba43b7821e0347e528c26d0608685bf0f Mon Sep 17 00:00:00 2001 From: Nathan Byrd Date: Mon, 20 Feb 2023 13:08:11 -0600 Subject: [PATCH] Several cosmetic fixes for off-by-one in padding and display issues when resetting lists --- art/themes/luciano_blocktronics/theme.hjson | 4 +- core/full_menu_view.js | 87 ++++++++++++++++----- core/string_util.js | 12 +-- core/vertical_menu_view.js | 18 +++-- 4 files changed, 88 insertions(+), 33 deletions(-) diff --git a/art/themes/luciano_blocktronics/theme.hjson b/art/themes/luciano_blocktronics/theme.hjson index e3946ceb..638132fb 100644 --- a/art/themes/luciano_blocktronics/theme.hjson +++ b/art/themes/luciano_blocktronics/theme.hjson @@ -495,8 +495,8 @@ VM1: { height: 15 width: 30 - itemFormat: "|00|03{subject}|00 {statusIndicator}" - focusItemFormat: "|00|19|15{subject}|00 {statusIndicator}" + itemFormat: "|00{statusIndicator} |00|03{subject}" + focusItemFormat: "|00{statusIndicator} |00|19|15{subject}" } MT2: { height: 15 diff --git a/core/full_menu_view.js b/core/full_menu_view.js index b3bb2415..396e827f 100644 --- a/core/full_menu_view.js +++ b/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); - if (this.hasTextOverflow() && item.col + renderLength > this.dimens.width) { + const renderLength = strUtil.renderStringLength(text); + + 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.textOverflow; + this.dimens.width - (relativeColumn + overflow.length) + ) + overflow; } let padLength = Math.min(item.fixedLength + 1, this.dimens.width); @@ -270,14 +282,29 @@ 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) { - for ( - let i = this.pages[this.currentPage].start; - i <= this.pages[this.currentPage].end; - ++i + if ( + this.currentPage > this.pages.length || + !_.isObject(this.pages[this.currentPage]) ) { - this.items[i].focused = this.focusedItemIndex === i; - this.drawItem(i); + 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; + ++i + ) { + this.items[i].focused = this.focusedItemIndex === i; + this.drawItem(i); + } } } }; @@ -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; @@ -378,10 +409,20 @@ FullMenuView.prototype.focusNext = function () { this.focusedItemIndex = 0; this.currentPage = 0; } else { - this.focusedItemIndex++; - if (this.focusedItemIndex > this.pages[this.currentPage].end) { - this.clearPage(); - this.currentPage++; + 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) { + this.clearPage(); + this.currentPage++; + } } } @@ -397,9 +438,19 @@ FullMenuView.prototype.focusPrevious = function () { this.currentPage = this.pages.length - 1; } else { this.focusedItemIndex--; - if (this.focusedItemIndex < this.pages[this.currentPage].start) { - this.clearPage(); - this.currentPage--; + 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--; + } } } diff --git a/core/string_util.js b/core/string_util.js index 3f78b37d..066548b9 100644 --- a/core/string_util.js +++ b/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 left = padlen - right; + const right = Math.ceil(padLen / 2); + 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: diff --git a/core/vertical_menu_view.js b/core/vertical_menu_view.js index 3379ca17..bf3ac57b 100644 --- a/core/vertical_menu_view.js +++ b/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( - ' ' - ); - let seq = ansi.goto(this.position.row, this.position.col) + this.getSGR() + blank; - let row = this.position.row + 1; + const blank = ' '.repeat(Math.max(this.oldDimens.width, this.dimens.width)); + let row = this.position.row; 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);