Skip to content

feat: add graphical selection support for input items #411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
forntoh merged 2 commits into from
Apr 25, 2026
Merged

feat: add graphical selection support for input items #411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9cf0214
feat: add graphical selection support for input items
forntoh Apr 24, 2026
1a1da7e
fix: align input selection clearing and preview intent
forntoh Apr 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion docs/source/overview/items/input-charset.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,14 @@ The charset input item can be created using the following syntax:
When the ``Pass`` menu item is selected, an input field will be displayed on the screen, allowing the user to enter a string value.
The input value will be restricted to the characters specified in the charset.

With graphical renderers that expose ``GraphicalValueSelectionRenderer``,
character preview and selection are rendered through the value-area highlight,
so charset cycling remains visible without relying on a text cursor blinker.

.. image:: images/item-charset-input.gif
:width: 400px
:alt: Example of a charset input menu item

You can create multiple charset input items in the same menu screen, each with its own label, default value, and charset.

For more information about the charset input item, check the :cpp:class:`API reference <ItemInputCharset>`.
For more information about the charset input item, check the :cpp:class:`API reference <ItemInputCharset>`.
12 changes: 12 additions & 0 deletions docs/source/overview/items/input.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,18 @@ You can create an input item by specifying the label and the default value:

When the ``Name`` menu item is selected, an input field will be displayed on the screen, allowing the user to enter a string value.

Graphical rendering
~~~~~~~~~~~~~~~~~~~

``ItemInput`` exposes optional graphical capabilities through
``GraphicalMenuItem`` and reports its value width for right-aligned graphical
layouts.

When a renderer exposes ``GraphicalValueSelectionRenderer`` via
``queryExtension()``, the input item highlights the active character instead of
using a blinking character cursor. Character-display renderers continue to use
the standard blinking cursor behavior.

.. image:: images/item-input.gif
:width: 400px
:alt: Example of an input menu item
Expand Down
154 changes: 140 additions & 14 deletions src/ItemInput.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,13 @@

#include "LcdMenu.h"
#include "MenuItem.h"
#include "display/GraphicalDisplayInterface.h"
#include "renderer/GraphicalMenuItem.h"
#include "renderer/GraphicalValueSelectionRenderer.h"
#include <utils/lcd_menu_utils.h>

#include <string.h>

