From d25fda5f322be243a67d8026696ef15dea6d409f Mon Sep 17 00:00:00 2001 From: Jakub <106778416+MarekZegare4@users.noreply.github.com> Date: Mon, 31 Aug 2026 20:09:59 +0200 Subject: [PATCH] refactor(ui): extract PopupMenu::beginConfirm() for destructive-action popups Found while re-reviewing this session's own commits: five screens each hand-built the same 2-row Action/Cancel confirm popup, defaulting the highlight to Cancel -- NearbyScreen's contact-delete, AdminScreen's OTA-start, and the three just added (Trail's reset, Messages' channel delete, RadioPresetPicker's preset delete). The plan that added those three had already flagged this exact duplication without acting on it, so it just tripled instead of getting fixed. One PopupMenu::beginConfirm(title, action_label, cancel_label="Cancel") replaces all five call sites, and makes "defaults to Cancel" a property of the popup itself rather than something each new confirm has to remember. Also drops two small redundancies spotted along the way: NearbyScreen's _confirm.active = true, dead since begin() already sets it, and RadioPresetPicker's deleting = false being set twice in a row (once inside openConfirm(), once again by its only caller). No behavior change; verified against the actual PopupMenu/menu-level state machines in each of the five call sites before touching them. --- examples/companion_radio/ui-new/AdminScreen.h | 5 +---- examples/companion_radio/ui-new/MessagesScreen.h | 5 +---- examples/companion_radio/ui-new/NearbyScreen.h | 6 +----- examples/companion_radio/ui-new/PopupMenu.h | 13 +++++++++++++ examples/companion_radio/ui-new/RadioPresetPicker.h | 6 +----- examples/companion_radio/ui-new/TrailScreen.h | 5 +---- 6 files changed, 18 insertions(+), 22 deletions(-) diff --git a/examples/companion_radio/ui-new/AdminScreen.h b/examples/companion_radio/ui-new/AdminScreen.h index 746b7189..f9be987c 100644 --- a/examples/companion_radio/ui-new/AdminScreen.h +++ b/examples/companion_radio/ui-new/AdminScreen.h @@ -238,10 +238,7 @@ class AdminScreen : public UIScreen { } else if (f.set_prefix == nullptr) { // Action if (!strcmp(f.get_cmd, "start ota")) { // see _confirm's comment _pending_confirm_field = &f; - _confirm.begin("Start OTA update?", 2); - _confirm.addItem("Start"); - _confirm.addItem("Cancel"); - _confirm.setSelected(1); // default highlight = Cancel + _confirm.beginConfirm("Start OTA update?", "Start"); return; } strncpy(_cmd_text, f.get_cmd, sizeof(_cmd_text) - 1); diff --git a/examples/companion_radio/ui-new/MessagesScreen.h b/examples/companion_radio/ui-new/MessagesScreen.h index dafba392..601cbc62 100644 --- a/examples/companion_radio/ui-new/MessagesScreen.h +++ b/examples/companion_radio/ui-new/MessagesScreen.h @@ -2091,10 +2091,7 @@ public: ChannelDetails ch; if (the_mesh.getChannel(ch_idx, ch)) _ch_view.openEdit(ch_idx, ch.name); } else if (sel == 6) { // Delete -- confirm first (destructive) - _ctx_menu.begin("Delete channel?", 2); - _ctx_menu.addItem("Delete"); - _ctx_menu.addItem("Cancel"); - _ctx_menu.setSelected(1); + _ctx_menu.beginConfirm("Delete channel?", "Delete"); _ch_delete_confirm_active = true; return true; // list rebuild below would close the submenu } diff --git a/examples/companion_radio/ui-new/NearbyScreen.h b/examples/companion_radio/ui-new/NearbyScreen.h index 20712a54..f256d733 100644 --- a/examples/companion_radio/ui-new/NearbyScreen.h +++ b/examples/companion_radio/ui-new/NearbyScreen.h @@ -468,11 +468,7 @@ class NearbyScreen : public UIScreen { void startDeleteConfirm() { const Entry* e = selected(); if (!e || !e->has_key || !entryIsContact(e)) return; - _confirm.begin("Delete contact?", 2); - _confirm.addItem("Delete"); - _confirm.addItem("Cancel"); - _confirm.setSelected(1); - _confirm.active = true; + _confirm.beginConfirm("Delete contact?", "Delete"); } void doDeleteSelected() { diff --git a/examples/companion_radio/ui-new/PopupMenu.h b/examples/companion_radio/ui-new/PopupMenu.h index 42ddc0cc..788762ba 100644 --- a/examples/companion_radio/ui-new/PopupMenu.h +++ b/examples/companion_radio/ui-new/PopupMenu.h @@ -49,6 +49,19 @@ struct PopupMenu { if (_count > i) _value_mask |= (1u << i); } + // A two-row Action/Cancel confirm for a destructive or hard-to-reverse + // action, defaulting the highlight to Cancel (row 1) so accepting it takes + // a deliberate move up. Same shape every such confirm in the UI uses -- + // see NearbyScreen's contact-delete confirm, AdminScreen's OTA-start + // confirm, Trail's reset confirm, Messages' channel-delete confirm, and + // RadioPresetPicker's preset-delete confirm. + void beginConfirm(const char* title, const char* action_label, const char* cancel_label = "Cancel") { + begin(title, 2); + addItem(action_label); + addItem(cancel_label); + setSelected(1); + } + int render(DisplayDriver& display) { // Everything is derived from the live font metrics so the box fits its // content on every display — including landscape e-ink, where the font (and diff --git a/examples/companion_radio/ui-new/RadioPresetPicker.h b/examples/companion_radio/ui-new/RadioPresetPicker.h index d13cc677..5d504b64 100644 --- a/examples/companion_radio/ui-new/RadioPresetPicker.h +++ b/examples/companion_radio/ui-new/RadioPresetPicker.h @@ -131,12 +131,8 @@ struct RadioPresetPicker { // is picked, onSelected() below is terminal -- the whole picker closes, // same as it already does after a built-in/user preset pick. void openConfirm(uint8_t slot) { - menu.begin("Delete preset?", 2); - menu.addItem("Delete"); - menu.addItem("Cancel"); - menu.setSelected(1); + menu.beginConfirm("Delete preset?", "Delete"); confirm_slot = slot; - deleting = false; } // Handle the index the popup reports as SELECTED. Mutates target fields on a diff --git a/examples/companion_radio/ui-new/TrailScreen.h b/examples/companion_radio/ui-new/TrailScreen.h index f3f16bf9..7b2c7c1f 100644 --- a/examples/companion_radio/ui-new/TrailScreen.h +++ b/examples/companion_radio/ui-new/TrailScreen.h @@ -405,10 +405,7 @@ private: void buildResetConfirmMenu() { _menu_level = ML_CONFIRM_RESET; _act_count = 0; - _action_menu.begin("Reset trail?", 2); - _action_menu.addItem("Reset"); - _action_menu.addItem("Cancel"); - _action_menu.setSelected(1); + _action_menu.beginConfirm("Reset trail?", "Reset"); } // Trail-file submenu — only the operations that make sense right now.