From 5052594789253f57c388d361183f1fb1759de533 Mon Sep 17 00:00:00 2001 From: Martin Milata Date: Tue, 30 Aug 2022 14:05:33 +0200 Subject: [PATCH] refactor(core/rust/ui): simplify button height computation [no changelog] --- core/embed/rust/src/ui/component/mod.rs | 2 +- core/embed/rust/src/ui/component/placed.rs | 44 ++++++++++++ .../rust/src/ui/model_tt/component/button.rs | 66 +++++++++--------- .../rust/src/ui/model_tt/component/dialog.rs | 38 ++++------- .../ui/model_tt/component/hold_to_confirm.rs | 32 ++++----- .../src/ui/model_tt/component/number_input.rs | 2 +- .../rust/src/ui/model_tt/component/page.rs | 67 +++++++++---------- core/embed/rust/src/ui/model_tt/layout.rs | 26 +++---- core/embed/rust/src/ui/model_tt/theme.rs | 15 +++++ 9 files changed, 168 insertions(+), 124 deletions(-) diff --git a/core/embed/rust/src/ui/component/mod.rs b/core/embed/rust/src/ui/component/mod.rs index 3b2b60321..915fb0b59 100644 --- a/core/embed/rust/src/ui/component/mod.rs +++ b/core/embed/rust/src/ui/component/mod.rs @@ -23,7 +23,7 @@ pub use maybe::Maybe; pub use pad::Pad; pub use paginated::{PageMsg, Paginate}; pub use painter::{qrcode_painter, Painter}; -pub use placed::GridPlaced; +pub use placed::{FixedHeightBar, GridPlaced}; pub use text::{ formatted::FormattedText, layout::{LineBreaking, PageBreaking, TextLayout}, diff --git a/core/embed/rust/src/ui/component/placed.rs b/core/embed/rust/src/ui/component/placed.rs index a80465ff9..8adfbb0de 100644 --- a/core/embed/rust/src/ui/component/placed.rs +++ b/core/embed/rust/src/ui/component/placed.rs @@ -77,3 +77,47 @@ where d.close(); } } + +pub struct FixedHeightBar { + inner: T, + height: i32, +} + +impl FixedHeightBar { + pub const fn bottom(inner: T, height: i32) -> Self { + Self { inner, height } + } +} + +impl Component for FixedHeightBar +where + T: Component, +{ + type Msg = T::Msg; + + fn place(&mut self, bounds: Rect) -> Rect { + let (_, bar) = bounds.split_bottom(self.height); + self.inner.place(bar) + } + + fn event(&mut self, ctx: &mut EventCtx, event: Event) -> Option { + self.inner.event(ctx, event) + } + + fn paint(&mut self) { + self.inner.paint() + } +} + +#[cfg(feature = "ui_debug")] +impl crate::trace::Trace for FixedHeightBar +where + T: Component, + T: crate::trace::Trace, +{ + fn trace(&self, d: &mut dyn crate::trace::Tracer) { + d.open("FixedHeightBar"); + d.field("inner", &self.inner); + d.close(); + } +} diff --git a/core/embed/rust/src/ui/model_tt/component/button.rs b/core/embed/rust/src/ui/model_tt/component/button.rs index 7e79f593a..876d219bb 100644 --- a/core/embed/rust/src/ui/model_tt/component/button.rs +++ b/core/embed/rust/src/ui/model_tt/component/button.rs @@ -1,7 +1,9 @@ use crate::{ time::Duration, ui::{ - component::{Component, ComponentExt, Event, EventCtx, GridPlaced, Map, TimerToken}, + component::{ + Component, ComponentExt, Event, EventCtx, FixedHeightBar, GridPlaced, Map, TimerToken, + }, display::{self, Color, Font}, event::TouchEvent, geometry::{Insets, Offset, Rect}, @@ -27,9 +29,6 @@ pub struct Button { } impl Button { - /// Standard height in pixels. - pub const HEIGHT: i32 = 38; - /// Offsets the baseline of the button text either up (negative) or down /// (positive). pub const BASELINE_OFFSET: i32 = -3; @@ -353,7 +352,7 @@ impl Button { T: AsRef, { let columns = 1 + right_size_factor; - ( + theme::button_bar(( GridPlaced::new(left) .with_grid(1, columns) .with_spacing(theme::BUTTON_SPACING) @@ -368,7 +367,7 @@ impl Button { .map(|msg| { (matches!(msg, ButtonMsg::Clicked)).then(|| CancelConfirmMsg::Confirmed) }), - ) + )) } pub fn cancel_confirm_text( @@ -407,26 +406,31 @@ impl Button { let right = Button::with_text(confirm).styled(theme::button_confirm()); let top = Button::with_text(info); let left = Button::with_icon(theme::ICON_CANCEL); - ( - GridPlaced::new(left) - .with_grid(2, 3) - .with_spacing(theme::BUTTON_SPACING) - .with_row_col(1, 0) - .map(|msg| { - (matches!(msg, ButtonMsg::Clicked)).then(|| CancelInfoConfirmMsg::Cancelled) - }), - GridPlaced::new(top) - .with_grid(2, 3) - .with_spacing(theme::BUTTON_SPACING) - .with_from_to((0, 0), (0, 2)) - .map(|msg| (matches!(msg, ButtonMsg::Clicked)).then(|| CancelInfoConfirmMsg::Info)), - GridPlaced::new(right) - .with_grid(2, 3) - .with_spacing(theme::BUTTON_SPACING) - .with_from_to((1, 1), (1, 2)) - .map(|msg| { - (matches!(msg, ButtonMsg::Clicked)).then(|| CancelInfoConfirmMsg::Confirmed) - }), + theme::button_bar_rows( + 2, + ( + GridPlaced::new(left) + .with_grid(2, 3) + .with_spacing(theme::BUTTON_SPACING) + .with_row_col(1, 0) + .map(|msg| { + (matches!(msg, ButtonMsg::Clicked)).then(|| CancelInfoConfirmMsg::Cancelled) + }), + GridPlaced::new(top) + .with_grid(2, 3) + .with_spacing(theme::BUTTON_SPACING) + .with_from_to((0, 0), (0, 2)) + .map(|msg| { + (matches!(msg, ButtonMsg::Clicked)).then(|| CancelInfoConfirmMsg::Info) + }), + GridPlaced::new(right) + .with_grid(2, 3) + .with_spacing(theme::BUTTON_SPACING) + .with_from_to((1, 1), (1, 2)) + .map(|msg| { + (matches!(msg, ButtonMsg::Clicked)).then(|| CancelInfoConfirmMsg::Confirmed) + }), + ), ) } @@ -452,25 +456,25 @@ impl Button { }; let [top, middle, bottom] = words; - (btn(0, top), btn(1, middle), btn(2, bottom)) + theme::button_bar_rows(3, (btn(0, top), btn(1, middle), btn(2, bottom))) } } -type CancelConfirm = ( +type CancelConfirm = FixedHeightBar<( Map>, F0>, Map>, F1>, -); +)>; pub enum CancelConfirmMsg { Cancelled, Confirmed, } -type CancelInfoConfirm = ( +type CancelInfoConfirm = FixedHeightBar<( Map>, F0>, Map>, F1>, Map>, F2>, -); +)>; pub enum CancelInfoConfirmMsg { Cancelled, diff --git a/core/embed/rust/src/ui/model_tt/component/dialog.rs b/core/embed/rust/src/ui/model_tt/component/dialog.rs index 580b49f2c..89df49576 100644 --- a/core/embed/rust/src/ui/model_tt/component/dialog.rs +++ b/core/embed/rust/src/ui/model_tt/component/dialog.rs @@ -3,7 +3,7 @@ use crate::ui::{ geometry::{Insets, LinearPlacement, Rect}, }; -use super::{theme, Button}; +use super::theme; pub enum DialogMsg { Content(T), @@ -40,9 +40,12 @@ where type Msg = DialogMsg; fn place(&mut self, bounds: Rect) -> Rect { - let layout = DialogLayout::middle(bounds); - self.content.place(layout.content); - self.controls.place(layout.controls); + let controls_area = self.controls.place(bounds); + let content_area = bounds + .inset(Insets::bottom(controls_area.height())) + .inset(Insets::bottom(theme::BUTTON_SPACING)) + .inset(Insets::left(theme::CONTENT_BORDER)); + self.content.place(content_area); bounds } @@ -64,21 +67,6 @@ where } } -pub struct DialogLayout { - pub content: Rect, - pub controls: Rect, -} - -impl DialogLayout { - pub fn middle(area: Rect) -> Self { - let (content, controls) = area.split_bottom(Button::<&str>::HEIGHT); - let content = content - .inset(Insets::bottom(theme::BUTTON_SPACING)) - .inset(Insets::left(theme::CONTENT_BORDER)); - Self { content, controls } - } -} - #[cfg(feature = "ui_debug")] impl crate::trace::Trace for Dialog where @@ -145,12 +133,14 @@ where let bounds = bounds .inset(theme::borders()) .inset(Insets::top(Self::ICON_AREA_PADDING)); - let (content, buttons) = bounds.split_bottom(Button::<&str>::HEIGHT); - let (image, content) = content.split_top(Self::ICON_AREA_HEIGHT); - self.image.place(image); - self.paragraphs.place(content); - self.controls.place(buttons); + let controls_area = self.controls.place(bounds); + let content_area = bounds.inset(Insets::bottom(controls_area.height())); + + let (image_area, content_area) = content_area.split_top(Self::ICON_AREA_HEIGHT); + + self.image.place(image_area); + self.paragraphs.place(content_area); bounds } diff --git a/core/embed/rust/src/ui/model_tt/component/hold_to_confirm.rs b/core/embed/rust/src/ui/model_tt/component/hold_to_confirm.rs index 2b1a8fc38..52775ccff 100644 --- a/core/embed/rust/src/ui/model_tt/component/hold_to_confirm.rs +++ b/core/embed/rust/src/ui/model_tt/component/hold_to_confirm.rs @@ -1,9 +1,8 @@ use crate::{ time::Instant, ui::{ - component::{Child, Component, ComponentExt, Event, EventCtx, Pad}, - geometry::{Grid, Rect}, - model_tt::component::DialogLayout, + component::{Child, Component, ComponentExt, Event, EventCtx, FixedHeightBar, Pad}, + geometry::{Grid, Insets, Rect}, }, }; @@ -18,7 +17,7 @@ pub enum HoldToConfirmMsg { pub struct HoldToConfirm { loader: Loader, content: Child, - buttons: CancelHold, + buttons: FixedHeightBar, pad: Pad, } @@ -47,11 +46,14 @@ where type Msg = HoldToConfirmMsg; fn place(&mut self, bounds: Rect) -> Rect { - let layout = DialogLayout::middle(bounds); - self.pad.place(layout.content); - self.loader.place(layout.content); - self.content.place(layout.content); - self.buttons.place(layout.controls); + let controls_area = self.buttons.place(bounds); + let content_area = bounds + .inset(Insets::bottom(controls_area.height())) + .inset(Insets::bottom(theme::BUTTON_SPACING)) + .inset(Insets::left(theme::CONTENT_BORDER)); + self.pad.place(content_area); + self.loader.place(content_area); + self.content.place(content_area); bounds } @@ -121,18 +123,18 @@ pub enum CancelHoldMsg { } impl CancelHold { - pub fn new() -> Self { - Self { + pub fn new() -> FixedHeightBar { + theme::button_bar(Self { cancel: Some(Button::with_icon(theme::ICON_CANCEL)), hold: Button::with_text("HOLD TO CONFIRM").styled(theme::button_confirm()), - } + }) } - pub fn without_cancel() -> Self { - Self { + pub fn without_cancel() -> FixedHeightBar { + theme::button_bar(Self { cancel: None, hold: Button::with_text("HOLD TO CONFIRM").styled(theme::button_confirm()), - } + }) } } diff --git a/core/embed/rust/src/ui/model_tt/component/number_input.rs b/core/embed/rust/src/ui/model_tt/component/number_input.rs index ca7caf641..7207ea54c 100644 --- a/core/embed/rust/src/ui/model_tt/component/number_input.rs +++ b/core/embed/rust/src/ui/model_tt/component/number_input.rs @@ -71,7 +71,7 @@ where fn place(&mut self, bounds: Rect) -> Rect { self.area = bounds; - let button_height = Button::<&str>::HEIGHT; + let button_height = theme::BUTTON_HEIGHT; let content_area = self.area.inset(Insets::top(2 * theme::BUTTON_SPACING)); let (input_area, content_area) = content_area.split_top(button_height); let (content_area, button_area) = content_area.split_bottom(button_height); diff --git a/core/embed/rust/src/ui/model_tt/component/page.rs b/core/embed/rust/src/ui/model_tt/component/page.rs index 10e99094c..3ebde2d45 100644 --- a/core/embed/rust/src/ui/model_tt/component/page.rs +++ b/core/embed/rust/src/ui/model_tt/component/page.rs @@ -1,14 +1,15 @@ use crate::ui::{ component::{ - base::ComponentExt, paginated::PageMsg, Component, Event, EventCtx, Label, Pad, Paginate, + base::ComponentExt, paginated::PageMsg, Component, Event, EventCtx, FixedHeightBar, Label, + Pad, Paginate, }, display::{self, Color}, - geometry::Rect, + geometry::{Insets, Rect}, }; use super::{ hold_to_confirm::{handle_hold_event, CancelHold, CancelHoldMsg}, - theme, Button, CancelConfirmMsg, Loader, ScrollBar, Swipe, SwipeDirection, + theme, CancelConfirmMsg, Loader, ScrollBar, Swipe, SwipeDirection, }; pub struct SwipePage { @@ -19,7 +20,6 @@ pub struct SwipePage { scrollbar: ScrollBar, hint: Label<&'static str>, fade: Option, - button_rows: i32, } impl SwipePage @@ -37,15 +37,9 @@ where pad: Pad::with_background(background), hint: Label::centered("SWIPE TO CONTINUE", theme::label_page_hint()), fade: None, - button_rows: 1, } } - pub fn with_button_rows(mut self, rows: usize) -> Self { - self.button_rows = rows as i32; - self - } - fn setup_swipe(&mut self) { self.swipe.allow_up = self.scrollbar.has_next_page(); self.swipe.allow_down = self.scrollbar.has_previous_page(); @@ -76,20 +70,27 @@ where type Msg = PageMsg; fn place(&mut self, bounds: Rect) -> Rect { - let layout = PageLayout::new(bounds, self.button_rows); + let layout = PageLayout::new(bounds); self.pad.place(bounds); self.swipe.place(bounds); self.hint.place(layout.hint); - self.buttons.place(layout.buttons); - self.scrollbar.place(layout.scrollbar); + let buttons_area = self.buttons.place(layout.buttons); + let buttons_height = buttons_area.height() + theme::BUTTON_SPACING; + self.scrollbar + .place(layout.scrollbar.inset(Insets::bottom(buttons_height))); // Layout the content. Try to fit it on a single page first, and reduce the area // to make space for a scrollbar if it doesn't fit. - self.content.place(layout.content_single_page); + self.content.place( + layout + .content_single_page + .inset(Insets::bottom(buttons_height)), + ); let page_count = { let count = self.content.page_count(); if count > 1 { - self.content.place(layout.content); + self.content + .place(layout.content.inset(Insets::bottom(buttons_height))); self.content.page_count() // Make sure to re-count it with the // new size. } else { @@ -197,14 +198,10 @@ impl PageLayout { const SCROLLBAR_SPACE: i32 = 10; const HINT_OFF: i32 = 19; - pub fn new(area: Rect, button_rows: i32) -> Self { - let buttons_height = button_rows * Button::<&str>::HEIGHT - + button_rows.saturating_sub(1) * theme::BUTTON_SPACING; - let (content, buttons) = area.split_bottom(buttons_height); + pub fn new(area: Rect) -> Self { let (_, hint) = area.split_bottom(Self::HINT_OFF); - let (content, _space) = content.split_bottom(theme::BUTTON_SPACING); - let (buttons, _space) = buttons.split_right(theme::CONTENT_BORDER); - let (_space, content) = content.split_left(theme::CONTENT_BORDER); + let (buttons, _space) = area.split_right(theme::CONTENT_BORDER); + let (_space, content) = area.split_left(theme::CONTENT_BORDER); let (content_single_page, _space) = content.split_right(theme::CONTENT_BORDER); let (content, scrollbar) = content.split_right(Self::SCROLLBAR_SPACE + Self::SCROLLBAR_WIDTH); @@ -221,7 +218,7 @@ impl PageLayout { } pub struct SwipeHoldPage { - inner: SwipePage, + inner: SwipePage>, loader: Loader, } @@ -328,7 +325,7 @@ mod tests { component::{text::paragraphs::Paragraphs, Empty}, event::TouchEvent, geometry::Point, - model_tt::{constant, theme}, + model_tt::{component::Button, constant, theme}, }, }; @@ -418,13 +415,13 @@ mod tests { theme::TEXT_BOLD, "This is somewhat long paragraph that goes on and on and on and on and on and will definitely not fit on just a single screen. You have to swipe a bit to see all the text it contains I guess. There's just so much letters in it.", ), - Empty, + theme::button_bar(Button::with_text("NO")), theme::BG, ); page.place(SCREEN); - let expected1 = " buttons: >"; - let expected2 = " buttons: >"; + let expected1 = " buttons: > >"; + let expected2 = " buttons: > >"; assert_eq!(trace(&page), expected1); swipe_down(&mut page); @@ -453,14 +450,14 @@ mod tests { theme::TEXT_BOLD, "Let's add another one for a good measure. This one should overflow all the way to the third page with a bit of luck.", ), - Empty, + theme::button_bar(Button::with_text("IDK")), theme::BG, ); page.place(SCREEN); - let expected1 = " buttons: >"; - let expected2 = " buttons: >"; - let expected3 = " buttons: >"; + let expected1 = " buttons: > >"; + let expected2 = " buttons: > >"; + let expected3 = " buttons: > >"; assert_eq!(trace(&page), expected1); swipe_down(&mut page); @@ -489,14 +486,14 @@ mod tests { .add_break() .add(theme::TEXT_NORMAL, "Short three.") .add_break(), - Empty, + theme::button_bar(Empty), theme::BG, ); page.place(SCREEN); - let expected1 = " buttons: >"; - let expected2 = " buttons: >"; - let expected3 = " buttons: >"; + let expected1 = " buttons: > >"; + let expected2 = " buttons: > >"; + let expected3 = " buttons: > >"; assert_eq!(trace(&page), expected1); swipe_up(&mut page); diff --git a/core/embed/rust/src/ui/model_tt/layout.rs b/core/embed/rust/src/ui/model_tt/layout.rs index c3a5c00df..cd0ca271a 100644 --- a/core/embed/rust/src/ui/model_tt/layout.rs +++ b/core/embed/rust/src/ui/model_tt/layout.rs @@ -586,9 +586,9 @@ extern "C" fn new_show_simple(n_args: usize, args: *const Obj, kwargs: *mut Map) t, Dialog::new( Paragraphs::new().add(theme::TEXT_NORMAL, description), - Button::with_text(button).map(|msg| { + theme::button_bar(Button::with_text(button).map(|msg| { (matches!(msg, ButtonMsg::Clicked)).then(|| CancelConfirmMsg::Confirmed) - }), + })), ), ))? .into() @@ -597,9 +597,9 @@ extern "C" fn new_show_simple(n_args: usize, args: *const Obj, kwargs: *mut Map) theme::borders(), Dialog::new( Paragraphs::new().add(theme::TEXT_NORMAL, description), - Button::with_text(button).map(|msg| { + theme::button_bar(Button::with_text(button).map(|msg| { (matches!(msg, ButtonMsg::Clicked)).then(|| CancelConfirmMsg::Confirmed) - }), + })), ), ))? .into() @@ -629,11 +629,7 @@ extern "C" fn new_confirm_with_info(n_args: usize, args: *const Obj, kwargs: *mu let buttons = Button::cancel_info_confirm(button, info_button); let obj = LayoutObj::new( - Frame::new( - title, - SwipePage::new(paragraphs, buttons, theme::BG).with_button_rows(2), - ) - .into_child(), + Frame::new(title, SwipePage::new(paragraphs, buttons, theme::BG)).into_child(), )?; Ok(obj.into()) }; @@ -729,11 +725,7 @@ extern "C" fn new_select_word(n_args: usize, args: *const Obj, kwargs: *mut Map) let buttons = Button::select_word(words); let obj = LayoutObj::new( - Frame::new( - title, - SwipePage::new(paragraphs, buttons, theme::BG).with_button_rows(3), - ) - .into_child(), + Frame::new(title, SwipePage::new(paragraphs, buttons, theme::BG)).into_child(), )?; Ok(obj.into()) }; @@ -821,9 +813,9 @@ extern "C" fn new_show_checklist(n_args: usize, args: *const Obj, kwargs: *mut M active, paragraphs, ), - Button::with_text(button).map(|msg| { + theme::button_bar(Button::with_text(button).map(|msg| { (matches!(msg, ButtonMsg::Clicked)).then(|| CancelConfirmMsg::Confirmed) - }), + })), ), ) .with_border(theme::borders()) @@ -1115,7 +1107,7 @@ mod tests { layout.place(SCREEN); assert_eq!( trace(&layout), - " controls: > 1: > > >", + " controls: > 1: > > > >", ) } } diff --git a/core/embed/rust/src/ui/model_tt/theme.rs b/core/embed/rust/src/ui/model_tt/theme.rs index ebd075724..4b31b03a9 100644 --- a/core/embed/rust/src/ui/model_tt/theme.rs +++ b/core/embed/rust/src/ui/model_tt/theme.rs @@ -2,6 +2,7 @@ use crate::ui::{ component::{ label::LabelStyle, text::{formatted::FormattedFonts, TextStyle}, + FixedHeightBar, }, display::{Color, Font}, geometry::Insets, @@ -407,8 +408,22 @@ pub const FORMATTED: FormattedFonts = FormattedFonts { pub const CONTENT_BORDER: i32 = 5; pub const KEYBOARD_SPACING: i32 = 8; +pub const BUTTON_HEIGHT: i32 = 38; pub const BUTTON_SPACING: i32 = 6; pub const CHECKLIST_SPACING: i32 = 10; +/// Standard button height in pixels. +pub const fn button_rows(count: usize) -> i32 { + let count = count as i32; + BUTTON_HEIGHT * count + BUTTON_SPACING * count.saturating_sub(1) +} + +pub const fn button_bar_rows(rows: usize, inner: T) -> FixedHeightBar { + FixedHeightBar::bottom(inner, button_rows(rows)) +} + +pub const fn button_bar(inner: T) -> FixedHeightBar { + button_bar_rows(1, inner) +} /// +----------+ /// | 13 |