From ab8de63c5b2d007ba07cdb09224608a25713f8bb Mon Sep 17 00:00:00 2001 From: grdddj Date: Sat, 12 Nov 2022 16:23:39 +0100 Subject: [PATCH] WIP - use horizontal scrollbar at the top right --- core/embed/rust/src/ui/component/base.rs | 18 ++++ .../rust/src/ui/model_tr/component/button.rs | 12 ++- .../rust/src/ui/model_tr/component/common.rs | 8 +- .../rust/src/ui/model_tr/component/flow.rs | 2 +- .../rust/src/ui/model_tr/component/frame.rs | 12 ++- .../rust/src/ui/model_tr/component/page.rs | 65 +++++++++---- .../src/ui/model_tr/component/scrollbar.rs | 91 ++++++------------- core/src/apps/management/change_pin.py | 2 +- core/src/trezor/ui/layouts/tr/__init__.py | 6 +- 9 files changed, 121 insertions(+), 95 deletions(-) diff --git a/core/embed/rust/src/ui/component/base.rs b/core/embed/rust/src/ui/component/base.rs index 05a659eb22..c54e79f5f7 100644 --- a/core/embed/rust/src/ui/component/base.rs +++ b/core/embed/rust/src/ui/component/base.rs @@ -38,6 +38,20 @@ pub trait Component { /// No painting should be done in this phase. fn place(&mut self, bounds: Rect) -> Rect; + /// Define the available area for scrollbar, when it is outside of the + /// component itself (in the area of parent component). + /// + /// This area is the *MAXIMUM* area that scrollbar can occupy. + /// However, the scrollbar itself may be found smaller than this + /// (after the content is placed and we know the page count). + /// In this case, its area will/should be decreased and taken from the right + /// (to minimize the area that is affected - as scrollbar has a `Pad` + /// that is cleared periodically). + /// + /// Use-case is putting the scrollbar on the same line as the title in + /// `Frame` and operating this scrollbar from the Child component. + fn set_scrollbar_area(&mut self, _area: Rect) {} + /// React to an outside event. See the `Event` type for possible cases. /// /// Component should modify its internal state as a response to the event, @@ -131,6 +145,10 @@ where self.component.place(bounds) } + fn set_scrollbar_area(&mut self, area: Rect) { + self.component.set_scrollbar_area(area); + } + fn event(&mut self, ctx: &mut EventCtx, event: Event) -> Option { self.mutate(ctx, |ctx, c| { // Handle the internal invalidation event here, so components don't have to. We 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 5ccf709781..c801683d83 100644 --- a/core/embed/rust/src/ui/model_tr/component/button.rs +++ b/core/embed/rust/src/ui/model_tr/component/button.rs @@ -454,16 +454,24 @@ impl> ButtonDetails { .with_offset(Offset::new(2, -2)) } - /// Left arrow to signal going back. + /// Left arrow to signal going back. No outline. pub fn left_arrow_icon() -> Self { Self::icon(Icon::new(theme::ICON_ARROW_LEFT)).with_no_outline() } - /// Right arrow to signal going forward. + /// Right arrow to signal going forward. No outline. pub fn right_arrow_icon() -> Self { Self::icon(Icon::new(theme::ICON_ARROW_RIGHT)).with_no_outline() } + /// Up arrow to signal paginating back. No outline. Offsetted little right + /// to not be on the boundary. + pub fn up_arrow_icon() -> Self { + Self::icon(Icon::new(theme::ICON_ARROW_UP)) + .with_no_outline() + .with_offset(Offset::new(2, -3)) + } + /// Down arrow to signal paginating forward. Takes half the screen's width pub fn down_arrow_icon_wide() -> Self { Self::icon(Icon::new(theme::ICON_ARROW_DOWN)).force_width(HALF_SCREEN_BUTTON_WIDTH) 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 6e4506c0d5..ede29fa4bb 100644 --- a/core/embed/rust/src/ui/model_tr/component/common.rs +++ b/core/embed/rust/src/ui/model_tr/component/common.rs @@ -68,12 +68,12 @@ pub fn display_secret_center_top>(secret: T, offset_from_top: i16) } } -/// Display title/header centered at the top of the given area. +/// Display title/header at the top left of the given area. /// Returning the painted height of the whole header. -pub fn paint_header_centered>(title: T, area: Rect) -> i16 { +pub fn paint_header_left>(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( + let title_baseline = area.top_left() + Offset::y(text_heigth); + display::text_left( title_baseline, title.as_ref(), theme::FONT_HEADER, diff --git a/core/embed/rust/src/ui/model_tr/component/flow.rs b/core/embed/rust/src/ui/model_tr/component/flow.rs index acec7a7298..db52f4fcd2 100644 --- a/core/embed/rust/src/ui/model_tr/component/flow.rs +++ b/core/embed/rust/src/ui/model_tr/component/flow.rs @@ -208,7 +208,7 @@ where // (not compatible with longer/centered titles) self.pad.paint(); if let Some(title) = &self.common_title { - common::paint_header_centered(title, self.title_area); + common::paint_header_left(title, self.title_area); } self.current_page.paint(); self.buttons.paint(); 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 33fa741aa9..dfe114756a 100644 --- a/core/embed/rust/src/ui/model_tr/component/frame.rs +++ b/core/embed/rust/src/ui/model_tr/component/frame.rs @@ -1,4 +1,4 @@ -use super::{common, theme}; +use super::{common, theme, ScrollBar}; use crate::ui::{ component::{Child, Component, Event, EventCtx}, geometry::{Insets, Rect}, @@ -39,9 +39,15 @@ where fn place(&mut self, bounds: Rect) -> Rect { const TITLE_SPACE: i16 = 4; - let (title_area, content_area) = bounds.split_top(theme::FONT_HEADER.line_height()); + let (title_and_scrollbar_area, content_area) = + 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); + self.area = title_area; self.content.place(content_area); bounds @@ -52,7 +58,7 @@ where } fn paint(&mut self) { - common::paint_header_centered(&self.title, self.area); + common::paint_header_left(&self.title, self.area); self.content.paint(); } } 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 5b561b2090..06f4c9476f 100644 --- a/core/embed/rust/src/ui/model_tr/component/page.rs +++ b/core/embed/rust/src/ui/model_tr/component/page.rs @@ -3,7 +3,7 @@ use crate::{ ui::{ component::{Child, Component, ComponentExt, Event, EventCtx, Pad, PageMsg, Paginate}, display::Color, - geometry::{Insets, Rect}, + geometry::{Insets, Offset, Rect}, }, }; @@ -14,10 +14,18 @@ use super::{ pub struct ButtonPage { content: Child, scrollbar: Child, + /// Optional available area for scrollbar defined by parent component. + parent_scrollbar_area: Option, pad: Pad, + /// Left button of the first screen cancel_btn_details: Option>, + /// Right button of the last screen confirm_btn_details: Option>, + /// Left button of the last page + last_back_btn_details: Option>, + /// Left button of every screen in the middle back_btn_details: Option>, + /// Right button of every screen apart the last one next_btn_details: Option>, buttons: Child>, /// Scrollbar may or may not be shown (but will be counting pages anyway). @@ -33,11 +41,13 @@ where pub fn new_str(content: T, background: Color) -> Self { Self { content: Child::new(content), - scrollbar: Child::new(ScrollBar::vertical_to_be_filled_later()), + scrollbar: Child::new(ScrollBar::to_be_filled_later()), + parent_scrollbar_area: None, pad: Pad::with_background(background).with_clear(), cancel_btn_details: Some(ButtonDetails::cancel_icon()), confirm_btn_details: Some(ButtonDetails::text("CONFIRM")), back_btn_details: Some(ButtonDetails::up_arrow_icon_wide()), + last_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. // Initial button layout will be set in `place()` after we can call @@ -57,11 +67,13 @@ where pub fn new_str_buf(content: T, background: Color) -> Self { Self { content: Child::new(content), - scrollbar: Child::new(ScrollBar::vertical_to_be_filled_later()), + scrollbar: Child::new(ScrollBar::to_be_filled_later()), + parent_scrollbar_area: None, pad: Pad::with_background(background).with_clear(), cancel_btn_details: Some(ButtonDetails::cancel_icon()), confirm_btn_details: Some(ButtonDetails::text("CONFIRM".into())), back_btn_details: Some(ButtonDetails::up_arrow_icon_wide()), + last_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. // Initial button layout will be set in `place()` after we can call @@ -136,19 +148,25 @@ 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_left = self.get_left_button_details(!has_prev, !has_next); let btn_right = self.get_right_button_details(has_next); ButtonLayout::new(btn_left, None, btn_right) } - fn get_left_button_details(&self, has_prev_page: bool) -> Option> { - if has_prev_page { - self.back_btn_details.clone() - } else { + /// Get the let button details, depending whether the page is first, last, + /// or in the middle. + fn get_left_button_details(&self, is_first: bool, is_last: bool) -> Option> { + if is_first { self.cancel_btn_details.clone() + } else if is_last { + self.last_back_btn_details.clone() + } else { + self.back_btn_details.clone() } } + /// Get the right button details, depending on whether there is a next + /// page. fn get_right_button_details(&self, has_next_page: bool) -> Option> { if has_next_page { self.next_btn_details.clone() @@ -168,14 +186,7 @@ where type Msg = PageMsg; fn place(&mut self, bounds: Rect) -> Rect { - let (content_and_scrollbar_area, button_area) = bounds.split_bottom(theme::BUTTON_HEIGHT); - let (content_area, scrollbar_area) = { - if self.show_scrollbar { - content_and_scrollbar_area.split_right(ScrollBar::WIDTH) - } else { - (content_and_scrollbar_area, Rect::zero()) - } - }; + let (content_area, button_area) = bounds.split_bottom(theme::BUTTON_HEIGHT); let content_area = content_area.inset(Insets::top(1)); // Do not pad the button area nor the scrollbar, leave it to them self.pad.place(content_area); @@ -184,14 +195,34 @@ where // and we can calculate the page count let page_count = self.content.inner_mut().page_count(); self.scrollbar.inner_mut().set_page_count(page_count); + self.set_buttons_for_initial_page(page_count); + + // Placing the scrollbar when requested. + // Put it into its dedicated area when parent component already chose it, + // otherwise place it into the right top of the content. if self.show_scrollbar { + let scrollbar_area = if let Some(scrollbar_area) = self.parent_scrollbar_area { + scrollbar_area + } else { + Rect::from_top_right_and_size( + content_area.top_right(), + Offset::new( + -self.scrollbar.inner().overall_width(), + ScrollBar::MAX_DOT_SIZE, + ), + ) + }; self.scrollbar.place(scrollbar_area); } - self.set_buttons_for_initial_page(page_count); + self.buttons.place(button_area); bounds } + fn set_scrollbar_area(&mut self, area: Rect) { + self.parent_scrollbar_area = Some(area); + } + fn event(&mut self, ctx: &mut EventCtx, event: Event) -> Option { ctx.set_page_count(self.scrollbar.inner().page_count); let button_event = self.buttons.event(ctx, event); diff --git a/core/embed/rust/src/ui/model_tr/component/scrollbar.rs b/core/embed/rust/src/ui/model_tr/component/scrollbar.rs index 27ca055b76..62b8e148b6 100644 --- a/core/embed/rust/src/ui/model_tr/component/scrollbar.rs +++ b/core/embed/rust/src/ui/model_tr/component/scrollbar.rs @@ -5,46 +5,40 @@ use crate::ui::{ model_tr::theme, }; -/// In which direction should the scrollbar be positioned -pub enum ScrollbarOrientation { - Vertical, - Horizontal, -} - +/// Scrollbar to be painted horizontally at the top right of the screen. pub struct ScrollBar { area: Rect, pad: Pad, pub page_count: usize, pub active_page: usize, - pub orientation: ScrollbarOrientation, } impl ScrollBar { - pub const WIDTH: i16 = 8; - pub const DOT_SIZE: Offset = Offset::new(4, 4); - pub const DOT_INTERVAL: i16 = 6; + /// How many dots at most will there be + pub const MAX_DOTS: i16 = 5; + /// Maximum size (width/height) of a dot + pub const MAX_DOT_SIZE: i16 = 5; + /// Distance between two dots + pub const DOTS_DISTANCE: i16 = 2; + pub const DOTS_INTERVAL: i16 = Self::MAX_DOT_SIZE + Self::DOTS_DISTANCE; + pub const MAX_WIDTH: i16 = Self::DOTS_INTERVAL * Self::MAX_DOTS - Self::DOTS_DISTANCE; - pub fn new(page_count: usize, orientation: ScrollbarOrientation) -> Self { + pub fn new(page_count: usize) -> Self { Self { area: Rect::zero(), pad: Pad::with_background(theme::BG), page_count, active_page: 0, - orientation, } } /// Page count will be given later as it is not available yet. - pub fn vertical_to_be_filled_later() -> Self { - Self::vertical(0) + pub fn to_be_filled_later() -> Self { + Self::new(0) } - pub fn vertical(page_count: usize) -> Self { - Self::new(page_count, ScrollbarOrientation::Vertical) - } - - pub fn horizontal(page_count: usize) -> Self { - Self::new(page_count, ScrollbarOrientation::Horizontal) + pub fn overall_width(&self) -> i16 { + Self::DOTS_INTERVAL * self.page_count as i16 - Self::DOTS_DISTANCE } pub fn set_page_count(&mut self, page_count: usize) { @@ -74,8 +68,9 @@ impl ScrollBar { /// Create a (seemingly circular) dot given its top left point. /// Make it full when it is active, otherwise paint just the perimeter and /// leave center empty. - fn paint_dot(&self, active: bool, top_left: Point) { - let full_square = Rect::from_top_left_and_size(top_left, ScrollBar::DOT_SIZE); + fn paint_dot(&self, active: bool, top_right: Point) { + let full_square = + Rect::from_top_right_and_size(top_right, Offset::uniform(Self::MAX_DOT_SIZE)); // FG - painting the full square display::rect_fill(full_square, theme::FG); @@ -91,45 +86,15 @@ impl ScrollBar { } } - fn paint_vertical(&mut self) { - let count = self.page_count as i16; - let interval = { - let available_space = self.area.height(); - let naive_space = count * Self::DOT_INTERVAL; - if naive_space > available_space { - available_space / count - } else { - Self::DOT_INTERVAL - } - }; - let mut top_left = Point::new( - self.area.center().x - Self::DOT_SIZE.x / 2, - self.area.center().y - (count / 2) * interval, - ); - for i in 0..self.page_count { - self.paint_dot(i == self.active_page, top_left); - top_left.y += interval; - } - } - + /// Drawing the dots horizontally and aligning to the right fn paint_horizontal(&mut self) { - let count = self.page_count as i16; - let interval = { - let available_space = self.area.width(); - let naive_space = count * Self::DOT_INTERVAL; - if naive_space > available_space { - available_space / count - } else { - Self::DOT_INTERVAL - } - }; - let mut top_left = Point::new( - self.area.center().x - (count / 2) * interval, - self.area.center().y - Self::DOT_SIZE.y / 2, - ); - for i in 0..self.page_count { - self.paint_dot(i == self.active_page, top_left); - top_left.x += interval; + let mut top_right = self.area.top_right(); + // TODO: implement smaller dots - two more sizes + // TODO: implement showing at most MAX_DIGITS + for i in (0..self.page_count).rev() { + self.paint_dot(i == self.active_page, top_right); + top_right.x -= Self::DOTS_INTERVAL; + top_right.print(); } } } @@ -156,10 +121,6 @@ impl Component for ScrollBar { self.pad.clear(); self.pad.paint(); - if matches!(self.orientation, ScrollbarOrientation::Vertical) { - self.paint_vertical() - } else { - self.paint_horizontal() - } + self.paint_horizontal(); } } diff --git a/core/src/apps/management/change_pin.py b/core/src/apps/management/change_pin.py index 87b4c18881..399877c654 100644 --- a/core/src/apps/management/change_pin.py +++ b/core/src/apps/management/change_pin.py @@ -95,7 +95,7 @@ def _require_confirm_change_pin(ctx: Context, msg: ChangePin) -> Awaitable[None] "enable PIN protection?", [ "PIN will be used to access this device.", - "It should contain at least 4 digits.", + "It must contain at least 4 digits.", ], ) diff --git a/core/src/trezor/ui/layouts/tr/__init__.py b/core/src/trezor/ui/layouts/tr/__init__.py index 30c9dad362..ad686464d7 100644 --- a/core/src/trezor/ui/layouts/tr/__init__.py +++ b/core/src/trezor/ui/layouts/tr/__init__.py @@ -1359,12 +1359,14 @@ async def confirm_set_new_pin( ) information.append( - "Position of individual numbers will change between entries for enhanced security." + "Position of individual numbers will change between entries for more security." ) return await confirm_action( ctx, br_type, title="", - description="\n\n".join(information), + description="\n".join(information), + verb="HOLD TO BEGIN", + hold=True, br_code=br_code, )