Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI tweaks for ETH contract flow #4346

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open
1 change: 1 addition & 0 deletions core/embed/rust/librust_qstr.h
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ static void _librust_qstrs(void) {
MP_QSTR_notification;
MP_QSTR_notification_level;
MP_QSTR_page_count;
MP_QSTR_page_counter;
MP_QSTR_page_limit;
MP_QSTR_pages;
MP_QSTR_paint;
Expand Down
2 changes: 1 addition & 1 deletion core/embed/rust/src/ui/component/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ where
}

impl<T: Paginate> Paginate for Child<T> {
fn page_count(&mut self) -> usize {
fn page_count(&self) -> usize {
self.component.page_count()
}

Expand Down
2 changes: 1 addition & 1 deletion core/embed/rust/src/ui/component/paginated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub enum PageMsg<T> {

pub trait Paginate {
/// How many pages of content are there in total?
fn page_count(&mut self) -> usize;
fn page_count(&self) -> usize;
/// Navigate to the given page.
fn change_page(&mut self, active_page: usize);
}
27 changes: 14 additions & 13 deletions core/embed/rust/src/ui/component/text/formatted.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ impl FormattedText {
.layout_ops(self.char_offset, Offset::y(self.y_offset), sink)
}

pub(crate) fn layout_content_with_offset(
matejcik marked this conversation as resolved.
Show resolved Hide resolved
&self,
sink: &mut dyn LayoutSink,
char_offset: usize,
y_offset: i16,
) -> LayoutFit {
self.op_layout
.layout_ops(char_offset, Offset::y(y_offset), sink)
}

fn align_vertically(&mut self, content_height: i16) {
let bounds_height = self.op_layout.layout.bounds.height();
if content_height >= bounds_height {
Expand All @@ -53,20 +63,15 @@ impl FormattedText {

// Pagination
impl Paginate for FormattedText {
fn page_count(&mut self) -> usize {
fn page_count(&self) -> usize {
let mut page_count = 1; // There's always at least one page.

// Make sure we're starting page counting from the very beginning
// (but remembering the offsets not to change them).
let initial_y_offset = self.y_offset;
let initial_char_offset = self.char_offset;
self.char_offset = 0;
self.y_offset = 0;
let mut char_offset = 0;

// Looping through the content and counting pages
// until we finally fit.
loop {
let fit = self.layout_content(&mut TextNoOp);
let fit = self.layout_content_with_offset(&mut TextNoOp, char_offset, 0);
match fit {
LayoutFit::Fitting { .. } => {
break;
Expand All @@ -75,15 +80,11 @@ impl Paginate for FormattedText {
processed_chars, ..
} => {
page_count += 1;
self.char_offset += processed_chars;
char_offset += processed_chars;
}
}
}

// Setting the offsets back to the initial values.
self.char_offset = initial_char_offset;
self.y_offset = initial_y_offset;

page_count
}

Expand Down
6 changes: 3 additions & 3 deletions core/embed/rust/src/ui/component/text/paragraphs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ impl<'a, T> Paginate for Paragraphs<T>
where
T: ParagraphSource<'a>,
{
fn page_count(&mut self) -> usize {
fn page_count(&self) -> usize {
// There's always at least one page.
self.break_pages().count().max(1)
}
Expand Down Expand Up @@ -374,7 +374,7 @@ struct PageOffset {
}

impl PageOffset {
/// Given an `PageOffset` and a `Rect` area, returns:
/// Given a `PageOffset` and a `Rect` area, returns:
///
/// - The next offset.
/// - Part of `area` that remains free after the current offset is rendered
Expand Down Expand Up @@ -690,7 +690,7 @@ impl<'a, T> Paginate for Checklist<T>
where
T: ParagraphSource<'a>,
{
fn page_count(&mut self) -> usize {
fn page_count(&self) -> usize {
1
}

Expand Down
8 changes: 8 additions & 0 deletions core/embed/rust/src/ui/flow/page.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ impl<T: Component + Paginate> SwipePage<T> {
self.limit = limit;
self
}

pub fn page_count(&self) -> usize {
self.pages
}

pub fn current_page(&self) -> usize {
self.current
}
}

impl<T: Component + Paginate> Component for SwipePage<T> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl AddressDetails {
}

impl Paginate for AddressDetails {
fn page_count(&mut self) -> usize {
fn page_count(&self) -> usize {
self.get_internal_page_count()
}

Expand Down
32 changes: 14 additions & 18 deletions core/embed/rust/src/ui/model_mercury/component/footer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,8 @@ impl<'a> Footer<'a> {
)
}

pub fn with_page_counter(max_pages: u8, instruction: TString<'static>) -> Self {
Self::from_content(FooterContent::PageCounter(PageCounter::new(
max_pages,
instruction,
)))
pub fn with_page_counter(instruction: TString<'static>) -> Self {
Self::from_content(FooterContent::PageCounter(PageCounter::new(instruction)))
}

pub fn with_page_hint(
Expand Down Expand Up @@ -115,18 +112,18 @@ impl<'a> Footer<'a> {
}
}

pub fn update_page_counter(&mut self, ctx: &mut EventCtx, current: usize, max: Option<usize>) {
pub fn update_page_counter(&mut self, ctx: &mut EventCtx, current: usize, max: usize) {
match &mut self.content {
FooterContent::PageCounter(counter) => {
counter.update_current_page(current);
counter.update_current_page(current, max);
self.swipe_allow_down = counter.is_first_page();
self.swipe_allow_up = counter.is_last_page();
ctx.request_paint();
}
FooterContent::PageHint(page_hint) => {
page_hint.update_current_page(current, max);
self.swipe_allow_down = page_hint.is_first_page();
self.swipe_allow_up = page_hint.is_last_page();
FooterContent::PageHint(hint) => {
hint.update_current_page(current, max);
self.swipe_allow_down = hint.is_first_page();
self.swipe_allow_up = hint.is_last_page();
ctx.request_paint();
}
_ => {
Expand Down Expand Up @@ -322,16 +319,17 @@ struct PageCounter {
}

impl PageCounter {
fn new(page_max: u8, instruction: TString<'static>) -> Self {
fn new(instruction: TString<'static>) -> Self {
Self {
instruction,
page_curr: 0,
page_max,
page_max: 0,
font: Font::SUB,
}
}

fn update_current_page(&mut self, new_value: usize) {
fn update_current_page(&mut self, new_value: usize, max: usize) {
self.page_max = max as u8;
self.page_curr = (new_value as u8).clamp(0, self.page_max.saturating_sub(1));
}

Expand Down Expand Up @@ -410,11 +408,9 @@ struct PageHint {
}

impl PageHint {
fn update_current_page(&mut self, current: usize, max: Option<usize>) {
fn update_current_page(&mut self, current: usize, max: usize) {
self.page_num = max as u8;
self.page_curr = (current as u8).clamp(0, self.page_num.saturating_sub(1));
if let Some(max) = max {
self.page_num = max as u8;
}
}

fn update_max_page(&mut self, max: usize) {
Expand Down
4 changes: 2 additions & 2 deletions core/embed/rust/src/ui/model_mercury/component/frame.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,8 @@ where
}

#[inline(never)]
pub fn with_footer_counter(mut self, instruction: TString<'static>, max_value: u8) -> Self {
self.footer = Some(Footer::with_page_counter(max_value, instruction));
pub fn with_footer_counter(mut self, instruction: TString<'static>) -> Self {
self.footer = Some(Footer::with_page_counter(instruction));
self
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ impl<F: Fn(usize) -> TString<'static>> PagedVerticalMenu<F> {
}

impl<F: Fn(usize) -> TString<'static>> Paginate for PagedVerticalMenu<F> {
fn page_count(&mut self) -> usize {
fn page_count(&self) -> usize {
self.num_pages()
}

Expand Down
29 changes: 25 additions & 4 deletions core/embed/rust/src/ui/model_mercury/flow/confirm_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
component::{
swipe_detect::SwipeSettings,
text::paragraphs::{Paragraph, ParagraphSource, ParagraphVecShort, VecExt},
Component, ComponentExt, Paginate,
Component, ComponentExt, EventCtx, Paginate,
},
flow::{
base::{Decision, DecisionBuilder as _},
Expand All @@ -19,7 +19,8 @@ use crate::{

use super::super::{
component::{
Frame, FrameMsg, PromptMsg, PromptScreen, SwipeContent, VerticalMenu, VerticalMenuChoiceMsg,
Footer, Frame, FrameMsg, PromptMsg, PromptScreen, SwipeContent, VerticalMenu,
VerticalMenuChoiceMsg,
},
theme,
};
Expand Down Expand Up @@ -171,15 +172,17 @@ pub fn new_confirm_action(
ConfirmActionStrings::new(title, subtitle, None, prompt_screen.then_some(prompt_title)),
hold,
None,
false,
)
}

#[inline(never)]
fn new_confirm_action_uni<T: Component + MaybeTrace + 'static>(
content: T,
fn new_confirm_action_uni<T: Component + Paginate + MaybeTrace + 'static>(
content: SwipeContent<SwipePage<T>>,
menu: ConfirmActionMenu,
strings: ConfirmActionStrings,
hold: bool,
show_page_counter: bool,
) -> Result<SwipeFlow, error::Error> {
let (prompt_screen, prompt_pages, flow, page) =
create_flow(strings.title, strings.prompt_screen, hold);
Expand All @@ -191,6 +194,22 @@ fn new_confirm_action_uni<T: Component + MaybeTrace + 'static>(
.with_menu_button()
.with_footer(TR::instructions__swipe_up.into(), None);

if show_page_counter {
fn footer_update_fn<T: Component + Paginate>(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think defining the fn inside this if has no benefit - it's not a Fn object which would not be created if show_page_counter was false. I would put it outside as a free-standing function and would just do

if show_page_counter {
     content_intro = content_intro
            .with_footer_counter(TR::instructions__swipe_up.into())
            .register_footer_update_fn(footer_update_fn::<T>);
}

content: &SwipeContent<SwipePage<T>>,
ctx: &mut EventCtx,
footer: &mut Footer,
) {
let current_page = content.inner().current_page();
let page_count = content.inner().page_count();
footer.update_page_counter(ctx, current_page, page_count);
}

content_intro = content_intro
.with_footer_counter(TR::instructions__swipe_up.into())
.register_footer_update_fn(footer_update_fn::<T>);
}

if let Some(subtitle) = strings.subtitle {
content_intro = content_intro.with_subtitle(subtitle);
}
Expand Down Expand Up @@ -318,11 +337,13 @@ pub fn new_confirm_action_simple<T: Component + Paginate + MaybeTrace + 'static>
strings: ConfirmActionStrings,
hold: bool,
page_limit: Option<usize>,
show_page_counter: bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could probably be also named page_counter just like in `ConfirmBlobParams.

) -> Result<SwipeFlow, error::Error> {
new_confirm_action_uni(
SwipeContent::new(SwipePage::vertical(content).with_limit(page_limit)),
menu,
strings,
hold,
show_page_counter,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ fn footer_update_fn(
) {
let current_page = content.inner().inner().current_page();
let total_pages = content.inner().inner().num_pages();
footer.update_page_counter(ctx, current_page, Some(total_pages));
footer.update_page_counter(ctx, current_page, total_pages);
}

impl ConfirmFido {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ fn footer_update_fn(
// to get total pages instead of using Paginate because it borrows mutably
let current_page = content.inner().inner().current_page();
let total_pages = content.inner().inner().inner().len() / 2; // 2 paragraphs per page
footer.update_page_counter(ctx, current_page, Some(total_pages));
footer.update_page_counter(ctx, current_page, total_pages);
}

pub fn new_continue_recovery(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ fn footer_updating_func(
) {
let current_page = content.inner().current_page();
let total_pages = content.inner().num_pages();
footer.update_page_counter(ctx, current_page, Some(total_pages));
footer.update_page_counter(ctx, current_page, total_pages);
}

pub fn new_show_share_words(
Expand Down Expand Up @@ -103,7 +103,6 @@ pub fn new_show_share_words(
.one_button_request(ButtonRequestCode::ResetDevice.with_name("share_words"))
.with_pages(move |_| nwords + 2);

let n_words = share_words_vec.len();
let content_words = Frame::left_aligned(
title,
InternallySwipableContent::new(ShareWords::new(share_words_vec, subtitle)),
Expand All @@ -113,7 +112,7 @@ pub fn new_show_share_words(
.with_vertical_pages()
.with_subtitle(subtitle)
.register_header_update_fn(header_updating_func)
.with_footer_counter(TR::instructions__swipe_up.into(), n_words as u8)
.with_footer_counter(TR::instructions__swipe_up.into())
.register_footer_update_fn(footer_updating_func)
.map(|_| None);

Expand Down
8 changes: 8 additions & 0 deletions core/embed/rust/src/ui/model_mercury/flow/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ pub struct ConfirmBlobParams {
hold: bool,
chunkify: bool,
text_mono: bool,
page_counter: bool,
page_limit: Option<usize>,
swipe_up: bool,
swipe_down: bool,
Expand Down Expand Up @@ -78,6 +79,7 @@ impl ConfirmBlobParams {
hold: false,
chunkify: false,
text_mono: true,
page_counter: false,
page_limit: None,
swipe_up: false,
swipe_down: false,
Expand Down Expand Up @@ -170,6 +172,11 @@ impl ConfirmBlobParams {
self
}

pub fn with_page_counter(mut self, page_counter: bool) -> Self {
self.page_counter = page_counter;
self
}

pub const fn with_page_limit(mut self, page_limit: Option<usize>) -> Self {
self.page_limit = page_limit;
self
Expand Down Expand Up @@ -266,6 +273,7 @@ impl ConfirmBlobParams {
),
self.hold,
self.page_limit,
self.page_counter,
)
}
}
Expand Down
Loading