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.
This commit is contained in:
Jakub
2026-08-31 20:09:59 +02:00
parent 691e2ab506
commit d25fda5f32
6 changed files with 18 additions and 22 deletions
@@ -238,10 +238,7 @@ class AdminScreen : public UIScreen {
} else if (f.set_prefix == nullptr) { // Action } else if (f.set_prefix == nullptr) { // Action
if (!strcmp(f.get_cmd, "start ota")) { // see _confirm's comment if (!strcmp(f.get_cmd, "start ota")) { // see _confirm's comment
_pending_confirm_field = &f; _pending_confirm_field = &f;
_confirm.begin("Start OTA update?", 2); _confirm.beginConfirm("Start OTA update?", "Start");
_confirm.addItem("Start");
_confirm.addItem("Cancel");
_confirm.setSelected(1); // default highlight = Cancel
return; return;
} }
strncpy(_cmd_text, f.get_cmd, sizeof(_cmd_text) - 1); strncpy(_cmd_text, f.get_cmd, sizeof(_cmd_text) - 1);
@@ -2091,10 +2091,7 @@ public:
ChannelDetails ch; ChannelDetails ch;
if (the_mesh.getChannel(ch_idx, ch)) _ch_view.openEdit(ch_idx, ch.name); if (the_mesh.getChannel(ch_idx, ch)) _ch_view.openEdit(ch_idx, ch.name);
} else if (sel == 6) { // Delete -- confirm first (destructive) } else if (sel == 6) { // Delete -- confirm first (destructive)
_ctx_menu.begin("Delete channel?", 2); _ctx_menu.beginConfirm("Delete channel?", "Delete");
_ctx_menu.addItem("Delete");
_ctx_menu.addItem("Cancel");
_ctx_menu.setSelected(1);
_ch_delete_confirm_active = true; _ch_delete_confirm_active = true;
return true; // list rebuild below would close the submenu return true; // list rebuild below would close the submenu
} }
@@ -468,11 +468,7 @@ class NearbyScreen : public UIScreen {
void startDeleteConfirm() { void startDeleteConfirm() {
const Entry* e = selected(); const Entry* e = selected();
if (!e || !e->has_key || !entryIsContact(e)) return; if (!e || !e->has_key || !entryIsContact(e)) return;
_confirm.begin("Delete contact?", 2); _confirm.beginConfirm("Delete contact?", "Delete");
_confirm.addItem("Delete");
_confirm.addItem("Cancel");
_confirm.setSelected(1);
_confirm.active = true;
} }
void doDeleteSelected() { void doDeleteSelected() {
@@ -49,6 +49,19 @@ struct PopupMenu {
if (_count > i) _value_mask |= (1u << i); 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) { int render(DisplayDriver& display) {
// Everything is derived from the live font metrics so the box fits its // 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 // content on every display — including landscape e-ink, where the font (and
@@ -131,12 +131,8 @@ struct RadioPresetPicker {
// is picked, onSelected() below is terminal -- the whole picker closes, // is picked, onSelected() below is terminal -- the whole picker closes,
// same as it already does after a built-in/user preset pick. // same as it already does after a built-in/user preset pick.
void openConfirm(uint8_t slot) { void openConfirm(uint8_t slot) {
menu.begin("Delete preset?", 2); menu.beginConfirm("Delete preset?", "Delete");
menu.addItem("Delete");
menu.addItem("Cancel");
menu.setSelected(1);
confirm_slot = slot; confirm_slot = slot;
deleting = false;
} }
// Handle the index the popup reports as SELECTED. Mutates target fields on a // Handle the index the popup reports as SELECTED. Mutates target fields on a
@@ -405,10 +405,7 @@ private:
void buildResetConfirmMenu() { void buildResetConfirmMenu() {
_menu_level = ML_CONFIRM_RESET; _menu_level = ML_CONFIRM_RESET;
_act_count = 0; _act_count = 0;
_action_menu.begin("Reset trail?", 2); _action_menu.beginConfirm("Reset trail?", "Reset");
_action_menu.addItem("Reset");
_action_menu.addItem("Cancel");
_action_menu.setSelected(1);
} }
// Trail-file submenu — only the operations that make sense right now. // Trail-file submenu — only the operations that make sense right now.