From 3b40950f7a3134f41c2048ae89d508e0457e9593 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ioan=20Biz=C4=83u?= Date: Tue, 12 Nov 2024 17:39:12 +0100 Subject: [PATCH] refactor(core): drop confirm_blob_with_optional_pagination Commit c300576d6c48662d2d83727cfbc0fb2dd5b83c23 introduced `confirm_blob_with_optional_pagination` which proved to be unpopular and impractical. This commit brings back the old behaviour of having the `ask_pagination` parameter on `confirm_blob`. It also reverts back to using the old way of paginating `confirm_blob` on model R, which the aforementioned commit ignored and re-implemented from scratch. [no changelog] --- .../rust/src/ui/model_tr/component/page.rs | 62 +-------- core/embed/rust/src/ui/model_tr/layout.rs | 47 +------ core/src/apps/ethereum/layout.py | 13 +- core/src/trezor/ui/layouts/common.py | 11 +- .../src/trezor/ui/layouts/mercury/__init__.py | 131 ++++++++++-------- core/src/trezor/ui/layouts/tr/__init__.py | 40 +++--- core/src/trezor/ui/layouts/tt/__init__.py | 56 ++++---- 7 files changed, 133 insertions(+), 227 deletions(-) diff --git a/core/embed/rust/src/ui/model_tr/component/page.rs b/core/embed/rust/src/ui/model_tr/component/page.rs index 836d9d4c17..7961bbb4de 100644 --- a/core/embed/rust/src/ui/model_tr/component/page.rs +++ b/core/embed/rust/src/ui/model_tr/component/page.rs @@ -2,7 +2,7 @@ use crate::{ translations::TR, ui::{ component::{Child, Component, ComponentExt, Event, EventCtx, Pad, PageMsg, Paginate}, - display::{Color, Font}, + display::Color, geometry::{Insets, Rect}, shape::Renderer, }, @@ -13,25 +13,16 @@ use super::{ ButtonDetails, ButtonLayout, ButtonPos, }; -#[derive(PartialEq)] -enum LastPageLayout { - Confirm, - ArmedConfirmPlusInfo, -} - pub struct ButtonPage where T: Component + Paginate, { page_count: usize, active_page: usize, - page_limit: Option, - last_page_layout: LastPageLayout, content: Child, pad: Pad, cancel_btn_details: Option, confirm_btn_details: Option, - info_btn_details: Option, back_btn_details: Option, next_btn_details: Option, buttons: Child, @@ -45,17 +36,10 @@ where Self { page_count: 0, // will be set in place() active_page: 0, - page_limit: None, - last_page_layout: LastPageLayout::Confirm, content: Child::new(content), pad: Pad::with_background(background).with_clear(), cancel_btn_details: Some(ButtonDetails::cancel_icon()), confirm_btn_details: Some(ButtonDetails::text(TR::buttons__confirm.into())), - info_btn_details: Some( - ButtonDetails::text("i".into()) - .with_fixed_width(theme::BUTTON_ICON_WIDTH) - .with_font(Font::NORMAL), - ), back_btn_details: Some(ButtonDetails::up_arrow_icon()), next_btn_details: Some(ButtonDetails::down_arrow_icon_wide()), // Setting empty layout for now, we do not yet know the page count. @@ -65,12 +49,6 @@ where } } - pub fn with_armed_confirm_plus_info(mut self) -> Self { - self.last_page_layout = LastPageLayout::ArmedConfirmPlusInfo; - self.confirm_btn_details = Some(ButtonDetails::armed_text(TR::words__confirm.into())); - self - } - pub fn with_cancel_btn(mut self, btn_details: Option) -> Self { self.cancel_btn_details = btn_details; self @@ -91,11 +69,6 @@ where self } - pub fn with_page_limit(mut self, page_limit: Option) -> Self { - self.page_limit = page_limit; - self - } - pub fn has_next_page(&self) -> bool { self.active_page < self.page_count - 1 } @@ -142,9 +115,8 @@ where fn get_button_layout(&self, has_prev: bool, has_next: bool) -> ButtonLayout { let btn_left = self.get_left_button_details(!has_prev); - let btn_middle = self.get_middle_button_details(has_next); let btn_right = self.get_right_button_details(has_next); - ButtonLayout::new(btn_left, btn_middle, btn_right) + ButtonLayout::new(btn_left, None, btn_right) } fn get_left_button_details(&self, is_first: bool) -> Option { @@ -155,21 +127,11 @@ where } } - fn get_middle_button_details(&self, has_next_page: bool) -> Option { - if has_next_page || self.last_page_layout == LastPageLayout::Confirm { - None - } else { - self.confirm_btn_details.clone() - } - } - fn get_right_button_details(&self, has_next_page: bool) -> Option { if has_next_page { self.next_btn_details.clone() - } else if self.last_page_layout == LastPageLayout::Confirm { - self.confirm_btn_details.clone() } else { - self.info_btn_details.clone() + self.confirm_btn_details.clone() } } } @@ -203,10 +165,6 @@ where // Need to be called here, only after content is placed // and we can calculate the page count. self.page_count = self.content.page_count(); - if let Some(limit) = self.page_limit { - self.page_count = self.page_count.min(limit); - } - self.set_buttons_for_initial_page(self.page_count); self.buttons.place(button_area); bounds @@ -226,25 +184,17 @@ where return Some(PageMsg::Cancelled); } } - ButtonPos::Middle => { - return Some(PageMsg::Confirmed); - } ButtonPos::Right => { if self.has_next_page() { // Clicked NEXT. Scroll down. self.go_to_next_page(); self.change_page(ctx); } else { - match self.last_page_layout { - LastPageLayout::Confirm => { - return Some(PageMsg::Confirmed); - } - LastPageLayout::ArmedConfirmPlusInfo => { - return Some(PageMsg::Info); - } - } + // Clicked CONFIRM. Send result. + return Some(PageMsg::Confirmed); } } + _ => {} } } diff --git a/core/embed/rust/src/ui/model_tr/layout.rs b/core/embed/rust/src/ui/model_tr/layout.rs index 313b7b195a..5b8d06b6ef 100644 --- a/core/embed/rust/src/ui/model_tr/layout.rs +++ b/core/embed/rust/src/ui/model_tr/layout.rs @@ -95,7 +95,6 @@ where match msg { PageMsg::Confirmed => Ok(CONFIRMED.as_obj()), PageMsg::Cancelled => Ok(CANCELLED.as_obj()), - PageMsg::Info => Ok(INFO.as_obj()), _ => Err(Error::TypeError), } } @@ -245,9 +244,7 @@ fn content_in_button_page( content: T, verb: TString<'static>, verb_cancel: Option>, - info: bool, hold: bool, - page_limit: Option, ) -> Result { // Left button - icon, text or nothing. let cancel_btn = verb_cancel.map(ButtonDetails::from_text_possible_icon); @@ -263,15 +260,10 @@ fn content_in_button_page( confirm_btn = confirm_btn.map(|btn| btn.with_default_duration()); } - let mut content = ButtonPage::new(content, theme::BG) + let content = ButtonPage::new(content, theme::BG) .with_cancel_btn(cancel_btn) - .with_page_limit(page_limit) .with_confirm_btn(confirm_btn); - if info { - content = content.with_armed_confirm_plus_info(); - } - let mut frame = ScrollableFrame::new(content); if !title.is_empty() { frame = frame.with_title(title); @@ -312,7 +304,7 @@ extern "C" fn new_confirm_action(n_args: usize, args: *const Obj, kwargs: *mut M paragraphs.into_paragraphs() }; - content_in_button_page(title, paragraphs, verb, verb_cancel, false, hold, None) + content_in_button_page(title, paragraphs, verb, verb_cancel, hold) }; unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } } @@ -335,13 +327,8 @@ extern "C" fn new_confirm_blob(n_args: usize, args: *const Obj, kwargs: *mut Map .get(Qstr::MP_QSTR_verb_cancel) .unwrap_or_else(|_| Obj::const_none()) .try_into_option()?; - let info: bool = kwargs.get_or(Qstr::MP_QSTR_info, false)?; let hold: bool = kwargs.get_or(Qstr::MP_QSTR_hold, false)?; let chunkify: bool = kwargs.get_or(Qstr::MP_QSTR_chunkify, false)?; - let page_limit: Option = kwargs - .get(Qstr::MP_QSTR_page_limit) - .unwrap_or_else(|_| Obj::const_none()) - .try_into_option()?; let style = if chunkify { // Chunkifying the address into smaller pieces when requested @@ -365,9 +352,7 @@ extern "C" fn new_confirm_blob(n_args: usize, args: *const Obj, kwargs: *mut Map paragraphs, verb.unwrap_or(TR::buttons__confirm.into()), verb_cancel, - info, hold, - page_limit, ) }; unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } @@ -420,9 +405,7 @@ extern "C" fn new_confirm_properties(n_args: usize, args: *const Obj, kwargs: *m paragraphs.into_paragraphs(), button_text, Some("".into()), - false, hold, - None, ) }; unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } @@ -452,15 +435,7 @@ extern "C" fn new_confirm_reset_device(n_args: usize, args: *const Obj, kwargs: .text_bold(TR::reset__tos_link); let formatted = FormattedText::new(ops).vertically_centered(); - content_in_button_page( - title, - formatted, - button, - Some("".into()), - false, - false, - None, - ) + content_in_button_page(title, formatted, button, Some("".into()), false) }; unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } } @@ -542,9 +517,7 @@ extern "C" fn new_confirm_value(n_args: usize, args: *const Obj, kwargs: *mut Ma paragraphs, verb.unwrap_or(TR::buttons__confirm.into()), Some("".into()), - false, hold, - None, ) }; unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } @@ -567,9 +540,7 @@ extern "C" fn new_confirm_joint_total(n_args: usize, args: *const Obj, kwargs: * paragraphs, TR::buttons__hold_to_confirm.into(), Some("".into()), - false, true, - None, ) }; unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } @@ -600,8 +571,6 @@ extern "C" fn new_confirm_modify_output(n_args: usize, args: *const Obj, kwargs: TR::buttons__confirm.into(), Some("".into()), false, - false, - None, ) }; unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } @@ -975,8 +944,6 @@ extern "C" fn new_confirm_modify_fee(n_args: usize, args: *const Obj, kwargs: *m TR::buttons__confirm.into(), Some("".into()), false, - false, - None, ) }; unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } @@ -1263,8 +1230,6 @@ extern "C" fn new_confirm_more(n_args: usize, args: *const Obj, kwargs: *mut Map button, Some("<".into()), false, - false, - None, ) }; unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } @@ -1290,9 +1255,7 @@ extern "C" fn new_confirm_coinjoin(n_args: usize, args: *const Obj, kwargs: *mut paragraphs, TR::buttons__hold_to_confirm.into(), None, - false, true, - None, ) }; unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } @@ -1488,8 +1451,6 @@ extern "C" fn new_confirm_recovery(n_args: usize, args: *const Obj, kwargs: *mut button, Some("".into()), false, - false, - None, ) }; unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } @@ -1542,8 +1503,6 @@ extern "C" fn new_show_group_share_success( TR::buttons__continue.into(), None, false, - false, - None, ) }; unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } diff --git a/core/src/apps/ethereum/layout.py b/core/src/apps/ethereum/layout.py index fb04896567..99f7af07fc 100644 --- a/core/src/apps/ethereum/layout.py +++ b/core/src/apps/ethereum/layout.py @@ -4,7 +4,6 @@ from trezor import TR, ui from trezor.enums import ButtonRequestType from trezor.ui.layouts import ( confirm_blob, - confirm_blob_with_optional_pagination, confirm_ethereum_staking_tx, confirm_text, should_show_more, @@ -174,15 +173,9 @@ def require_confirm_address(address_bytes: bytes) -> Awaitable[None]: def require_confirm_other_data(data: bytes, data_total: int) -> Awaitable[None]: - return confirm_blob_with_optional_pagination( - "confirm_data", - TR.ethereum__title_input_data, - data, - subtitle=TR.ethereum__data_size_template.format(data_total), - verb=TR.buttons__confirm, - verb_cancel=TR.send__cancel_sign, - br_code=ButtonRequestType.SignTx, - ) + from trezor.ui.layouts import confirm_other_data + + return confirm_other_data(data, data_total) async def confirm_typed_data_final() -> None: diff --git a/core/src/trezor/ui/layouts/common.py b/core/src/trezor/ui/layouts/common.py index 4374bba5b2..52d92c5c7f 100644 --- a/core/src/trezor/ui/layouts/common.py +++ b/core/src/trezor/ui/layouts/common.py @@ -73,6 +73,7 @@ async def with_info( br_name: str, br_code: ButtonRequestType, repeat_button_request: bool = False, # TODO this should eventually always be true + info_layout_can_confirm: bool = False, # whether the info layout is allowed to confirm the whole flow ) -> None: first_br = br_name next_br = br_name if repeat_button_request else None @@ -95,8 +96,14 @@ async def with_info( if result is trezorui2.CONFIRMED: return elif result is trezorui2.INFO: - await interact(info_layout, next_br, br_code, raise_on_cancel=None) - continue + info_result = await interact( + info_layout, next_br, br_code, raise_on_cancel=None + ) + if info_layout_can_confirm and info_result is trezorui2.CONFIRMED: + return + else: + # no matter what the info layout returns, we always go back to the main layout + continue else: raise RuntimeError # unexpected result diff --git a/core/src/trezor/ui/layouts/mercury/__init__.py b/core/src/trezor/ui/layouts/mercury/__init__.py index 28069c5a4b..f99c84b386 100644 --- a/core/src/trezor/ui/layouts/mercury/__init__.py +++ b/core/src/trezor/ui/layouts/mercury/__init__.py @@ -460,48 +460,6 @@ async def should_show_more( raise ActionCancelled -def confirm_blob_with_optional_pagination( - br_name: str, - title: str, - data: bytes | str, - subtitle: str | None = None, - verb: str | None = None, - verb_cancel: str | None = None, - br_code: ButtonRequestType = BR_CODE_OTHER, - chunkify: bool = False, -) -> Awaitable[None]: - main_layout = trezorui2.confirm_blob( - title=title, - data=data, - description=TR.instructions__view_all_data, - description_font_green=True, - subtitle=subtitle, - verb=verb, - verb_cancel=verb_cancel, - verb_info=TR.buttons__view_all_data, - info=True, - hold=False, - chunkify=chunkify, - prompt_screen=False, - page_limit=1, - ) - info_layout = trezorui2.confirm_blob( - title=title, - data=data, - description=None, - verb=None, - verb_cancel=verb_cancel, - info=False, - hold=False, - chunkify=chunkify, - prompt_screen=False, - ) - - return with_info( - main_layout, info_layout, br_name, br_code, repeat_button_request=True - ) - - def confirm_blob( br_name: str, title: str, @@ -514,27 +472,80 @@ def confirm_blob( info: bool = True, hold: bool = False, br_code: ButtonRequestType = BR_CODE_OTHER, + ask_pagination: bool = False, chunkify: bool = False, prompt_screen: bool = True, ) -> Awaitable[None]: - layout = trezorui2.confirm_blob( - title=title, - data=data, - description=description, - text_mono=text_mono, - subtitle=subtitle, - verb=verb, - verb_cancel=verb_cancel, - info=info, - hold=hold, - chunkify=chunkify, - prompt_screen=prompt_screen, - ) - return raise_if_not_confirmed( - layout, - br_name, - br_code, - ) + if ask_pagination: + main_layout = trezorui2.confirm_blob( + title=title, + data=data, + description=TR.instructions__view_all_data, + description_font_green=True, + subtitle=subtitle, + verb=verb, + verb_cancel=verb_cancel, + verb_info=TR.buttons__view_all_data, + info=True, + hold=False, + chunkify=chunkify, + prompt_screen=False, + page_limit=1, + ) + info_layout = trezorui2.confirm_blob( + title=title, + data=data, + description=None, + verb=None, + verb_cancel=verb_cancel, + info=False, + hold=False, + chunkify=chunkify, + prompt_screen=False, + ) + + return with_info( + main_layout, + info_layout, + br_name, + br_code, + repeat_button_request=True, + info_layout_can_confirm=True, + ) + else: + layout = trezorui2.confirm_blob( + title=title, + data=data, + description=description, + text_mono=text_mono, + subtitle=subtitle, + verb=verb, + verb_cancel=verb_cancel, + info=info, + hold=hold, + chunkify=chunkify, + prompt_screen=prompt_screen, + ) + return raise_if_not_confirmed( + layout, + br_name, + br_code, + ) + + +if not utils.BITCOIN_ONLY: + + def confirm_other_data(data: bytes, data_total: int) -> Awaitable[None]: + return confirm_blob( + "confirm_data", + TR.ethereum__title_input_data, + data, + subtitle=TR.ethereum__data_size_template.format(data_total), + verb=TR.buttons__confirm, + verb_cancel=TR.send__cancel_sign, + br_code=ButtonRequestType.SignTx, + ask_pagination=True, + ) def confirm_address( diff --git a/core/src/trezor/ui/layouts/tr/__init__.py b/core/src/trezor/ui/layouts/tr/__init__.py index 1914846702..8ca962ff20 100644 --- a/core/src/trezor/ui/layouts/tr/__init__.py +++ b/core/src/trezor/ui/layouts/tr/__init__.py @@ -560,29 +560,6 @@ async def should_show_more( raise ActionCancelled -def confirm_blob_with_optional_pagination( - br_name: str, - title: str, - data: bytes | str, - subtitle: str | None = None, - verb: str | None = None, - verb_cancel: str | None = None, - br_code: ButtonRequestType = BR_CODE_OTHER, - chunkify: bool = False, -) -> Awaitable[None]: - return confirm_blob( - br_name, - title, - data, - subtitle=subtitle, - verb=verb, - verb_cancel="", - br_code=br_code, - chunkify=chunkify, - ask_pagination=True, - ) - - def confirm_blob( br_name: str, title: str, @@ -595,9 +572,9 @@ def confirm_blob( info: bool = True, hold: bool = False, br_code: ButtonRequestType = BR_CODE_OTHER, + ask_pagination: bool = False, chunkify: bool = False, prompt_screen: bool = True, - ask_pagination: bool = False, ) -> Awaitable[None]: verb = verb or TR.buttons__confirm # def_arg layout = trezorui2.confirm_blob( @@ -654,6 +631,21 @@ async def _confirm_ask_pagination( assert False +if not utils.BITCOIN_ONLY: + + def confirm_other_data(data: bytes, data_total: int) -> Awaitable[None]: + return confirm_blob( + "confirm_data", + TR.ethereum__title_input_data, + data, + subtitle=TR.ethereum__data_size_template.format(data_total), + verb=TR.buttons__confirm, + verb_cancel=None, + br_code=ButtonRequestType.SignTx, + ask_pagination=True, + ) + + def confirm_address( title: str, address: str, diff --git a/core/src/trezor/ui/layouts/tt/__init__.py b/core/src/trezor/ui/layouts/tt/__init__.py index 8c689ee11d..3f5a5ef94c 100644 --- a/core/src/trezor/ui/layouts/tt/__init__.py +++ b/core/src/trezor/ui/layouts/tt/__init__.py @@ -523,32 +523,6 @@ async def should_show_more( raise ActionCancelled -def confirm_blob_with_optional_pagination( - br_name: str, - title: str, - data: bytes | str, - subtitle: str | None = None, - verb: str | None = None, - verb_cancel: str | None = None, - br_code: ButtonRequestType = BR_CODE_OTHER, - chunkify: bool = False, -) -> Awaitable[None]: - verb = verb or TR.buttons__confirm # def_arg - layout = trezorui2.confirm_blob( - title=title, - description=None, - data=data, - verb=verb, - verb_cancel=None, - chunkify=chunkify, - ) - - if layout.page_count() > 1: - return _confirm_ask_pagination(br_name, title, data, subtitle or "", br_code) - else: - return raise_if_not_confirmed(layout, br_name, br_code) - - async def _confirm_ask_pagination( br_name: str, title: str, @@ -598,6 +572,7 @@ def confirm_blob( info: bool = True, hold: bool = False, br_code: ButtonRequestType = BR_CODE_OTHER, + ask_pagination: bool = False, chunkify: bool = False, prompt_screen: bool = True, ) -> Awaitable[None]: @@ -613,11 +588,30 @@ def confirm_blob( chunkify=chunkify, ) - return raise_if_not_confirmed( - layout, - br_name, - br_code, - ) + if ask_pagination and layout.page_count() > 1: + assert not hold + return _confirm_ask_pagination(br_name, title, data, description or "", br_code) + else: + return raise_if_not_confirmed( + layout, + br_name, + br_code, + ) + + +if not utils.BITCOIN_ONLY: + + def confirm_other_data(data: bytes, data_total: int) -> Awaitable[None]: + return confirm_blob( + "confirm_data", + TR.ethereum__title_input_data, + data, + subtitle=TR.ethereum__data_size_template.format(data_total), + verb=TR.buttons__confirm, + verb_cancel=None, + br_code=ButtonRequestType.SignTx, + ask_pagination=True, + ) def confirm_address(