From 566f6fe7cbdbc3b23c872de05b6314d89e931f1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ioan=20Biz=C4=83u?= Date: Mon, 7 Jul 2025 11:41:02 +0200 Subject: [PATCH] refactor(delizia): use external menu on `confirm_action` [no changelog] --- .../rust/src/ui/api/firmware_micropython.rs | 3 ++ .../rust/src/ui/layout_bolt/ui_firmware.rs | 2 + .../rust/src/ui/layout_caesar/ui_firmware.rs | 1 + .../ui/layout_delizia/flow/confirm_action.rs | 27 ++++++++-- .../rust/src/ui/layout_delizia/ui_firmware.rs | 4 +- .../rust/src/ui/layout_eckhart/ui_firmware.rs | 1 + core/embed/rust/src/ui/ui_firmware.rs | 1 + core/mocks/generated/trezorui_api.pyi | 1 + .../src/trezor/ui/layouts/delizia/__init__.py | 54 +++++++++++++------ 9 files changed, 72 insertions(+), 22 deletions(-) diff --git a/core/embed/rust/src/ui/api/firmware_micropython.rs b/core/embed/rust/src/ui/api/firmware_micropython.rs index 727df97b46..2309bf4885 100644 --- a/core/embed/rust/src/ui/api/firmware_micropython.rs +++ b/core/embed/rust/src/ui/api/firmware_micropython.rs @@ -71,6 +71,7 @@ extern "C" fn new_confirm_action(n_args: usize, args: *const Obj, kwargs: *mut M .get(Qstr::MP_QSTR_prompt_title) .unwrap_or_else(|_| Obj::const_none()) .try_into_option()?; + let external_menu: bool = kwargs.get_or(Qstr::MP_QSTR_external_menu, false)?; let layout = ModelUI::confirm_action( title, @@ -84,6 +85,7 @@ extern "C" fn new_confirm_action(n_args: usize, args: *const Obj, kwargs: *mut M reverse, prompt_screen, prompt_title, + external_menu, )?; Ok(LayoutObj::new_root(layout)?.into()) }; @@ -1351,6 +1353,7 @@ pub static mp_module_trezorui_api: Module = obj_module! { /// reverse: bool = False, /// prompt_screen: bool = False, /// prompt_title: str | None = None, + /// external_menu: bool = False, /// ) -> LayoutObj[UiResult]: /// """Confirm action.""" Qstr::MP_QSTR_confirm_action => obj_fn_kw!(0, new_confirm_action).as_obj(), diff --git a/core/embed/rust/src/ui/layout_bolt/ui_firmware.rs b/core/embed/rust/src/ui/layout_bolt/ui_firmware.rs index 0528f50214..52369d6ca0 100644 --- a/core/embed/rust/src/ui/layout_bolt/ui_firmware.rs +++ b/core/embed/rust/src/ui/layout_bolt/ui_firmware.rs @@ -57,6 +57,7 @@ impl FirmwareUI for UIBolt { reverse: bool, _prompt_screen: bool, _prompt_title: Option>, + _external_menu: bool, // TODO: will eventually replace the internal menu ) -> Result { let paragraphs = { let action = action.unwrap_or("".into()); @@ -946,6 +947,7 @@ impl FirmwareUI for UIBolt { false, false, None, + false, ) } diff --git a/core/embed/rust/src/ui/layout_caesar/ui_firmware.rs b/core/embed/rust/src/ui/layout_caesar/ui_firmware.rs index c5aec1d2ef..98a2dd38f2 100644 --- a/core/embed/rust/src/ui/layout_caesar/ui_firmware.rs +++ b/core/embed/rust/src/ui/layout_caesar/ui_firmware.rs @@ -58,6 +58,7 @@ impl FirmwareUI for UICaesar { reverse: bool, _prompt_screen: bool, _prompt_title: Option>, + _external_menu: bool, // TODO: will eventually replace the internal menu ) -> Result { let paragraphs = { let action = action.unwrap_or("".into()); diff --git a/core/embed/rust/src/ui/layout_delizia/flow/confirm_action.rs b/core/embed/rust/src/ui/layout_delizia/flow/confirm_action.rs index f8979f7a18..b580c20541 100644 --- a/core/embed/rust/src/ui/layout_delizia/flow/confirm_action.rs +++ b/core/embed/rust/src/ui/layout_delizia/flow/confirm_action.rs @@ -30,6 +30,8 @@ const MENU_ITEM_INFO: usize = 1; // Extra button at the top-right corner of the Action screen #[derive(PartialEq)] pub enum ConfirmActionExtra { + // Shows a menu button that simply returns INFO so it can be handled externally + ExternalMenu, // Opens a menu which can (optionally) lead to an extra Info screen, or cancel the action Menu(ConfirmActionMenuStrings), // Shows a cancel button directly @@ -114,6 +116,7 @@ impl FlowController for ConfirmAction { fn handle_event(&'static self, msg: FlowMsg) -> Decision { match (self, msg) { (Self::Action, FlowMsg::Cancelled) => self.return_msg(FlowMsg::Cancelled), + (Self::Action, FlowMsg::Info) => self.return_msg(FlowMsg::Info), _ => self.do_nothing(), } } @@ -143,6 +146,7 @@ impl FlowController for ConfirmActionWithConfirmation { fn handle_event(&'static self, msg: FlowMsg) -> Decision { match (self, msg) { (Self::Action, FlowMsg::Cancelled) => self.return_msg(FlowMsg::Cancelled), + (Self::Action, FlowMsg::Info) => self.return_msg(FlowMsg::Info), (Self::Confirmation, FlowMsg::Confirmed) => self.return_msg(FlowMsg::Confirmed), _ => self.do_nothing(), } @@ -227,6 +231,7 @@ pub fn new_confirm_action( hold: bool, prompt_screen: bool, prompt_title: TString<'static>, + external_menu: bool, ) -> Result { let paragraphs = { let action = action.unwrap_or("".into()); @@ -244,9 +249,19 @@ pub fn new_confirm_action( paragraphs.into_paragraphs() }; + if external_menu && (prompt_screen || hold) { + return Err(Error::ValueError( + c"external_menu currently not supported in tandem with prompt_screen/hold", + )); + } + new_confirm_action_simple( paragraphs, - ConfirmActionExtra::Menu(ConfirmActionMenuStrings::new().with_verb_cancel(verb_cancel)), + if external_menu { + ConfirmActionExtra::ExternalMenu + } else { + ConfirmActionExtra::Menu(ConfirmActionMenuStrings::new().with_verb_cancel(verb_cancel)) + }, ConfirmActionStrings::new(title, subtitle, None, prompt_screen.then_some(prompt_title)), hold, None, @@ -273,7 +288,7 @@ fn new_confirm_action_uni( .with_vertical_pages(); match extra { - ConfirmActionExtra::Menu { .. } => { + ConfirmActionExtra::Menu { .. } | ConfirmActionExtra::ExternalMenu => { content = content.with_menu_button(); } ConfirmActionExtra::Cancel => { @@ -327,8 +342,12 @@ fn create_flow( let initial_page: &dyn FlowController = match (extra, prompt_screen.is_some()) { (ConfirmActionExtra::Menu { .. }, false) => &ConfirmActionWithMenu::Action, (ConfirmActionExtra::Menu { .. }, true) => &ConfirmActionWithMenuAndConfirmation::Action, - (ConfirmActionExtra::Cancel, false) => &ConfirmAction::Action, - (ConfirmActionExtra::Cancel, true) => &ConfirmActionWithConfirmation::Action, + (ConfirmActionExtra::Cancel | ConfirmActionExtra::ExternalMenu, false) => { + &ConfirmAction::Action + } + (ConfirmActionExtra::Cancel | ConfirmActionExtra::ExternalMenu, true) => { + &ConfirmActionWithConfirmation::Action + } }; ( diff --git a/core/embed/rust/src/ui/layout_delizia/ui_firmware.rs b/core/embed/rust/src/ui/layout_delizia/ui_firmware.rs index 5bab900eae..7ca6339eb5 100644 --- a/core/embed/rust/src/ui/layout_delizia/ui_firmware.rs +++ b/core/embed/rust/src/ui/layout_delizia/ui_firmware.rs @@ -61,17 +61,19 @@ impl FirmwareUI for UIDelizia { reverse: bool, prompt_screen: bool, prompt_title: Option>, + external_menu: bool, // TODO: will eventually replace the internal menu ) -> Result { let flow = flow::confirm_action::new_confirm_action( title, action, description, subtitle, - verb_cancel, + if external_menu { None } else { verb_cancel }, reverse, hold, prompt_screen, prompt_title.unwrap_or(TString::empty()), + external_menu, )?; Ok(flow) } diff --git a/core/embed/rust/src/ui/layout_eckhart/ui_firmware.rs b/core/embed/rust/src/ui/layout_eckhart/ui_firmware.rs index ebd3eea226..7c278e3f98 100644 --- a/core/embed/rust/src/ui/layout_eckhart/ui_firmware.rs +++ b/core/embed/rust/src/ui/layout_eckhart/ui_firmware.rs @@ -57,6 +57,7 @@ impl FirmwareUI for UIEckhart { reverse: bool, _prompt_screen: bool, _prompt_title: Option>, + _external_menu: bool, // TODO: will eventually replace the internal menu ) -> Result { let paragraphs = { let action = action.unwrap_or("".into()); diff --git a/core/embed/rust/src/ui/ui_firmware.rs b/core/embed/rust/src/ui/ui_firmware.rs index 4124d446a7..05eaf409ee 100644 --- a/core/embed/rust/src/ui/ui_firmware.rs +++ b/core/embed/rust/src/ui/ui_firmware.rs @@ -32,6 +32,7 @@ pub trait FirmwareUI { reverse: bool, prompt_screen: bool, prompt_title: Option>, + external_menu: bool, // TODO: will eventually replace the internal menu ) -> Result; fn confirm_address( diff --git a/core/mocks/generated/trezorui_api.pyi b/core/mocks/generated/trezorui_api.pyi index 86d02999be..f2cbd5e6cf 100644 --- a/core/mocks/generated/trezorui_api.pyi +++ b/core/mocks/generated/trezorui_api.pyi @@ -110,6 +110,7 @@ def confirm_action( reverse: bool = False, prompt_screen: bool = False, prompt_title: str | None = None, + external_menu: bool = False, ) -> LayoutObj[UiResult]: """Confirm action.""" diff --git a/core/src/trezor/ui/layouts/delizia/__init__.py b/core/src/trezor/ui/layouts/delizia/__init__.py index 197016de94..f88e9192da 100644 --- a/core/src/trezor/ui/layouts/delizia/__init__.py +++ b/core/src/trezor/ui/layouts/delizia/__init__.py @@ -39,28 +39,48 @@ def confirm_action( prompt_screen: bool = False, prompt_title: str | None = None, ) -> Awaitable[None]: + from trezor.ui.layouts.menu import Menu, confirm_with_menu + if description is not None and description_param is not None: description = description.format(description_param) - return raise_if_not_confirmed( - trezorui_api.confirm_action( - title=title, - action=action, - description=description, - subtitle=subtitle, - verb=verb, - verb_cancel=verb_cancel, - hold=hold, - hold_danger=hold_danger, - reverse=reverse, - prompt_screen=prompt_screen, - prompt_title=prompt_title or title, - ), - br_name, - br_code, - exc, + flow = trezorui_api.confirm_action( + title=title, + action=action, + description=description, + subtitle=subtitle, + verb=verb, + verb_cancel=verb_cancel, + hold=hold, + hold_danger=hold_danger, + reverse=reverse, + prompt_screen=prompt_screen, + prompt_title=prompt_title or title, + external_menu=not (prompt_screen or hold), ) + if prompt_screen or hold: + # Note: multi-step confirm (prompt_screen/hold) + # can't work with external menus yet + return raise_if_not_confirmed( + flow, + br_name, + br_code, + exc, + ) + else: + menu = Menu.root( + cancel=verb_cancel or TR.buttons__cancel, + ) + + return confirm_with_menu( + flow, + menu, + br_name, + br_code, + exc, + ) + def confirm_single( br_name: str,