From 53f5122c736d32456c3d8e10967f2af89ab8f27c Mon Sep 17 00:00:00 2001 From: grdddj Date: Fri, 4 Nov 2022 11:19:43 +0100 Subject: [PATCH] WIP - fixes, wording, tutorial --- .../rust/src/ui/component/text/formatted.rs | 6 +- core/embed/rust/src/ui/display/font.rs | 11 +- core/embed/rust/src/ui/layout/util.rs | 2 +- .../rust/src/ui/model_tr/component/button.rs | 32 ++++ .../rust/src/ui/model_tr/component/dialog.rs | 3 +- .../rust/src/ui/model_tr/component/loader.rs | 3 +- .../rust/src/ui/model_tr/component/mod.rs | 1 + core/embed/rust/src/ui/model_tr/layout.rs | 172 ++++++++++-------- core/mocks/generated/trezorui2.pyi | 5 +- core/src/trezor/ui/layouts/tr/__init__.py | 4 +- core/src/trezor/ui/layouts/tr/reset.py | 8 +- 11 files changed, 151 insertions(+), 96 deletions(-) diff --git a/core/embed/rust/src/ui/component/text/formatted.rs b/core/embed/rust/src/ui/component/text/formatted.rs index 93574abd9a..c99e4e4f8c 100644 --- a/core/embed/rust/src/ui/component/text/formatted.rs +++ b/core/embed/rust/src/ui/component/text/formatted.rs @@ -8,12 +8,10 @@ use heapless::LinearMap; use crate::ui::{ component::{Component, Event, EventCtx, Never}, display::{Color, Font}, - geometry::{Rect}, + geometry::Rect, }; -use super::layout::{ - LayoutFit, LayoutSink, LineBreaking, Op, TextLayout, TextRenderer, TextStyle, -}; +use super::layout::{LayoutFit, LayoutSink, LineBreaking, Op, TextLayout, TextRenderer, TextStyle}; pub const MAX_ARGUMENTS: usize = 6; diff --git a/core/embed/rust/src/ui/display/font.rs b/core/embed/rust/src/ui/display/font.rs index 862b22458b..f1d833a695 100644 --- a/core/embed/rust/src/ui/display/font.rs +++ b/core/embed/rust/src/ui/display/font.rs @@ -1,8 +1,13 @@ -use crate::{ui::{constant, geometry::{Point, Offset, Rect}}, trezorhal::display}; +use crate::{ + trezorhal::display, + ui::{ + constant, + geometry::{Offset, Point, Rect}, + }, +}; use core::slice; -use super::{Color, get_color_table, pixeldata, set_window, get_offset}; - +use super::{get_color_table, get_offset, pixeldata, set_window, Color}; pub struct Glyph { pub width: i16, diff --git a/core/embed/rust/src/ui/layout/util.rs b/core/embed/rust/src/ui/layout/util.rs index e94d0fc4ea..9e69322362 100644 --- a/core/embed/rust/src/ui/layout/util.rs +++ b/core/embed/rust/src/ui/layout/util.rs @@ -44,7 +44,7 @@ where T: TryFrom, { let err = Error::ValueError(cstr!("Invalid iterable length")); - let vec: Vec = iter_into_vec(iterable)?; + let vec: Vec = iter_into_vec(iterable)?; // Returns error if array.len() != N vec.into_array().map_err(|_| err) } 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 393ceafb5b..d8e1f96b21 100644 --- a/core/embed/rust/src/ui/model_tr/component/button.rs +++ b/core/embed/rust/src/ui/model_tr/component/button.rs @@ -576,6 +576,15 @@ impl ButtonLayout<&'static str> { ) } + /// Left and right texts. + pub fn left_right_text(text_left: &'static str, text_right: &'static str) -> Self { + Self::new( + Some(ButtonDetails::text(text_left)), + None, + Some(ButtonDetails::text(text_right)), + ) + } + /// Left and right arrow icons for navigation. pub fn left_right_arrows() -> Self { Self::new( @@ -611,6 +620,29 @@ impl ButtonLayout<&'static str> { Some(ButtonDetails::text(text).with_duration(duration)), ) } + + /// Arrow back on left and hold-to-confirm text on the right. + pub fn back_and_htc_text(text: &'static str, duration: Duration) -> Self { + Self::new( + Some(ButtonDetails::left_arrow_icon()), + None, + Some(ButtonDetails::text(text).with_duration(duration)), + ) + } + + /// Arrow back on left and text on the right. + pub fn back_and_text(text: &'static str) -> Self { + Self::new( + Some(ButtonDetails::left_arrow_icon()), + None, + Some(ButtonDetails::text(text)), + ) + } + + /// Only armed text in the middle. + pub fn middle_armed_text(text: &'static str) -> Self { + Self::new(None, Some(ButtonDetails::armed_text(text)), None) + } } /// What happens when a button is triggered. diff --git a/core/embed/rust/src/ui/model_tr/component/dialog.rs b/core/embed/rust/src/ui/model_tr/component/dialog.rs index 630466b3fe..e61f2a3aed 100644 --- a/core/embed/rust/src/ui/model_tr/component/dialog.rs +++ b/core/embed/rust/src/ui/model_tr/component/dialog.rs @@ -1,7 +1,8 @@ use super::button::{Button, ButtonMsg::Clicked}; use crate::ui::{ component::{Child, Component, Event, EventCtx}, - geometry::Rect, model_tr::theme, + geometry::Rect, + model_tr::theme, }; pub enum DialogMsg { diff --git a/core/embed/rust/src/ui/model_tr/component/loader.rs b/core/embed/rust/src/ui/model_tr/component/loader.rs index 1fd629c4c9..f6c5ce6471 100644 --- a/core/embed/rust/src/ui/model_tr/component/loader.rs +++ b/core/embed/rust/src/ui/model_tr/component/loader.rs @@ -167,7 +167,8 @@ impl> Loader { // TODO: support painting icons if let Some(text_overlay) = &mut self.text_overlay { // NOTE: need to calculate this in `i32`, it would overflow using `i16` - let invert_from = ((self.area.width() as i32 + 1) * done) / (display::LOADER_MAX as i32); + let invert_from = + ((self.area.width() as i32 + 1) * done) / (display::LOADER_MAX as i32); // TODO: the text should be moved one pixel to the top so it is centered in the // loader diff --git a/core/embed/rust/src/ui/model_tr/component/mod.rs b/core/embed/rust/src/ui/model_tr/component/mod.rs index 5c0b679dc6..9e9c49a257 100644 --- a/core/embed/rust/src/ui/model_tr/component/mod.rs +++ b/core/embed/rust/src/ui/model_tr/component/mod.rs @@ -36,6 +36,7 @@ pub use choice_item::ChoiceItem; pub use dialog::{Dialog, DialogMsg}; pub use flow::{Flow, FlowMsg}; pub use flow_pages::{FlowPages, Page}; +pub use flow_pages_poc_helpers::LineAlignment; pub use frame::Frame; pub use loader::{Loader, LoaderMsg, LoaderStyle, LoaderStyleSheet}; pub use page::ButtonPage; diff --git a/core/embed/rust/src/ui/model_tr/layout.rs b/core/embed/rust/src/ui/model_tr/layout.rs index 740379301a..5ac531eea1 100644 --- a/core/embed/rust/src/ui/model_tr/layout.rs +++ b/core/embed/rust/src/ui/model_tr/layout.rs @@ -28,6 +28,7 @@ use crate::{ util::iter_into_vec, util::upy_disable_animation, }, + model_tr::component::LineAlignment, }, }; @@ -305,6 +306,34 @@ extern "C" fn confirm_total(n_args: usize, args: *const Obj, kwargs: *mut Map) - unsafe { util::try_with_args_and_kwargs(n_args, args, kwargs, block) } } +/// General pattern of most tutorial screens. +/// (title, text, btn_layout, btn_actions) +fn tutorial_screen( + data: ( + StrBuffer, + StrBuffer, + ButtonLayout<&'static str>, + ButtonActions, + ), +) -> Page<10> { + let (title, text, btn_layout, btn_actions) = data; + let mut page = Page::<10>::new( + btn_layout, + btn_actions, + if !title.is_empty() { + Font::BOLD + } else { + Font::MONO + }, + ); + // Add title if present + if !title.is_empty() { + page = page.text_bold(title).newline().newline_half() + } + page = page.text_mono(text); + page +} + extern "C" fn tutorial(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj { let block = |_args: &[Obj], _kwargs: &Map| { const PAGE_COUNT: u8 = 7; @@ -315,83 +344,72 @@ extern "C" fn tutorial(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj // Cancelling the first screen will point to the last one, // which asks for confirmation whether user wants to // really cancel the tutorial. - let screen = match page_index { + match page_index { // title, text, btn_layout, btn_actions - 0 => ( - "HELLO", - "Welcome to Trezor.\nPress right to continue.", - ButtonLayout::cancel_and_arrow(), - ButtonActions::last_next(), - ), - 1 => ( - "", - "Use Trezor by clicking left & right.\n\nContinue right.", - ButtonLayout::left_right_arrows(), - ButtonActions::prev_next(), - ), - 2 => ( - "HOLD TO CONFIRM", - "Press & hold right to approve important operations.", - ButtonLayout::new( - Some(ButtonDetails::left_arrow_icon()), - None, - Some( - ButtonDetails::text("HOLD TO CONFIRM") - .with_duration(Duration::from_millis(2000)), - ), - ), - ButtonActions::prev_next(), - ), - 3 => ( - "SCREEN SCROLL", - "Press right to scroll down to read all content when text\ndoesn't fit on one screen. Press left to scroll up.", - ButtonLayout::new( - Some(ButtonDetails::left_arrow_icon()), - None, - Some(ButtonDetails::text("GOT IT")), ), - ButtonActions::prev_next(), - ), - 4 => ( - "CONFIRM", - "Press both left & right at the same time to confirm.", - ButtonLayout::new( - Some(ButtonDetails::left_arrow_icon()), - Some(ButtonDetails::armed_text("CONFIRM")), - None, - ), - ButtonActions::prev_next_with_middle(), - ), - 5 => ( - "CONGRATS!", - "You're ready to use Trezor.", - ButtonLayout::new( - Some(ButtonDetails::text("AGAIN")), - None, - Some(ButtonDetails::text("FINISH")), - ), - ButtonActions::beginning_confirm(), - ), - 6 => ( - "SKIP TUTORIAL", - "Sure you want to skip the tutorial?", - ButtonLayout::new( - Some(ButtonDetails::left_arrow_icon()), - None, - Some(ButtonDetails::text("SKIP")), - ), - ButtonActions::beginning_cancel(), - ), + 0 => { + tutorial_screen(( + "HELLO".into(), + "Welcome to Trezor.\nPress right to continue.".into(), + ButtonLayout::cancel_and_arrow(), + ButtonActions::last_next(), + )) + }, + 1 => { + tutorial_screen(( + "".into(), + "Use Trezor by clicking left and right.\n\nContinue right.".into(), + ButtonLayout::left_right_arrows(), + ButtonActions::prev_next(), + )) + }, + 2 => { + tutorial_screen(( + "HOLD TO CONFIRM".into(), + "Press and hold right to approve important operations.".into(), + ButtonLayout::back_and_htc_text("HOLD TO CONFIRM", Duration::from_millis(1000)), + ButtonActions::prev_next(), + )) + }, + 3 => { + tutorial_screen(( + "SCREEN SCROLL".into(), + "Press right to scroll down to read all content when text\ndoesn't fit on one screen. Press left to scroll up.".into(), + ButtonLayout::back_and_text("GOT IT"), + ButtonActions::prev_next(), + )) + }, + 4 => { + tutorial_screen(( + "CONFIRM".into(), + "Press both left and right at the same time to confirm.".into(), + ButtonLayout::middle_armed_text("CONFIRM"), + ButtonActions::prev_next_with_middle(), + )) + }, + // This page is special + 5 => { + Page::<10>::new( + ButtonLayout::left_right_text("AGAIN", "FINISH"), + ButtonActions::beginning_confirm(), + Font::MONO, + ) + .newline() + .text_mono("Tutorial complete.".into()) + .newline() + .newline() + .alignment(LineAlignment::Center) + .text_bold("You're ready to\nuse Trezor.".into()) + }, + 6 => { + tutorial_screen(( + "SKIP TUTORIAL".into(), + "Are you sure you want to skip the tutorial?".into(), + ButtonLayout::cancel_and_text("SKIP"), + ButtonActions::beginning_cancel(), + )) + }, _ => unreachable!(), - }; - - let mut page = Page::<10>::new(screen.2.clone(), screen.3.clone(), Font::BOLD); - - // Add title if present - if !screen.0.is_empty() { - page = page.text_bold(screen.0.into()).newline().newline_half() } - page = page.text_mono(screen.1.into()); - page }; let pages = FlowPages::new(get_page, PAGE_COUNT); @@ -532,6 +550,7 @@ extern "C" fn select_word(n_args: usize, args: *const Obj, kwargs: *mut Map) -> let words_iterable: Obj = kwargs.get(Qstr::MP_QSTR_words)?; let words: Vec = iter_into_vec(words_iterable)?; + // TODO: should return int, to be consistent with TT's select_word let obj = LayoutObj::new(Frame::new(title, SimpleChoice::new(words).into_child()))?; Ok(obj.into()) }; @@ -660,9 +679,8 @@ pub static mp_module_trezorui2: Module = obj_module! { /// *, /// title: str, /// words: Iterable[str], - /// ) -> int: - /// """Select mnemonic word from three possibilities - seed check after backup. The - /// iterable must be of exact size. Returns index in range `0..3`.""" + /// ) -> str: + /// """Select a word from a list. TODO: should return int, to be consistent with TT's select_word""" Qstr::MP_QSTR_select_word => obj_fn_kw!(0, select_word).as_obj(), /// def request_word_count( diff --git a/core/mocks/generated/trezorui2.pyi b/core/mocks/generated/trezorui2.pyi index b73c10c6ba..947cae5cba 100644 --- a/core/mocks/generated/trezorui2.pyi +++ b/core/mocks/generated/trezorui2.pyi @@ -121,9 +121,8 @@ def select_word( *, title: str, words: Iterable[str], -) -> int: - """Select mnemonic word from three possibilities - seed check after backup. The - iterable must be of exact size. Returns index in range `0..3`.""" +) -> str: + """Select a word from a list. TODO: should return int, to be consistent with TT's select_word""" # rust/src/ui/model_tr/layout.rs diff --git a/core/src/trezor/ui/layouts/tr/__init__.py b/core/src/trezor/ui/layouts/tr/__init__.py index e0f7df3f6d..3eecff5036 100644 --- a/core/src/trezor/ui/layouts/tr/__init__.py +++ b/core/src/trezor/ui/layouts/tr/__init__.py @@ -561,8 +561,8 @@ async def confirm_reset_device( return await _placeholder_confirm( ctx=ctx, br_type="recover_device" if recovery else "setup_device", - title="RECOVERY MODE" if recovery else "CREATE NEW WALLET", - data="By continuing you agree to trezor.io/tos", + title="START RECOVERY" if recovery else "CREATE NEW WALLET", + data="By continuing you agree to our terms and conditions.\nSee trezor.io/tos.", description=prompt, br_code=ButtonRequestType.ProtectCall if recovery diff --git a/core/src/trezor/ui/layouts/tr/reset.py b/core/src/trezor/ui/layouts/tr/reset.py index 34bd7a6ed3..b061e4a135 100644 --- a/core/src/trezor/ui/layouts/tr/reset.py +++ b/core/src/trezor/ui/layouts/tr/reset.py @@ -51,10 +51,10 @@ async def select_word( ) ) ) - if __debug__ and isinstance(result, str): - return result - assert isinstance(result, int) and 0 <= result <= 2 - return words[result] + for word in words: + if word.upper() == result: + return word + raise ValueError("Invalid word") async def slip39_show_checklist(