From 3434838bbe22970eac91a17a101950e839c22fd6 Mon Sep 17 00:00:00 2001 From: grdddj Date: Mon, 14 Nov 2022 09:49:57 +0100 Subject: [PATCH] WIP - bip39 design improvements --- .../rust/src/ui/model_tr/component/button.rs | 8 +++- .../rust/src/ui/model_tr/component/common.rs | 15 +++++++ .../rust/src/ui/model_tr/component/frame.rs | 39 ++++++++++++++++--- .../src/ui/model_tr/component/passphrase.rs | 24 ++++-------- core/embed/rust/src/ui/model_tr/layout.rs | 17 +++----- .../management/recovery_device/homescreen.py | 2 +- core/src/trezor/ui/layouts/tr/__init__.py | 7 ++-- core/src/trezor/ui/layouts/tr/recovery.py | 19 ++++++--- 8 files changed, 89 insertions(+), 42 deletions(-) diff --git a/core/embed/rust/src/ui/model_tr/component/button.rs b/core/embed/rust/src/ui/model_tr/component/button.rs index c801683d83..26fbbf4094 100644 --- a/core/embed/rust/src/ui/model_tr/component/button.rs +++ b/core/embed/rust/src/ui/model_tr/component/button.rs @@ -515,7 +515,13 @@ impl> ButtonDetails { self } - /// Duration of the hold-to-confirm. + /// Default duration of the hold-to-confirm. + pub fn with_default_duration(mut self) -> Self { + self.duration = Some(Duration::from_millis(1000)); + self + } + + /// Specific duration of the hold-to-confirm. pub fn with_duration(mut self, duration: Duration) -> Self { self.duration = Some(duration); self diff --git a/core/embed/rust/src/ui/model_tr/component/common.rs b/core/embed/rust/src/ui/model_tr/component/common.rs index ede29fa4bb..8a4db0a13f 100644 --- a/core/embed/rust/src/ui/model_tr/component/common.rs +++ b/core/embed/rust/src/ui/model_tr/component/common.rs @@ -83,6 +83,21 @@ pub fn paint_header_left>(title: T, area: Rect) -> i16 { text_heigth } +/// Display title/header centered at the top of the given area. +/// Returning the painted height of the whole header. +pub fn paint_header_centered>(title: T, area: Rect) -> i16 { + let text_heigth = theme::FONT_HEADER.text_height(); + let title_baseline = area.top_center() + Offset::y(text_heigth); + display::text_center( + title_baseline, + title.as_ref(), + theme::FONT_HEADER, + theme::FG, + theme::BG, + ); + text_heigth +} + /// Draws icon and text on the same line - icon on the left. pub fn icon_with_text>(baseline: Point, icon: Icon, text: T, font: Font) { icon.draw_bottom_left(baseline, theme::FG, theme::BG); diff --git a/core/embed/rust/src/ui/model_tr/component/frame.rs b/core/embed/rust/src/ui/model_tr/component/frame.rs index dfe114756a..b7d8d12cb5 100644 --- a/core/embed/rust/src/ui/model_tr/component/frame.rs +++ b/core/embed/rust/src/ui/model_tr/component/frame.rs @@ -5,9 +5,12 @@ use crate::ui::{ }; /// Component for holding another component and displaying a title. +/// Also is allocating space for a scrollbar. pub struct Frame { area: Rect, title: U, + title_centered: bool, + account_for_scrollbar: bool, content: Child, } @@ -20,6 +23,8 @@ where Self { title, area: Rect::zero(), + title_centered: false, + account_for_scrollbar: true, content: Child::new(content), } } @@ -27,6 +32,20 @@ where pub fn inner(&self) -> &T { self.content.inner() } + + /// Aligning the title to the center, instead of the left. + /// Also disabling scrollbar in this case, as they are not compatible. + pub fn with_title_center(mut self, title_centered: bool) -> Self { + self.title_centered = title_centered; + self.account_for_scrollbar = false; + self + } + + /// Allocating space for scrollbar in the top right. True by default. + pub fn with_scrollbar(mut self, account_for_scrollbar: bool) -> Self { + self.account_for_scrollbar = account_for_scrollbar; + self + } } impl Component for Frame @@ -43,10 +62,16 @@ where bounds.split_top(theme::FONT_HEADER.line_height()); let content_area = content_area.inset(Insets::top(TITLE_SPACE)); - let (title_area, scrollbar_area) = - title_and_scrollbar_area.split_right(ScrollBar::MAX_WIDTH); - - self.content.set_scrollbar_area(scrollbar_area); + // Title area is different based on scrollbar. + let title_area = if self.account_for_scrollbar { + let (title_area, scrollbar_area) = + title_and_scrollbar_area.split_right(ScrollBar::MAX_WIDTH); + // Sending the scrollbar area to the child component. + self.content.set_scrollbar_area(scrollbar_area); + title_area + } else { + title_and_scrollbar_area + }; self.area = title_area; self.content.place(content_area); @@ -58,7 +83,11 @@ where } fn paint(&mut self) { - common::paint_header_left(&self.title, self.area); + if self.title_centered { + common::paint_header_centered(&self.title, self.area); + } else { + common::paint_header_left(&self.title, self.area); + } self.content.paint(); } } diff --git a/core/embed/rust/src/ui/model_tr/component/passphrase.rs b/core/embed/rust/src/ui/model_tr/component/passphrase.rs index d882f6d54e..7a36f8a1fe 100644 --- a/core/embed/rust/src/ui/model_tr/component/passphrase.rs +++ b/core/embed/rust/src/ui/model_tr/component/passphrase.rs @@ -1,12 +1,9 @@ -use crate::{ - time::Duration, - ui::{ - component::{text::common::TextBox, Child, Component, ComponentExt, Event, EventCtx}, - display::Icon, - geometry::Rect, - model_tr::theme, - util::char_to_string, - }, +use crate::ui::{ + component::{text::common::TextBox, Child, Component, ComponentExt, Event, EventCtx}, + display::Icon, + geometry::Rect, + model_tr::theme, + util::char_to_string, }; use super::{ @@ -31,7 +28,6 @@ enum ChoiceCategory { } const MAX_PASSPHRASE_LENGTH: usize = 50; -const HOLD_DURATION: Duration = Duration::from_secs(1); const DIGITS: [char; 10] = ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9']; const LOWERCASE_LETTERS: [char; 26] = [ @@ -115,14 +111,10 @@ impl ChoiceFactoryPassphrase { // Including accept button on the left and cancel on the very right. // TODO: could have some icons instead of the shortcut text if choice_index == 0 { - menu_item.set_left_btn(Some( - ButtonDetails::text("ACC").with_duration(HOLD_DURATION), - )); + menu_item.set_left_btn(Some(ButtonDetails::text("ACC").with_default_duration())); } if choice_index == MENU.len() as u8 - 1 { - menu_item.set_right_btn(Some( - ButtonDetails::text("CAN").with_duration(HOLD_DURATION), - )); + menu_item.set_right_btn(Some(ButtonDetails::text("CAN").with_default_duration())); } // Including icons for some items. diff --git a/core/embed/rust/src/ui/model_tr/layout.rs b/core/embed/rust/src/ui/model_tr/layout.rs index 43382e3610..05d9aff788 100644 --- a/core/embed/rust/src/ui/model_tr/layout.rs +++ b/core/embed/rust/src/ui/model_tr/layout.rs @@ -175,7 +175,7 @@ extern "C" fn new_confirm_action(n_args: usize, args: *const Obj, kwargs: *mut M // Optional HoldToConfirm if hold { // TODO: clients might want to set the duration - confirm_btn = confirm_btn.map(|btn| btn.with_duration(Duration::from_secs(2))); + confirm_btn = confirm_btn.map(|btn| btn.with_default_duration()); } let content = ButtonPage::new_str_buf( @@ -255,8 +255,7 @@ extern "C" fn confirm_properties(n_args: usize, args: *const Obj, kwargs: *mut M let mut content = ButtonPage::new_str(paragraphs.into_paragraphs(), theme::BG); if hold { - let confirm_btn = - Some(ButtonDetails::text("CONFIRM").with_duration(Duration::from_secs(1))); + let confirm_btn = Some(ButtonDetails::text("CONFIRM").with_default_duration()); content = content.with_confirm_btn(confirm_btn); } let obj = LayoutObj::new(Frame::new(title, content))?; @@ -298,10 +297,7 @@ extern "C" fn confirm_output(n_args: usize, args: *const Obj, kwargs: *mut Map) let btn_layout = ButtonLayout::new( Some(ButtonDetails::cancel_icon()), None, - Some( - ButtonDetails::text("HOLD TO CONFIRM") - .with_duration(Duration::from_secs(2)), - ), + Some(ButtonDetails::text("HOLD TO CONFIRM").with_default_duration()), ); let btn_actions = ButtonActions::cancel_confirm(); Page::<20>::new(btn_layout, btn_actions, Font::MONO) @@ -342,7 +338,7 @@ extern "C" fn confirm_total(n_args: usize, args: *const Obj, kwargs: *mut Map) - let btn_layout = ButtonLayout::new( Some(ButtonDetails::cancel_icon()), None, - Some(ButtonDetails::text("HOLD TO SEND").with_duration(Duration::from_secs(2))), + Some(ButtonDetails::text("HOLD TO SEND").with_default_duration()), ); let btn_actions = ButtonActions::cancel_confirm(); @@ -523,8 +519,7 @@ extern "C" fn show_share_words(n_args: usize, args: *const Obj, kwargs: *mut Map let share_words_obj: Obj = kwargs.get(Qstr::MP_QSTR_share_words)?; let share_words: Vec = iter_into_vec(share_words_obj)?; - let confirm_btn = - Some(ButtonDetails::text("HOLD TO CONFIRM").with_duration(Duration::from_secs(1))); + let confirm_btn = Some(ButtonDetails::text("HOLD TO CONFIRM").with_default_duration()); let obj = LayoutObj::new( ButtonPage::new_str(ShareWords::new(share_words), theme::BG) @@ -564,7 +559,7 @@ extern "C" fn request_word_bip39(n_args: usize, args: *const Obj, kwargs: *mut M let block = |_args: &[Obj], kwargs: &Map| { let prompt: StrBuffer = kwargs.get(Qstr::MP_QSTR_prompt)?.try_into()?; - let obj = LayoutObj::new(Frame::new(prompt, Bip39Entry::new()))?; + let obj = LayoutObj::new(Frame::new(prompt, Bip39Entry::new()).with_title_center(true))?; Ok(obj.into()) }; unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } diff --git a/core/src/apps/management/recovery_device/homescreen.py b/core/src/apps/management/recovery_device/homescreen.py index f8b80a21c6..a7b2dfc094 100644 --- a/core/src/apps/management/recovery_device/homescreen.py +++ b/core/src/apps/management/recovery_device/homescreen.py @@ -66,7 +66,7 @@ async def _continue_recovery_process(ctx: GenericContext) -> Success: # If we are starting recovery, ask for word count first... # _request_word_count await layout.homescreen_dialog( - ctx, "Select", "Select the number of words in your recovery seed" + ctx, "Select", "Select the number of words in your recovery seed." ) # ask for the number of words word_count = await layout.request_word_count(ctx, dry_run) diff --git a/core/src/trezor/ui/layouts/tr/__init__.py b/core/src/trezor/ui/layouts/tr/__init__.py index 8d34fb954d..be2d2cf4f7 100644 --- a/core/src/trezor/ui/layouts/tr/__init__.py +++ b/core/src/trezor/ui/layouts/tr/__init__.py @@ -419,11 +419,12 @@ async def get_bool( ctx: wire.GenericContext, br_type: str, title: str, - data: str, + data: str | None = None, description: str | None = None, verb: str | None = "CONFIRM", verb_cancel: str | None = "", hold: bool = False, + reverse: bool = False, br_code: ButtonRequestType = ButtonRequestType.Other, ) -> bool: result = await interact( @@ -436,7 +437,7 @@ async def get_bool( verb=verb, verb_cancel=verb_cancel, hold=hold, - reverse=False, + reverse=reverse, ) ), br_type, @@ -526,7 +527,7 @@ async def confirm_reset_device( if show_tutorial: await tutorial(ctx) - to_show = "By continuing you agree to our terms and conditions.\n\nMore info at trezor.io/tos." + to_show = "By continuing you agree to our terms and conditions.\nMore info at trezor.io/tos." if not recovery: to_show += "\nUse you backup to recover your wallet." diff --git a/core/src/trezor/ui/layouts/tr/recovery.py b/core/src/trezor/ui/layouts/tr/recovery.py index 37b5dbe225..524cbbf33b 100644 --- a/core/src/trezor/ui/layouts/tr/recovery.py +++ b/core/src/trezor/ui/layouts/tr/recovery.py @@ -73,11 +73,20 @@ async def continue_recovery( info_func: Callable | None, dry_run: bool, ) -> bool: + # NOTE: no need to implement `info_func`, as it is used only in + # Shamir backup, which is not implemented for TR. + + description = text + if subtext: + description += f"\n\n{subtext}" return await get_bool( - ctx=ctx, - title="START RECOVERY", - data=f"{text}\n\n{subtext or ''}", - verb="START", - br_type="recovery", + ctx, + "recovery", + "START RECOVERY", + None, + description, + verb="HOLD TO BEGIN", + hold=True, + reverse=True, br_code=ButtonRequestType.RecoveryHomepage, )