From 0afc97227f1893a37fbe6d19bbcbab02d2525438 Mon Sep 17 00:00:00 2001 From: obrusvit Date: Sat, 10 May 2025 12:53:04 +0200 Subject: [PATCH] fix(eckhart): a few alignment and style issues - fix pyright --- .../ui/layout_eckhart/flow/confirm_output.rs | 54 +++++++++---------- .../ui/layout_eckhart/flow/confirm_summary.rs | 6 +-- .../layout_eckhart/flow/confirm_with_menu.rs | 14 +++-- .../src/ui/layout_eckhart/theme/firmware.rs | 4 +- .../rust/src/ui/layout_eckhart/theme/mod.rs | 1 + .../src/ui/layout_eckhart/ui_bootloader.rs | 15 +++--- .../rust/src/ui/layout_eckhart/ui_firmware.rs | 39 ++++++-------- .../src/trezor/ui/layouts/eckhart/__init__.py | 4 +- 8 files changed, 66 insertions(+), 71 deletions(-) diff --git a/core/embed/rust/src/ui/layout_eckhart/flow/confirm_output.rs b/core/embed/rust/src/ui/layout_eckhart/flow/confirm_output.rs index 1b64db4a98..19edd30c20 100644 --- a/core/embed/rust/src/ui/layout_eckhart/flow/confirm_output.rs +++ b/core/embed/rust/src/ui/layout_eckhart/flow/confirm_output.rs @@ -261,8 +261,7 @@ fn content_menu_info( paragraphs .map_or_else(ParagraphVecShort::new, |p| p) .into_paragraphs() - .with_placement(LinearPlacement::vertical()) - .with_spacing(12), + .with_placement(LinearPlacement::vertical().with_spacing(theme::PARAGRAPHS_SPACING)), ) .with_header(Header::new(title).with_close_button()) .with_subtitle(subtitle.unwrap_or(TString::empty())) @@ -302,8 +301,7 @@ pub fn new_confirm_output( let content_main = TextScreen::new( main_paragraphs .into_paragraphs() - .with_placement(LinearPlacement::vertical()) - .with_spacing(12), + .with_placement(LinearPlacement::vertical().with_spacing(theme::PARAGRAPHS_SPACING)), ) .with_header(Header::new(title.unwrap_or(TString::empty())).with_menu_button()) .with_action_bar(ActionBar::new_single(Button::with_text( @@ -399,31 +397,29 @@ pub fn new_confirm_output( flow } else if let Some(summary_paragraphs) = summary_paragraphs { // Summary - let content_summary = TextScreen::new( - summary_paragraphs - .into_paragraphs() - .with_placement(LinearPlacement::vertical()) - .with_spacing(12), - ) - .with_header( - Header::new(summary_title.unwrap_or(TR::words__title_summary.into())) - .with_menu_button(), - ) - .with_action_bar(ActionBar::new_double( - Button::with_icon(theme::ICON_CHEVRON_UP), - Button::with_text(TR::instructions__hold_to_sign.into()) - .with_long_press(theme::CONFIRM_HOLD_DURATION) - .styled(theme::button_confirm()), - )) - .map(|msg| match msg { - TextScreenMsg::Confirmed => Some(FlowMsg::Confirmed), - TextScreenMsg::Cancelled => Some(FlowMsg::Cancelled), - TextScreenMsg::Menu => Some(FlowMsg::Info), - }) - .one_button_request(ButtonRequest::from_num( - summary_br_code.unwrap(), - summary_br_name.unwrap(), - )); + let content_summary = + TextScreen::new(summary_paragraphs.into_paragraphs().with_placement( + LinearPlacement::vertical().with_spacing(theme::PARAGRAPHS_SPACING), + )) + .with_header( + Header::new(summary_title.unwrap_or(TR::words__title_summary.into())) + .with_menu_button(), + ) + .with_action_bar(ActionBar::new_double( + Button::with_icon(theme::ICON_CHEVRON_UP), + Button::with_text(TR::instructions__hold_to_sign.into()) + .with_long_press(theme::CONFIRM_HOLD_DURATION) + .styled(theme::button_confirm()), + )) + .map(|msg| match msg { + TextScreenMsg::Confirmed => Some(FlowMsg::Confirmed), + TextScreenMsg::Cancelled => Some(FlowMsg::Cancelled), + TextScreenMsg::Menu => Some(FlowMsg::Info), + }) + .one_button_request(ButtonRequest::from_num( + summary_br_code.unwrap(), + summary_br_name.unwrap(), + )); // SummaryMenu let mut summary_menu = VerticalMenu::::empty(); diff --git a/core/embed/rust/src/ui/layout_eckhart/flow/confirm_summary.rs b/core/embed/rust/src/ui/layout_eckhart/flow/confirm_summary.rs index 281c860a2f..57ceaef1a2 100644 --- a/core/embed/rust/src/ui/layout_eckhart/flow/confirm_summary.rs +++ b/core/embed/rust/src/ui/layout_eckhart/flow/confirm_summary.rs @@ -80,8 +80,7 @@ fn content_menu_info( paragraphs .map_or_else(ParagraphVecShort::new, |p| p) .into_paragraphs() - .with_placement(LinearPlacement::vertical()) - .with_spacing(12), + .with_placement(LinearPlacement::vertical().with_spacing(theme::PARAGRAPHS_SPACING)), ) .with_header(Header::new(title).with_close_button()) .with_subtitle(subtitle.unwrap_or(TString::empty())) @@ -115,8 +114,7 @@ pub fn new_confirm_summary( let content_summary = TextScreen::new( summary_paragraphs .into_paragraphs() - .with_placement(LinearPlacement::vertical()) - .with_spacing(12), + .with_placement(LinearPlacement::vertical().with_spacing(theme::PARAGRAPHS_SPACING)), ) .with_header(Header::new(title).with_menu_button()) .with_action_bar(ActionBar::new_single( diff --git a/core/embed/rust/src/ui/layout_eckhart/flow/confirm_with_menu.rs b/core/embed/rust/src/ui/layout_eckhart/flow/confirm_with_menu.rs index 7271eacd5b..a0ab0b0538 100644 --- a/core/embed/rust/src/ui/layout_eckhart/flow/confirm_with_menu.rs +++ b/core/embed/rust/src/ui/layout_eckhart/flow/confirm_with_menu.rs @@ -63,13 +63,17 @@ pub fn new_confirm_with_menu( cancel_menu_label: Option>, ) -> Result { let cancel_menu_label = cancel_menu_label.unwrap_or(TR::buttons__cancel.into()); - let confirm_label = confirm_label.unwrap_or(TR::buttons__confirm.into()); // Value - let mut confirm_button = Button::with_text(confirm_label).styled(theme::button_confirm()); - if hold { - confirm_button = confirm_button.with_long_press(theme::LOCK_HOLD_DURATION); - } + let confirm_button = if hold { + let confirm_label = confirm_label.unwrap_or(TR::buttons__hold_to_confirm.into()); + Button::with_text(confirm_label) + .styled(theme::button_confirm()) + .with_long_press(theme::LOCK_HOLD_DURATION) + } else { + let confirm_label = confirm_label.unwrap_or(TR::buttons__confirm.into()); + Button::with_text(confirm_label).styled(theme::button_default()) + }; let content_value = TextScreen::new(content) .with_header(Header::new(title).with_menu_button()) .with_action_bar(ActionBar::new_single(confirm_button)) diff --git a/core/embed/rust/src/ui/layout_eckhart/theme/firmware.rs b/core/embed/rust/src/ui/layout_eckhart/theme/firmware.rs index b9e379c9fa..9f9749c2f4 100644 --- a/core/embed/rust/src/ui/layout_eckhart/theme/firmware.rs +++ b/core/embed/rust/src/ui/layout_eckhart/theme/firmware.rs @@ -69,7 +69,7 @@ pub const TEXT_MONO_MEDIUM: TextStyle = TextStyle::new( GREY_EXTRA_LIGHT, GREY_EXTRA_LIGHT, ) -.with_line_breaking(LineBreaking::BreakWordsNoHyphen); +.with_line_breaking(LineBreaking::BreakAtWhitespace); pub const TEXT_MONO_MEDIUM_LIGHT: TextStyle = TextStyle::new( fonts::FONT_MONO_MEDIUM_38, @@ -78,7 +78,7 @@ pub const TEXT_MONO_MEDIUM_LIGHT: TextStyle = TextStyle::new( GREY_LIGHT, GREY_LIGHT, ) -.with_line_breaking(LineBreaking::BreakWordsNoHyphen); +.with_line_breaking(LineBreaking::BreakAtWhitespace); /// Roboto Mono Light - 30 (Address, data) pub const TEXT_MONO_LIGHT: TextStyle = TextStyle::new( diff --git a/core/embed/rust/src/ui/layout_eckhart/theme/mod.rs b/core/embed/rust/src/ui/layout_eckhart/theme/mod.rs index ef5ca33c83..410a8256b5 100644 --- a/core/embed/rust/src/ui/layout_eckhart/theme/mod.rs +++ b/core/embed/rust/src/ui/layout_eckhart/theme/mod.rs @@ -54,6 +54,7 @@ pub const PADDING: i16 = 24; // px pub const HEADER_HEIGHT: i16 = 96; // [px] pub const SIDE_INSETS: Insets = Insets::sides(PADDING); pub const ACTION_BAR_HEIGHT: i16 = 90; // [px] +pub const PARAGRAPHS_SPACING: i16 = 12; // px pub const TEXT_VERTICAL_SPACING: i16 = 24; // [px] // Tile pattern grid constants diff --git a/core/embed/rust/src/ui/layout_eckhart/ui_bootloader.rs b/core/embed/rust/src/ui/layout_eckhart/ui_bootloader.rs index 6071d970d8..4bfaad2937 100644 --- a/core/embed/rust/src/ui/layout_eckhart/ui_bootloader.rs +++ b/core/embed/rust/src/ui/layout_eckhart/ui_bootloader.rs @@ -38,8 +38,9 @@ pub type BootloaderString = String<128>; const RECONNECT_MESSAGE: &str = "Please reconnect\nthe device"; const SCREEN: Rect = UIEckhart::SCREEN; -// TODO: adjust offset -const PROGRESS_TEXT_ORIGIN: Point = SCREEN.top_left().ofs(Offset::new(24, 48)); +const PROGRESS_TEXT_ORIGIN: Point = SCREEN + .top_left() + .ofs(Offset::new(theme::PADDING, theme::HEADER_HEIGHT)); const SCREEN_BORDER_BLUE: ScreenBorder = ScreenBorder::new(BLUE); const SCREEN_BORDER_RED: ScreenBorder = ScreenBorder::new(RED); @@ -57,17 +58,17 @@ impl UIEckhart { display::sync(); render_on_display(None, Some(bg_color), |target| { - shape::Text::new(PROGRESS_TEXT_ORIGIN, text, fonts::FONT_SATOSHI_REGULAR_38) - .with_align(Alignment::Start) - .with_fg(BLD_FG) - .render(target); - let border: &ScreenBorder = match bg_color { RED => &SCREEN_BORDER_RED, _ => &SCREEN_BORDER_BLUE, }; render_loader(progress, border, target); + shape::Text::new(PROGRESS_TEXT_ORIGIN, text, fonts::FONT_SATOSHI_REGULAR_38) + .with_align(Alignment::Start) + .with_fg(BLD_FG) + .render(target); + if let Some(center_text) = center_text { shape::Text::new(SCREEN.center(), center_text, fonts::FONT_SATOSHI_REGULAR_38) .with_align(Alignment::Center) diff --git a/core/embed/rust/src/ui/layout_eckhart/ui_firmware.rs b/core/embed/rust/src/ui/layout_eckhart/ui_firmware.rs index 0d6c9f51a3..02ea813a5f 100644 --- a/core/embed/rust/src/ui/layout_eckhart/ui_firmware.rs +++ b/core/embed/rust/src/ui/layout_eckhart/ui_firmware.rs @@ -225,10 +225,9 @@ impl FirmwareUI for UIEckhart { let flow = flow::new_confirm_with_menu( title, None, - paragraphs - .into_paragraphs() - .with_spacing(12) - .with_placement(LinearPlacement::vertical()), + paragraphs.into_paragraphs().with_placement( + LinearPlacement::vertical().with_spacing(theme::PARAGRAPHS_SPACING), + ), None, false, Some(TR::words__title_information.into()), @@ -256,12 +255,9 @@ impl FirmwareUI for UIEckhart { ]); let layout = RootComponent::new( - TextScreen::new( - paragraphs - .into_paragraphs() - .with_placement(LinearPlacement::vertical()) - .with_spacing(12), - ) + TextScreen::new(paragraphs.into_paragraphs().with_placement( + LinearPlacement::vertical().with_spacing(theme::PARAGRAPHS_SPACING), + )) .with_header(Header::new(TR::modify_amount__title.into())) .with_action_bar(ActionBar::new_double( Button::with_icon(theme::ICON_CROSS), @@ -352,10 +348,9 @@ impl FirmwareUI for UIEckhart { let flow = flow::new_confirm_with_menu( title, None, - paragraphs - .into_paragraphs() - .with_spacing(12) - .with_placement(LinearPlacement::vertical()), + paragraphs.into_paragraphs().with_placement( + LinearPlacement::vertical().with_spacing(theme::PARAGRAPHS_SPACING), + ), None, hold, None, @@ -869,7 +864,9 @@ impl FirmwareUI for UIEckhart { theme::ICON_CHEVRON_RIGHT_MINI, theme::ICON_CHECKMARK_MINI, active, - paragraphs.into_paragraphs().with_spacing(40), + paragraphs + .into_paragraphs() + .with_placement(LinearPlacement::vertical().with_spacing(40)), ) .with_check_width(32) .with_icon_done_color(theme::GREEN_LIGHT) @@ -1060,13 +1057,11 @@ impl FirmwareUI for UIEckhart { } } - let screen = TextScreen::new( - paragraphs - .into_paragraphs() - .with_spacing(12) - .with_placement(LinearPlacement::vertical()), - ) - .with_header(Header::new(title).with_close_button()); + let screen = + TextScreen::new(paragraphs.into_paragraphs().with_placement( + LinearPlacement::vertical().with_spacing(theme::PARAGRAPHS_SPACING), + )) + .with_header(Header::new(title).with_close_button()); let layout = RootComponent::new(screen); Ok(layout) } diff --git a/core/src/trezor/ui/layouts/eckhart/__init__.py b/core/src/trezor/ui/layouts/eckhart/__init__.py index 38a21bed82..223de75b90 100644 --- a/core/src/trezor/ui/layouts/eckhart/__init__.py +++ b/core/src/trezor/ui/layouts/eckhart/__init__.py @@ -896,7 +896,7 @@ if not utils.BITCOIN_ONLY: account_path: str, vote_account: str, stake_item: tuple[str, str] | None, - amount_item: tuple[str, str], + amount_item: tuple[str, str] | None, fee_item: tuple[str, str], fee_details: Iterable[tuple[str, str]], blockhash_item: tuple[str, str], @@ -922,7 +922,7 @@ if not utils.BITCOIN_ONLY: extra_item=blockhash_item, fee_items=fee_details, summary_title=title, - summary_items=(amount_item, fee_item), + summary_items=(amount_item, fee_item) if amount_item else (fee_item,), summary_br_name="confirm_total", summary_br_code=ButtonRequestType.SignTx, cancel_text=TR.buttons__cancel,