From 7b63af1068fc77147419fb57ab2f7cd96248bdd6 Mon Sep 17 00:00:00 2001 From: Jakub <106778416+MarekZegare4@users.noreply.github.com> Date: Thu, 14 May 2026 08:06:26 +0200 Subject: [PATCH] refactor: use PopupMenu in keyboard and context menus, fix bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - KeyboardWidget: replace inline placeholder overlay with PopupMenu - UITask QuickMsgScreen: replace inline CONTACT_PICK/CHANNEL_PICK context menus with PopupMenu; notif label stored in _ctx_notif_item at open time - BotScreen: clear placeholder list for trigger field (expansion tokens would never match incoming literal text) - MsgExpand: fix APPEND macro off-by-one (oi+_l < out_len-1 → < out_len) Co-Authored-By: Claude Sonnet 4.6 --- examples/companion_radio/MsgExpand.h | 2 +- examples/companion_radio/ui-new/BotScreen.h | 5 +- .../companion_radio/ui-new/KeyboardWidget.h | 72 ++-------- examples/companion_radio/ui-new/UITask.cpp | 135 +++++------------- 4 files changed, 57 insertions(+), 157 deletions(-) diff --git a/examples/companion_radio/MsgExpand.h b/examples/companion_radio/MsgExpand.h index b149fbb3..24e9d91e 100644 --- a/examples/companion_radio/MsgExpand.h +++ b/examples/companion_radio/MsgExpand.h @@ -72,7 +72,7 @@ inline void expandMsg(const char* tmpl, char* out, int out_len, // helper macro: append a buffer whose length is already known #define APPEND(s, slen) do { \ int _l = (slen); \ - if (oi + _l < out_len - 1) { memcpy(out + oi, (s), _l); oi += _l; } \ + if (oi + _l < out_len) { memcpy(out + oi, (s), _l); oi += _l; } \ } while(0) if (strncmp(p, "{loc}", 5) == 0) { diff --git a/examples/companion_radio/ui-new/BotScreen.h b/examples/companion_radio/ui-new/BotScreen.h index 9edd2f57..49ab0585 100644 --- a/examples/companion_radio/ui-new/BotScreen.h +++ b/examples/companion_radio/ui-new/BotScreen.h @@ -175,8 +175,11 @@ public: max = sizeof(_prefs->bot_reply_ch) - 1; } _kb.begin(initial, max); - if (_sel != 2) // trigger field doesn't support sensor placeholders + if (_sel == 2) { + _kb._ph_count = 0; // trigger is literal text — placeholders never match incoming msgs + } else { kbAddSensorPlaceholders(_kb, &sensors); + } return true; } return false; diff --git a/examples/companion_radio/ui-new/KeyboardWidget.h b/examples/companion_radio/ui-new/KeyboardWidget.h index ab917551..8ea7334b 100644 --- a/examples/companion_radio/ui-new/KeyboardWidget.h +++ b/examples/companion_radio/ui-new/KeyboardWidget.h @@ -2,6 +2,7 @@ #include #include +#include "PopupMenu.h" // Layout constants shared by all keyboard users static const char KB_CHARS[4][10] = { @@ -31,11 +32,9 @@ struct KeyboardWidget { int max_len; int row, col; bool caps; - bool ph_mode; - int ph_sel; - int ph_scroll; char _ph_buf[KB_PH_MAX][KB_PH_LEN]; int _ph_count; + PopupMenu _ph_menu; enum Result { NONE, DONE, CANCELLED }; @@ -45,8 +44,8 @@ struct KeyboardWidget { buf[max_len] = '\0'; len = strlen(buf); row = col = 0; - caps = ph_mode = false; - ph_sel = ph_scroll = 0; + caps = false; + _ph_menu.active = false; // default placeholders — always available _ph_count = 0; addPlaceholder("{loc}"); @@ -114,70 +113,26 @@ struct KeyboardWidget { display.setColor(DisplayDriver::LIGHT); } - // placeholder picker overlay - if (ph_mode) { - int box_h = 12 + KB_PH_VISIBLE * 10; - display.setColor(DisplayDriver::DARK); - display.fillRect(20, 20, 88, box_h); - display.setColor(DisplayDriver::LIGHT); - display.drawRect(20, 20, 88, box_h); - display.setCursor(24, 21); - display.print("Placeholder:"); - display.fillRect(20, 30, 88, 1); - - if (ph_scroll > 0) { - display.setCursor(100, 21); - display.print("^"); - } - if (ph_scroll + KB_PH_VISIBLE < _ph_count) { - display.setCursor(100, 33 + (KB_PH_VISIBLE - 1) * 10); - display.print("v"); - } - - for (int i = 0; i < KB_PH_VISIBLE && (ph_scroll + i) < _ph_count; i++) { - int idx = ph_scroll + i; - int py = 33 + i * 10; - if (idx == ph_sel) { - display.setColor(DisplayDriver::LIGHT); - display.fillRect(21, py - 1, 86, 10); - display.setColor(DisplayDriver::DARK); - } else { - display.setColor(DisplayDriver::LIGHT); - } - display.setCursor(24, py); - display.print(_ph_buf[idx]); - } - display.setColor(DisplayDriver::LIGHT); - } + // placeholder picker overlay (drawn on top of keyboard) + if (_ph_menu.active) _ph_menu.render(display); return 50; } Result handleInput(char c) { // placeholder overlay consumes all input - if (ph_mode) { - if (c == KEY_UP && ph_sel > 0) { - ph_sel--; - if (ph_sel < ph_scroll) ph_scroll = ph_sel; - return NONE; - } - if (c == KEY_DOWN && ph_sel < _ph_count - 1) { - ph_sel++; - if (ph_sel >= ph_scroll + KB_PH_VISIBLE) ph_scroll = ph_sel - KB_PH_VISIBLE + 1; - return NONE; - } - if (c == KEY_ENTER) { - const char* ph = _ph_buf[ph_sel]; + if (_ph_menu.active) { + auto res = _ph_menu.handleInput(c); + if (res == PopupMenu::SELECTED) { + int idx = _ph_menu.selectedIndex(); + const char* ph = _ph_buf[idx]; int ph_len = strlen(ph); if (len + ph_len <= max_len) { memcpy(buf + len, ph, ph_len); len += ph_len; buf[len] = '\0'; } - ph_mode = false; - return NONE; } - ph_mode = false; // CANCEL or any other key closes the overlay return NONE; } @@ -226,9 +181,8 @@ struct KeyboardWidget { if (len > 0) buf[--len] = '\0'; break; case 3: - ph_mode = true; - ph_sel = 0; - ph_scroll = 0; + _ph_menu.begin("Placeholder:", KB_PH_VISIBLE); + for (int i = 0; i < _ph_count; i++) _ph_menu.addItem(_ph_buf[i]); break; case 4: return DONE; diff --git a/examples/companion_radio/ui-new/UITask.cpp b/examples/companion_radio/ui-new/UITask.cpp index a6324b9b..34527069 100644 --- a/examples/companion_radio/ui-new/UITask.cpp +++ b/examples/companion_radio/ui-new/UITask.cpp @@ -632,10 +632,10 @@ class QuickMsgScreen : public UIScreen { // KEYBOARD KeyboardWidget _kb; - // Context menu (opened by long-press ENTER in CHANNEL_PICK) - bool _ctx_open; - bool _ctx_dirty; - int _ctx_sel; + // Context menu (opened by KEY_CONTEXT_MENU in CHANNEL_PICK / CONTACT_PICK) + PopupMenu _ctx_menu; + bool _ctx_dirty; + char _ctx_notif_item[22]; struct ChHistEntry { uint8_t ch_idx; char text[140]; }; ChHistEntry _hist[CH_HIST_MAX]; @@ -896,7 +896,7 @@ public: _hist_head(0), _hist_count(0), _dm_hist_head(0), _dm_hist_count(0), _dm_hist_sel(-1), _dm_hist_scroll(0), - _ctx_open(false), _ctx_dirty(false), _ctx_sel(0) { + _ctx_dirty(false) { memset(_ch_unread, 0, sizeof(_ch_unread)); } @@ -958,9 +958,8 @@ public: buildContactList(); buildChannelList(); - _ctx_open = false; + _ctx_menu.active = false; _ctx_dirty = false; - _ctx_sel = 0; _unread_at_entry = 0; _viewing_max_seen = 0; } @@ -1048,38 +1047,8 @@ public: display.setColor(DisplayDriver::LIGHT); renderScrollHints(display, _contact_scroll, _num_contacts); - // Context menu overlay (long-press ENTER), DM mode only - if (_ctx_open && _num_contacts > 0 && !_room_mode) { - static const char* NOTIF_LABELS[] = { "default", "OFF", "ON" }; - ContactInfo ci; - the_mesh.getContactByIdx(_sorted[_contact_sel], ci); - uint8_t nstate = dmNotifState(ci.id.pub_key); - char notif_item[22]; - snprintf(notif_item, sizeof(notif_item), "Notif: %s", NOTIF_LABELS[nstate]); - const char* items[] = { "Mark as read", notif_item }; - const int CTX_COUNT = 2; - - display.setColor(DisplayDriver::DARK); - display.fillRect(15, 14, 98, 34); - display.setColor(DisplayDriver::LIGHT); - display.drawRect(15, 14, 98, 34); - display.setCursor(19, 15); - display.print("Contact options"); - display.fillRect(15, 24, 98, 1); - for (int i = 0; i < CTX_COUNT; i++) { - int py = 27 + i * 10; - if (i == _ctx_sel) { - display.setColor(DisplayDriver::LIGHT); - display.fillRect(16, py - 1, 96, 10); - display.setColor(DisplayDriver::DARK); - } else { - display.setColor(DisplayDriver::LIGHT); - } - display.setCursor(19, py); - display.print(items[i]); - } - display.setColor(DisplayDriver::LIGHT); - } + // Context menu overlay + if (_ctx_menu.active) _ctx_menu.render(display); } else if (_phase == CHANNEL_PICK) { display.drawTextCentered(display.width()/2, 0, "SELECT CHANNEL"); @@ -1123,37 +1092,8 @@ public: display.setColor(DisplayDriver::LIGHT); renderScrollHints(display, _channel_scroll, _num_channels); - // Context menu overlay (long-press ENTER) - if (_ctx_open && _num_channels > 0) { - static const char* NOTIF_LABELS[] = { "default", "OFF", "ON" }; - uint8_t ch_idx = _channel_indices[_channel_sel]; - uint8_t nstate = chNotifState(ch_idx); - char notif_item[22]; - snprintf(notif_item, sizeof(notif_item), "Notif: %s", NOTIF_LABELS[nstate]); - const char* items[] = { "Mark all read", notif_item }; - const int CTX_COUNT = 2; - - display.setColor(DisplayDriver::DARK); - display.fillRect(15, 14, 98, 34); - display.setColor(DisplayDriver::LIGHT); - display.drawRect(15, 14, 98, 34); - display.setCursor(19, 15); - display.print("Channel options"); - display.fillRect(15, 24, 98, 1); - for (int i = 0; i < CTX_COUNT; i++) { - int py = 27 + i * 10; - if (i == _ctx_sel) { - display.setColor(DisplayDriver::LIGHT); - display.fillRect(16, py - 1, 96, 10); - display.setColor(DisplayDriver::DARK); - } else { - display.setColor(DisplayDriver::LIGHT); - } - display.setCursor(19, py); - display.print(items[i]); - } - display.setColor(DisplayDriver::LIGHT); - } + // Context menu overlay + if (_ctx_menu.active) _ctx_menu.render(display); } else if (_phase == DM_HIST) { display.setTextSize(1); @@ -1408,18 +1348,12 @@ public: } else if (_phase == CONTACT_PICK) { // Context menu consumes all input while open - if (_ctx_open) { - if (c == KEY_CANCEL) { - if (_ctx_dirty) { the_mesh.savePrefs(); _ctx_dirty = false; } - _ctx_open = false; - return true; - } - if (c == KEY_UP && _ctx_sel > 0) { _ctx_sel--; return true; } - if (c == KEY_DOWN && _ctx_sel < 1) { _ctx_sel++; return true; } - if (c == KEY_ENTER && _num_contacts > 0) { + if (_ctx_menu.active) { + auto res = _ctx_menu.handleInput(c); + if (res == PopupMenu::SELECTED && _num_contacts > 0) { ContactInfo ci; if (the_mesh.getContactByIdx(_sorted[_contact_sel], ci)) { - if (_ctx_sel == 0) { + if (_ctx_menu.selectedIndex() == 0) { _task->clearDMUnread(ci.id.pub_key); } else { uint8_t nstate = dmNotifState(ci.id.pub_key); @@ -1427,7 +1361,9 @@ public: _ctx_dirty = true; } } - return true; + } + if (res != PopupMenu::NONE && _ctx_dirty) { + the_mesh.savePrefs(); _ctx_dirty = false; } return true; } @@ -1453,32 +1389,34 @@ public: return true; } if (c == KEY_CONTEXT_MENU && _num_contacts > 0 && !_room_mode) { - _ctx_sel = 0; + static const char* NOTIF_LABELS[] = { "default", "OFF", "ON" }; + ContactInfo ci; + the_mesh.getContactByIdx(_sorted[_contact_sel], ci); + snprintf(_ctx_notif_item, sizeof(_ctx_notif_item), "Notif: %s", + NOTIF_LABELS[dmNotifState(ci.id.pub_key)]); + _ctx_menu.begin("Contact options", 2); + _ctx_menu.addItem("Mark as read"); + _ctx_menu.addItem(_ctx_notif_item); _ctx_dirty = false; - _ctx_open = true; return true; } } else if (_phase == CHANNEL_PICK) { // Context menu consumes all input while open - if (_ctx_open) { - if (c == KEY_CANCEL) { - if (_ctx_dirty) { the_mesh.savePrefs(); _ctx_dirty = false; } - _ctx_open = false; - return true; - } - if (c == KEY_UP && _ctx_sel > 0) { _ctx_sel--; return true; } - if (c == KEY_DOWN && _ctx_sel < 1) { _ctx_sel++; return true; } - if (c == KEY_ENTER && _num_channels > 0) { + if (_ctx_menu.active) { + auto res = _ctx_menu.handleInput(c); + if (res == PopupMenu::SELECTED && _num_channels > 0) { uint8_t ch_idx = _channel_indices[_channel_sel]; - if (_ctx_sel == 0) { + if (_ctx_menu.selectedIndex() == 0) { _ch_unread[ch_idx] = 0; } else { uint8_t nstate = chNotifState(ch_idx); setChNotifState(ch_idx, (nstate + 1) % 3); _ctx_dirty = true; } - return true; + } + if (res != PopupMenu::NONE && _ctx_dirty) { + the_mesh.savePrefs(); _ctx_dirty = false; } return true; } @@ -1505,9 +1443,14 @@ public: return true; } if (c == KEY_CONTEXT_MENU && _num_channels > 0) { - _ctx_sel = 0; + uint8_t ch_idx = _channel_indices[_channel_sel]; + static const char* NOTIF_LABELS[] = { "default", "OFF", "ON" }; + snprintf(_ctx_notif_item, sizeof(_ctx_notif_item), "Notif: %s", + NOTIF_LABELS[chNotifState(ch_idx)]); + _ctx_menu.begin("Channel options", 2); + _ctx_menu.addItem("Mark all read"); + _ctx_menu.addItem(_ctx_notif_item); _ctx_dirty = false; - _ctx_open = true; return true; }