/**
* @brief Item that allows user to input string information.
*
Expand All @@ -18,7 +23,7 @@
* Has internal `edit` state.
* Value area is scrollable, see `view`.
*/
class ItemInput : public MenuItem {
class ItemInput : public MenuItem, public GraphicalMenuItem {
protected:
/**
* @brief String value of item.
Expand Down Expand Up @@ -62,6 +67,13 @@ class ItemInput : public MenuItem {
*/
fptrStr callback;

inline GraphicalValueSelectionRenderer* getGraphicalValueSelectionRenderer(MenuRenderer* renderer) const {
if (renderer == NULL) {
return NULL;
}
return static_cast<GraphicalValueSelectionRenderer*>(renderer->queryExtension(GraphicalValueSelectionRenderer::extensionId()));
}

public:
/**
* Construct a new ItemInput object with an initial value.
Expand Down Expand Up @@ -105,6 +117,22 @@ class ItemInput : public MenuItem {
}
return false;
}

uint8_t measureGraphicalValueWidth(GraphicalDisplayInterface* display) const override {
return display == NULL ? 0 : display->getTextWidth(value);
}

bool useTightGraphicalSelectionBox() const override {
return true;
}

const void* queryCapability(uint8_t capabilityId) const override {
if (capabilityId == GraphicalMenuItem::capabilityId()) {
return static_cast<const GraphicalMenuItem*>(this);
}
return MenuItem::queryCapability(capabilityId);
}

/**
* Get the callback function for this item.
*
Expand All @@ -114,9 +142,48 @@ class ItemInput : public MenuItem {

protected:
void draw(MenuRenderer* renderer) override {
GraphicalValueSelectionRenderer* selectionRenderer = getGraphicalValueSelectionRenderer(renderer);
if (selectionRenderer != NULL) {
char* graphicalValue = value;
char* insertionBuffer = NULL;
bool editing = MenuItem::isEditing();

if (editing) {
renderer->viewShift = 0;
uint8_t len = strlen(value);
uint8_t selectionStart = cursor > len ? len : cursor;
uint8_t selectionLength = 1;

if (selectionStart >= len) {
insertionBuffer = new char[len + 2];
memcpy(insertionBuffer, value, len);
insertionBuffer[len] = ' ';
insertionBuffer[len + 1] = '\0';
graphicalValue = insertionBuffer;
}

selectionRenderer->setValueSelection(selectionStart, selectionLength);
} else {
selectionRenderer->clearValueSelection();
}

renderer->drawItem(text, graphicalValue);
if (editing) {
selectionRenderer->clearValueSelection();
}

if (insertionBuffer != NULL) {
delete[] insertionBuffer;
}

return;
}
Comment on lines 144 to +180

@coderabbitai coderabbitai Bot Apr 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Second clearValueSelection() after drawItem is redundant in the non-editing branch.

When isEditing() is false, the code already calls selectionRenderer->clearValueSelection() on line 166 before drawItem, and then unconditionally calls it again on line 170 right after. It's harmless, but on embedded targets with a more expensive implementation of clearValueSelection it would be wasted work.

♻️ Optional cleanup 
-            renderer->drawItem(text, graphicalValue);
-            selectionRenderer->clearValueSelection();
+            renderer->drawItem(text, graphicalValue);
+            if (MenuItem::isEditing()) {
+                selectionRenderer->clearValueSelection();
+            }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/ItemInput.h` around lines 144 - 177, The second unconditional call to
selectionRenderer->clearValueSelection() after renderer->drawItem() is redundant
for the non-editing case and removes the only clear for the editing case if you
simply delete it; instead, make clearValueSelection() conditional: remove the
unconditional call and ensure clearValueSelection() is invoked in the editing
path after drawItem (keep the existing clear before drawItem in the non-editing
branch), so that setValueSelection()/drawItem/clearValueSelection() sequence
happens when MenuItem::isEditing() is true and clearValueSelection()/drawItem()
happens when false; update the block using selectionRenderer, drawItem,
isEditing(), insertionBuffer and value accordingly.


const uint8_t viewSize = getViewSize(renderer);
char* vbuf = new char[viewSize + 1];
substring(value, view, viewSize, vbuf);
if (viewSize > 0) {
substring(value, view, viewSize, vbuf);
}
vbuf[viewSize] = '\0';
renderer->drawItem(text, vbuf);
delete[] vbuf;
Expand Down Expand Up @@ -165,31 +232,54 @@ class ItemInput : public MenuItem {
}
}
void enter(MenuRenderer* renderer) {
// Move cursor to the latest index
bool graphicalSelection = getGraphicalValueSelectionRenderer(renderer) != NULL;

// Move cursor to the latest editable index
uint8_t length = strlen(value);
cursor = length;
// Move view if needed
uint8_t viewSize = getViewSize(renderer);
if (cursor > viewSize) {
view = length - (viewSize - 1);
if (graphicalSelection && length > 0) {
cursor = length - 1;
} else {
cursor = length;
}

if (graphicalSelection) {
view = 0;
renderer->viewShift = 0;
} else {
// Move view if needed
uint8_t viewSize = getViewSize(renderer);
if (viewSize == 0) {
viewSize = 1;
}
if (cursor > viewSize) {
view = length - (viewSize - 1);
}
}

// Redraw
MenuItem::beginEdit();
draw(renderer);
renderer->drawBlinker();
if (!graphicalSelection) {
renderer->drawBlinker();
}
// Log
LOG(F("ItemInput::enterEditMode"), value);
};
Comment thread
forntoh marked this conversation as resolved.
void back(MenuRenderer* renderer) {
renderer->clearBlinker();
MenuItem::endEdit();

renderer->viewShift = 0;

if (callback != NULL) {
callback(value);
}

// Move view to 0 and redraw before exit
cursor = 0;
view = 0;
draw(renderer);
if (callback != NULL) {
callback(value);
}

// Log
LOG(F("ItemInput::exitEditMode"), value);
};
Comment thread
forntoh marked this conversation as resolved.
Expand All @@ -198,8 +288,15 @@ class ItemInput : public MenuItem {
return;
}
cursor--;

if (getGraphicalValueSelectionRenderer(renderer) != NULL) {
draw(renderer);
LOG(F("ItemInput::left"), value);
return;
}

uint8_t cursorCol = renderer->getCursorCol();
if (cursor <= view - 1) {
if (view > 0 && cursor < view) {
view--;
draw(renderer);
} else {
Expand All @@ -218,7 +315,17 @@ class ItemInput : public MenuItem {
return;
}
cursor++;

if (getGraphicalValueSelectionRenderer(renderer) != NULL) {
draw(renderer);
LOG(F("ItemInput::right"), value);
return;
}

uint8_t viewSize = getViewSize(renderer);
if (viewSize == 0) {
viewSize = 1;
}
uint8_t cursorCol = renderer->getCursorCol();
if (cursor > (view + viewSize - 1)) {
view++;
Expand All @@ -240,6 +347,13 @@ class ItemInput : public MenuItem {
}
remove(value, cursor - 1, 1);
cursor--;

if (getGraphicalValueSelectionRenderer(renderer) != NULL) {
draw(renderer);
LOG(F("ItemInput::backspace"), value);
return;
}

uint8_t cursorCol = renderer->getCursorCol();
if (view > 0) {
view--;
Expand Down Expand Up @@ -279,7 +393,17 @@ class ItemInput : public MenuItem {
delete[] value;
value = buf;
cursor++;

if (getGraphicalValueSelectionRenderer(renderer) != NULL) {
draw(renderer);
LOG(F("ItemInput::typeChar"), character);
return;
}

uint8_t viewSize = getViewSize(renderer);
if (viewSize == 0) {
viewSize = 1;
}
if (cursor > (view + viewSize - 1)) {
view++;
}
Expand All @@ -296,7 +420,9 @@ class ItemInput : public MenuItem {
cursor = 0;
view = 0;
draw(renderer);
renderer->drawBlinker();
if (getGraphicalValueSelectionRenderer(renderer) == NULL) {
renderer->drawBlinker();
}
// Log
LOG(F("ItemInput::clear"), value);
}
Expand Down
38 changes: 38 additions & 0 deletions src/ItemInputCharset.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#include "LcdMenu.h"
#include <utils/lcd_menu_utils.h>

#include <string.h>

class ItemInputCharset : public ItemInput {
private:
const char* charset;
Expand Down Expand Up @@ -117,6 +119,14 @@ class ItemInputCharset : public ItemInput {
*/
void abortCharEdit(MenuRenderer* renderer) {
charEdit = false;

if (getGraphicalValueSelectionRenderer(renderer) != NULL) {
renderer->viewShift = 0;
ItemInput::draw(renderer);
LOG(F("ItemInputCharset::abortCharEdit"));
return;
}
Comment thread
forntoh marked this conversation as resolved.

uint8_t cursorCol = renderer->getCursorCol();
if (cursor < strlen(value)) {
renderer->draw(value[cursor]);
Expand Down Expand Up @@ -167,6 +177,34 @@ class ItemInputCharset : public ItemInput {
}

void drawChar(MenuRenderer* renderer) {
if (getGraphicalValueSelectionRenderer(renderer) != NULL) {
renderer->viewShift = 0;
uint8_t length = strlen(value);

// Update in place when cursor points to an existing character; when cursor
// is at the insertion position, render through a temporary preview buffer
// to avoid writing past the current string bounds.
if (cursor < length) {
char original = value[cursor];
value[cursor] = charset[charsetPosition];
ItemInput::draw(renderer);
value[cursor] = original;
} else {
char* preview = new char[length + 2];
memcpy(preview, value, length);
preview[length] = charset[charsetPosition];
preview[length + 1] = '\0';

char* originalValue = value;
value = preview;
ItemInput::draw(renderer);
value = originalValue;

delete[] preview;
}
return;
}
Comment on lines 179 to +206

@coderabbitai coderabbitai Bot Apr 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider unifying the two graphical preview paths.

The cursor < length branch mutates value[cursor] in-place and restores it, while the cursor >= length branch allocates a full preview buffer and temporarily swaps the value member pointer. The behaviors are equivalent, but the two styles make the method harder to reason about (and the pointer-swap approach would work for both cases, at the cost of one allocation per draw on char cycling).

If you want to keep zero-allocation behavior for the common in-range case, that is fine, but a brief comment explaining why the two branches differ would aid readers.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/ItemInputCharset.h` around lines 179 - 203, The two branches in drawChar
(function drawChar, fields value, charset, charsetPosition, cursor, and call
ItemInput::draw) implement the same visual preview with different techniques
(in-place byte swap when cursor < length vs. pointer-swap to an allocated
preview when cursor >= length), which is confusing; either refactor to a single
approach (e.g., use a small stack buffer or always create a local preview buffer
and point value at it) or, if keeping the zero-allocation in-place path for
performance, add a short clarifying comment above the if/else explaining why the
in-place mutation is used for cursor < length and why a temporary preview buffer
is needed for cursor >= length to avoid out-of-bounds writes.


renderer->moveCursor(renderer->getCursorCol(), renderer->getCursorRow());
renderer->draw(charset[charsetPosition]);
renderer->moveCursor(renderer->getCursorCol(), renderer->getCursorRow());
Expand Down
Loading
Loading