feat(core): encapsulate select menu item in a struct
What changed, and why it matters
This commit is a straightforward internal code cleanup in the Trezor firmware's user-interface code. It replaces a raw two-value pair (text label + intent) with a named struct called SelectMenuItem. There is no change to user-visible behavior, no bug fix, and no security-related change.
No security action required. Treat as normal refactoring/technical-debt cleanup.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors the select_menu API across multiple UI layouts (bolt, caesar, delizia, eckhart) and the firmware Micropython binding. It introduces a SelectMenuItem struct containing text: TString<’static> and intent: MenuItemIntent, and updates all call sites to construct and destructure this struct. The change is purely structural encapsulation; logic, capacity limits, intent handling, and rendering remain identical.
Changed components
core/embed/rust/src/ui/ui_firmware.rscore/embed/rust/src/ui/api/firmware_micropython.rscore/embed/rust/src/ui/layout_bolt/component/select_menu.rscore/embed/rust/src/ui/layout_bolt/ui_firmware.rscore/embed/rust/src/ui/layout_caesar/ui_firmware.rscore/embed/rust/src/ui/layout_delizia/ui_firmware.rscore/embed/rust/src/ui/layout_eckhart/ui_firmware.rsInspect captured patch +51 / −34
### core/embed/rust/src/ui/api/firmware_micropython.rs
@@ -20,14 +20,14 @@ use crate::ui::component::Empty;
use crate::ui::display::{fade_backlight_duration, get_backlight, set_backlight};
use crate::ui::layout::base::LAYOUT_STATE;
use crate::ui::layout::device_menu_result::DEVICE_MENU_RESULT;
-use crate::ui::layout::menu_item_intent::{MenuItemIntent, MENU_ITEM_INTENT_OBJ};
+use crate::ui::layout::menu_item_intent::MENU_ITEM_INTENT_OBJ;
use crate::ui::layout::obj::{ComponentMsgObj, LayoutObj, ATTACH_TYPE_OBJ};
use crate::ui::layout::result::{BACK, CANCELLED, CONFIRMED, INFO};
use crate::ui::layout::util::{upy_disable_animation, RecoveryType};
use crate::ui::notification::{Notification, NotificationLevel, NOTIFICATION_LEVEL_OBJ};
use crate::ui::ui_firmware::{
- FirmwareUI, MAX_CHECKLIST_ITEMS, MAX_GROUP_SHARE_LINES, MAX_MENU_ITEMS, MAX_PAIRED_DEVICES,
- MAX_WORD_QUIZ_ITEMS,
+ FirmwareUI, SelectMenuItem, MAX_CHECKLIST_ITEMS, MAX_GROUP_SHARE_LINES, MAX_MENU_ITEMS,
+ MAX_PAIRED_DEVICES, MAX_WORD_QUIZ_ITEMS,
};
use crate::ui::ModelUI;
@@ -741,11 +741,11 @@ extern "C" fn new_request_string(n_args: usize, args: *const Obj, kwargs: *mut M
extern "C" fn new_select_menu(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj {
let block = move |_args: &[Obj], kwargs: &Map| {
let items_iterable: Obj = kwargs.get(Qstr::MP_QSTR_items)?;
- let mut items = Vec::<(TString, MenuItemIntent), MAX_MENU_ITEMS>::new();
+ let mut items = Vec::<SelectMenuItem, MAX_MENU_ITEMS>::new();
for item in IterBuf::new().try_iterate(items_iterable)? {
let [text, intent]: [Obj; 2] = util::iter_into_array(item)?;
items
- .push((text.try_into()?, intent.try_into()?))
+ .push(SelectMenuItem::new(text.try_into()?, intent.try_into()?))
.map_err(|_| Error::OutOfRange)?;
}
let current = kwargs.get(Qstr::MP_QSTR_current)?.try_into()?;
### core/embed/rust/src/ui/layout_bolt/component/select_menu.rs
@@ -7,7 +7,7 @@ use crate::ui::component::{Component, Event, EventCtx};
use crate::ui::geometry::{Insets, Rect};
use crate::ui::layout::menu_item_intent::MenuItemIntent;
use crate::ui::shape::Renderer;
-use crate::ui::ui_firmware::MAX_MENU_ITEMS;
+use crate::ui::ui_firmware::{SelectMenuItem, MAX_MENU_ITEMS};
/// Maximum number of buttons shown on the screen at once.
/// TODO: pagination for menus with more items.
@@ -29,16 +29,14 @@ pub struct SelectMenu {
}
impl SelectMenu {
- pub fn new(
- items: Vec<(TString<'static>, MenuItemIntent), MAX_MENU_ITEMS>,
- ) -> Result<Self, Error> {
+ pub fn new(items: Vec<SelectMenuItem, MAX_MENU_ITEMS>) -> Result<Self, Error> {
if items.len() > MAX_VISIBLE_BUTTONS {
return Err(Error::NotImplementedError);
}
let choice_buttons = items
.into_iter()
- .map(|(text, intent)| {
- Button::with_text(text).styled(match intent {
+ .map(|item| {
+ Button::with_text(item.text).styled(match item.intent {
MenuItemIntent::Danger => theme::button_cancel(),
MenuItemIntent::Standard => theme::button_default(),
})
### core/embed/rust/src/ui/layout_bolt/ui_firmware.rs
@@ -33,8 +33,8 @@ use crate::ui::layout::obj::{LayoutMaybeTrace, LayoutObj, RootComponent};
use crate::ui::layout::util::{ConfirmValueParams, PropsList, RecoveryType};
use crate::ui::notification::Notification;
use crate::ui::ui_firmware::{
- FirmwareUI, MAX_CHECKLIST_ITEMS, MAX_GROUP_SHARE_LINES, MAX_MENU_ITEMS, MAX_PAIRED_DEVICES,
- MAX_WORD_QUIZ_ITEMS,
+ FirmwareUI, SelectMenuItem, MAX_CHECKLIST_ITEMS, MAX_GROUP_SHARE_LINES, MAX_MENU_ITEMS,
+ MAX_PAIRED_DEVICES, MAX_WORD_QUIZ_ITEMS,
};
use crate::ui::{geometry, ModelUI};
@@ -720,7 +720,7 @@ impl FirmwareUI for UIBolt {
}
fn select_menu(
- items: heapless::Vec<(TString<'static>, MenuItemIntent), MAX_MENU_ITEMS>,
+ items: heapless::Vec<SelectMenuItem, MAX_MENU_ITEMS>,
_current: usize,
) -> Result<impl LayoutMaybeTrace, Error> {
let layout = RootComponent::new(SelectMenu::new(items)?);
### core/embed/rust/src/ui/layout_caesar/ui_firmware.rs
@@ -33,8 +33,8 @@ use crate::ui::layout::obj::{LayoutMaybeTrace, LayoutObj, RootComponent};
use crate::ui::layout::util::{ConfirmValueParams, PropsList, RecoveryType};
use crate::ui::notification::Notification;
use crate::ui::ui_firmware::{
- FirmwareUI, MAX_CHECKLIST_ITEMS, MAX_GROUP_SHARE_LINES, MAX_MENU_ITEMS, MAX_PAIRED_DEVICES,
- MAX_WORD_QUIZ_ITEMS,
+ FirmwareUI, SelectMenuItem, MAX_CHECKLIST_ITEMS, MAX_GROUP_SHARE_LINES, MAX_MENU_ITEMS,
+ MAX_PAIRED_DEVICES, MAX_WORD_QUIZ_ITEMS,
};
use crate::ui::{geometry, ModelUI};
@@ -913,12 +913,12 @@ impl FirmwareUI for UICaesar {
}
fn select_menu(
- items: heapless::Vec<(TString<'static>, MenuItemIntent), MAX_MENU_ITEMS>,
+ items: heapless::Vec<SelectMenuItem, MAX_MENU_ITEMS>,
current: usize,
) -> Result<impl LayoutMaybeTrace, Error> {
// the entry's intent is not rendered on this model
let labels: heapless::Vec<TString<'static>, MAX_MENU_ITEMS> =
- items.into_iter().map(|(text, _intent)| text).collect();
+ items.into_iter().map(|item| item.text).collect();
// Returning the index of the selected menu item
let layout = RootComponent::new(
SimpleChoice::new(
### core/embed/rust/src/ui/layout_delizia/ui_firmware.rs
@@ -40,8 +40,8 @@ use crate::ui::layout::obj::{LayoutMaybeTrace, LayoutObj, RootComponent};
use crate::ui::layout::util::{ContentType, PropsList, RecoveryType};
use crate::ui::notification::Notification;
use crate::ui::ui_firmware::{
- FirmwareUI, MAX_CHECKLIST_ITEMS, MAX_GROUP_SHARE_LINES, MAX_MENU_ITEMS, MAX_PAIRED_DEVICES,
- MAX_WORD_QUIZ_ITEMS,
+ FirmwareUI, SelectMenuItem, MAX_CHECKLIST_ITEMS, MAX_GROUP_SHARE_LINES, MAX_MENU_ITEMS,
+ MAX_PAIRED_DEVICES, MAX_WORD_QUIZ_ITEMS,
};
use crate::ui::ModelUI;
@@ -719,17 +719,17 @@ impl FirmwareUI for UIDelizia {
}
fn select_menu(
- items: heapless::Vec<(TString<'static>, MenuItemIntent), MAX_MENU_ITEMS>,
+ items: heapless::Vec<SelectMenuItem, MAX_MENU_ITEMS>,
current: usize,
) -> Result<impl LayoutMaybeTrace, Error> {
let mut menu_items = VerticalMenuItems::new();
- for (text, intent) in items {
- unwrap!(menu_items.push(match intent {
+ for item in items {
+ unwrap!(menu_items.push(match item.intent {
// TODO: mapping `Danger` onto the `Cancel` variant is temporary -
// the variant is named after what the entry did, not how it looks.
// `VerticalMenuItem` should carry the intent instead.
- MenuItemIntent::Danger => VerticalMenuItem::Cancel(text),
- MenuItemIntent::Standard => VerticalMenuItem::Item(text),
+ MenuItemIntent::Danger => VerticalMenuItem::Cancel(item.text),
+ MenuItemIntent::Standard => VerticalMenuItem::Item(item.text),
}));
}
let menu = ScrolledVerticalMenu::new(menu_items, current);
### core/embed/rust/src/ui/layout_eckhart/ui_firmware.rs
@@ -42,8 +42,8 @@ use crate::ui::layout::util::{
};
use crate::ui::notification::Notification;
use crate::ui::ui_firmware::{
- FirmwareUI, MAX_CHECKLIST_ITEMS, MAX_GROUP_SHARE_LINES, MAX_MENU_ITEMS, MAX_PAIRED_DEVICES,
- MAX_WORD_QUIZ_ITEMS,
+ FirmwareUI, SelectMenuItem, MAX_CHECKLIST_ITEMS, MAX_GROUP_SHARE_LINES, MAX_MENU_ITEMS,
+ MAX_PAIRED_DEVICES, MAX_WORD_QUIZ_ITEMS,
};
use crate::ui::ModelUI;
use crate::util::interpolate;
@@ -879,18 +879,20 @@ impl FirmwareUI for UIEckhart {
}
fn select_menu(
- items: heapless::Vec<(TString<'static>, MenuItemIntent), MAX_MENU_ITEMS>,
+ items: heapless::Vec<SelectMenuItem, MAX_MENU_ITEMS>,
_current: usize,
) -> Result<impl LayoutMaybeTrace, Error> {
let mut menu = VerticalMenu::<ShortMenuVec>::empty();
- for (text, intent) in &items {
- menu.item(match intent {
+ for item in &items {
+ menu.item(match item.intent {
// TODO: reusing the cancel button to render a dangerous entry is
// temporary - `Danger` describes how the entry looks, while
// `new_cancel_menu_item` names what it used to do. The styling
// should be lifted out of the cancel-specific constructor.
- MenuItemIntent::Danger => Button::new_cancel_menu_item(*text),
- MenuItemIntent::Standard => Button::new_menu_item(*text, theme::menu_item_title()),
+ MenuItemIntent::Danger => Button::new_cancel_menu_item(item.text),
+ MenuItemIntent::Standard => {
+ Button::new_menu_item(item.text, theme::menu_item_title())
+ }
});
}
let screen = VerticalMenuScreen::new(menu)
### core/embed/rust/src/ui/ui_firmware.rs
@@ -19,6 +19,24 @@ pub const MAX_MENU_ITEMS: usize = 6;
pub const MAX_PAIRED_DEVICES: usize = 8; // Maximum number of paired devices in the device menu
+/// One entry of `select_menu()`: its label plus what the entry means.
+///
+/// TODO: named after `select_menu` only to avoid colliding with the existing
+/// `MenuItem` types in `layout_eckhart` (device menu) and `layout_caesar`
+/// (passphrase keyboard), both of which are private and file-local. Once those
+/// are renamed to something model-specific, this should take the generic
+/// `MenuItem` name, since nothing about it is specific to `select_menu`.
+pub struct SelectMenuItem {
+ pub text: TString<'static>,
+ pub intent: MenuItemIntent,
+}
+
+impl SelectMenuItem {
+ pub fn new(text: TString<'static>, intent: MenuItemIntent) -> Self {
+ Self { text, intent }
+ }
+}
+
pub trait FirmwareUI {
#[allow(clippy::too_many_arguments)]
fn confirm_action(
@@ -277,9 +295,8 @@ pub trait FirmwareUI {
prefill: Option<TString<'static>>,
) -> Result<impl LayoutMaybeTrace, Error>;
- /// Each item is its label plus what the entry means.
fn select_menu(
- items: heapless::Vec<(TString<'static>, MenuItemIntent), MAX_MENU_ITEMS>,
+ items: heapless::Vec<SelectMenuItem, MAX_MENU_ITEMS>,
current: usize,
) -> Result<impl LayoutMaybeTrace, Error>;
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.