feat(core): improve sign message designs

[no changelog]

core/embed/rust/src/ui/model_tr/component/button.rs

@@ -351,7 +351,10 @@ pub struct ButtonDetails<T> {
     offset: Offset,
 }
 
-impl<T> ButtonDetails<T> {
+impl<T> ButtonDetails<T>
+where
+    T: StringType,
+{
     /// Text button.
     pub fn text(text: T) -> Self {
         Self {
@@ -376,6 +379,19 @@ impl<T> ButtonDetails<T> {
         }
     }
 
+    /// Resolves text and finds possible icon names.
+    pub fn from_text_possible_icon(text: T) -> Self {
+        if text.as_ref() == "" {
+            Self::cancel_icon()
+        } else if text.as_ref() == "left_arrow_icon" {
+            Self::left_arrow_icon()
+        } else if text.as_ref() == "up_arrow_icon" {
+            Self::up_arrow_icon()
+        } else {
+            Self::text(text)
+        }
+    }
+
     /// Text with arms signalling double press.
     pub fn armed_text(text: T) -> Self {
         Self::text(text).with_arms()
@@ -529,6 +545,15 @@ where
         )
     }
 
+    /// Left text, armed text and right info icon/text.
+    pub fn text_armed_info(left: T, middle: T) -> Self {
+        Self::new(
+            Some(ButtonDetails::from_text_possible_icon(left)),
+            Some(ButtonDetails::armed_text(middle)),
+            Some(ButtonDetails::text("i".into()).with_fixed_width(theme::BUTTON_ICON_WIDTH)),
+        )
+    }
+
     /// Left cancel, armed text and right info icon/text.
     pub fn cancel_armed_info(middle: T) -> Self {
         Self::new(
@@ -559,16 +584,16 @@ where
     /// Left and right texts.
     pub fn text_none_text(left: T, right: T) -> Self {
         Self::new(
-            Some(ButtonDetails::text(left)),
+            Some(ButtonDetails::from_text_possible_icon(left)),
             None,
-            Some(ButtonDetails::text(right)),
+            Some(ButtonDetails::from_text_possible_icon(right)),
         )
     }
 
     /// Left text and right arrow.
     pub fn text_none_arrow(text: T) -> Self {
         Self::new(
-            Some(ButtonDetails::text(text)),
+            Some(ButtonDetails::from_text_possible_icon(text)),
             None,
             Some(ButtonDetails::right_arrow_icon()),
         )
@@ -577,7 +602,7 @@ where
     /// Left text and WIDE right arrow.
     pub fn text_none_arrow_wide(text: T) -> Self {
         Self::new(
-            Some(ButtonDetails::text(text)),
+            Some(ButtonDetails::from_text_possible_icon(text)),
             None,
             Some(ButtonDetails::down_arrow_icon_wide()),
         )
@@ -585,7 +610,11 @@ where
 
     /// Only right text.
     pub fn none_none_text(text: T) -> Self {
-        Self::new(None, None, Some(ButtonDetails::text(text)))
+        Self::new(
+            None,
+            None,
+            Some(ButtonDetails::from_text_possible_icon(text)),
+        )
     }
 
     /// Left and right arrow icons for navigation.
@@ -602,7 +631,7 @@ where
         Self::new(
             Some(ButtonDetails::left_arrow_icon()),
             None,
-            Some(ButtonDetails::text(text)),
+            Some(ButtonDetails::from_text_possible_icon(text)),
         )
     }
 
@@ -611,7 +640,7 @@ where
         Self::new(
             Some(ButtonDetails::up_arrow_icon()),
             None,
-            Some(ButtonDetails::text(text)),
+            Some(ButtonDetails::from_text_possible_icon(text)),
         )
     }
 
@@ -656,7 +685,7 @@ where
         Self::new(
             Some(ButtonDetails::cancel_icon()),
             None,
-            Some(ButtonDetails::text(text)),
+            Some(ButtonDetails::from_text_possible_icon(text)),
         )
     }
 

core/embed/rust/src/ui/model_tr/component/show_more.rs

@@ -27,8 +27,12 @@ where
     T: Component,
     U: StringType + Clone,
 {
-    pub fn new(content: T) -> Self {
+    pub fn new(content: T, cancel_button: Option<U>, button: U) -> Self {
-        let btn_layout = ButtonLayout::cancel_armed_info("CONFIRM".into());
+        let btn_layout = if let Some(cancel_text) = cancel_button {
+            ButtonLayout::text_armed_info(cancel_text, button)
+        } else {
+            ButtonLayout::cancel_armed_info(button)
+        };
         Self {
             content: Child::new(content),
             buttons: Child::new(ButtonController::new(btn_layout)),

core/embed/rust/src/ui/model_tr/layout.rs

@@ -254,19 +254,7 @@ fn content_in_button_page<T: Component + Paginate + MaybeTrace + 'static>(
     hold: bool,
 ) -> Result<Obj, Error> {
     // Left button - icon, text or nothing.
-    let cancel_btn = if let Some(verb_cancel) = verb_cancel {
+    let cancel_btn = verb_cancel.map(ButtonDetails::from_text_possible_icon);
-        if !verb_cancel.is_empty() {
-            if verb_cancel.as_ref() == "left_arrow_icon" {
-                Some(ButtonDetails::left_arrow_icon())
-            } else {
-                Some(ButtonDetails::text(verb_cancel))
-            }
-        } else {
-            Some(ButtonDetails::cancel_icon())
-        }
-    } else {
-        None
-    };
 
     // Right button - text or nothing.
     // Optional HoldToConfirm
@@ -1060,6 +1048,11 @@ extern "C" fn new_show_mismatch() -> Obj {
 extern "C" fn new_confirm_with_info(n_args: usize, args: *const Obj, kwargs: *mut Map) -> Obj {
     let block = move |_args: &[Obj], kwargs: &Map| {
         let title: StrBuffer = kwargs.get(Qstr::MP_QSTR_title)?.try_into()?;
+        let button: StrBuffer = kwargs.get(Qstr::MP_QSTR_button)?.try_into()?;
+        let verb_cancel: Option<StrBuffer> = kwargs
+            .get(Qstr::MP_QSTR_verb_cancel)
+            .unwrap_or_else(|_| Obj::const_none())
+            .try_into_option()?;
         let items: Obj = kwargs.get(Qstr::MP_QSTR_items)?;
 
         let mut paragraphs = ParagraphVecShort::new();
@@ -1078,6 +1071,8 @@ extern "C" fn new_confirm_with_info(n_args: usize, args: *const Obj, kwargs: *mu
             title,
             ShowMore::<Paragraphs<ParagraphVecShort<StrBuffer>>, StrBuffer>::new(
                 paragraphs.into_paragraphs(),
+                verb_cancel,
+                button,
             ),
         ))?;
         Ok(obj.into())
@@ -1670,9 +1665,10 @@ pub static mp_module_trezorui2: Module = obj_module! {
     /// def confirm_with_info(
     ///     *,
     ///     title: str,
-    ///     button: str,  # unused on TR
+    ///     button: str,
     ///     info_button: str,  # unused on TR
     ///     items: Iterable[Tuple[int, str]],
+    ///     verb_cancel: str | None = None,
     /// ) -> object:
     ///     """Confirm given items but with third button. Always single page
     ///     without scrolling."""

core/mocks/generated/trezorui2.pyi

@@ -223,9 +223,10 @@ def show_mismatch() -> object:
 def confirm_with_info(
     *,
     title: str,
-    button: str,  # unused on TR
+    button: str,
     info_button: str,  # unused on TR
     items: Iterable[Tuple[int, str]],
+    verb_cancel: str | None = None,
 ) -> object:
     """Confirm given items but with third button. Always single page
     without scrolling."""

core/src/trezor/ui/layouts/tr/__init__.py

@@ -711,6 +711,7 @@ async def should_show_more(
     br_type: str = "should_show_more",
     br_code: ButtonRequestType = BR_TYPE_OTHER,
     confirm: str | bytes | None = None,
+    verb_cancel: str | None = None,
 ) -> bool:
     """Return True if the user wants to show more (they click a special button)
     and False when the user wants to continue without showing details.
@@ -726,7 +727,8 @@ async def should_show_more(
                 title=title.upper(),
                 items=para,
                 button=confirm.upper(),
-                info_button=button_text.upper(),
+                verb_cancel=verb_cancel,  # type: ignore [No parameter named "verb_cancel"]
+                info_button=button_text.upper(),  # unused on TR
             )
         ),
         br_type,
@@ -747,6 +749,8 @@ async def confirm_blob(
     title: str,
     data: bytes | str,
     description: str | None = None,
+    verb: str = "CONFIRM",
+    verb_cancel: str | None = "",  # icon
     hold: bool = False,
     br_code: ButtonRequestType = BR_TYPE_OTHER,
     ask_pagination: bool = False,
@@ -759,15 +763,17 @@ async def confirm_blob(
             description=description,
             data=data,
             extra=None,
-            verb_cancel="",  # to show the cancel icon
+            verb=verb,
+            verb_cancel=verb_cancel,
             hold=hold,
-            verb_cancel="",  # icon
         )
     )
 
     if ask_pagination and layout.page_count() > 1:
         assert not hold
-        await _confirm_ask_pagination(br_type, title, data, description, br_code)
+        await _confirm_ask_pagination(
+            br_type, title, data, description, verb_cancel, br_code
+        )
 
     else:
         await raise_if_not_confirmed(
@@ -784,6 +790,7 @@ async def _confirm_ask_pagination(
     title: str,
     data: bytes | str,
     description: str,
+    verb_cancel: str | None,
     br_code: ButtonRequestType,
 ) -> None:
     paginated: ui.Layout | None = None
@@ -796,6 +803,7 @@ async def _confirm_ask_pagination(
         if not await should_show_more(
             title,
             para=[(ui.NORMAL, description), (ui.MONO, data)],
+            verb_cancel=verb_cancel,
             br_type=br_type,
             br_code=br_code,
         ):
@@ -806,7 +814,7 @@ async def _confirm_ask_pagination(
                 trezorui2.confirm_more(
                     title=title,
                     button="GO BACK",
-                    items=[(ui.MONO, data)],
+                    items=[(ui.BOLD, f"Size: {len(data)} bytes"), (ui.MONO, data)],
                 )
             )
         else:
@@ -1089,29 +1097,31 @@ async def confirm_sign_identity(
 async def confirm_signverify(
     coin: str, message: str, address: str, verify: bool
 ) -> None:
-    if verify:
+    br_type = "verify_message" if verify else "sign_message"
-        header = f"Verify {coin} message"
-        br_type = "verify_message"
-    else:
-        header = f"Sign {coin} message"
-        br_type = "sign_message"
 
-    await confirm_blob(
+    # Allowing to go back from the second screen
-        br_type,
+    while True:
-        header.upper(),
+        await confirm_blob(
-        address,
+            br_type,
-        "Confirm address:",
+            "SIGNING ADDRESS",
-        br_code=BR_TYPE_OTHER,
+            address,
-    )
+            verb="CONTINUE",
+            br_code=BR_TYPE_OTHER,
+        )
 
-    await confirm_value(
+        try:
-        header.upper(),
+            await confirm_blob(
-        message,
+                br_type,
-        "Confirm message:",
+                "CONFIRM MESSAGE",
-        br_type,
+                message,
-        BR_TYPE_OTHER,
+                verb_cancel="up_arrow_icon",
-        verb="CONFIRM",
+                br_code=BR_TYPE_OTHER,
-    )
+                ask_pagination=True,
+            )
+        except ActionCancelled:
+            continue
+        else:
+            break
 
 
 async def show_error_popup(

core/src/trezor/ui/layouts/tt_v2/__init__.py

@@ -725,6 +725,7 @@ async def confirm_blob(
     data: bytes | str,
     description: str | None = None,
     verb: str = "CONFIRM",
+    verb_cancel: str | None = None,
     hold: bool = False,
     br_code: ButtonRequestType = BR_TYPE_OTHER,
     ask_pagination: bool = False,
@@ -739,6 +740,7 @@ async def confirm_blob(
             extra=None,
             hold=hold,
             verb=verb,
+            verb_cancel=verb_cancel,
         )
     )
 

tests/device_tests/test_cancel.py

@@ -90,6 +90,15 @@ def test_cancel_on_paginated(client: Client):
 
     resp = client._raw_read()
     assert isinstance(resp, m.ButtonRequest)
+
+    # In TR, confirm message is no longer paginated by default,
+    # user needs to click info button
+    if client.debug.model == "R":
+        client._raw_write(m.ButtonAck())
+        client.debug.press_right()
+        resp = client._raw_read()
+        assert isinstance(resp, m.ButtonRequest)
+
     assert resp.pages is not None
     client._raw_write(m.ButtonAck())
 

tests/input_flows.py

@@ -235,6 +235,11 @@ class InputFlowSignMessagePagination(InputFlowBase):
         yield
         self.debug.press_yes()
 
+        # press info
+        yield
+        self.debug.press_right()
+
+        # paginate through the whole message
         br = yield
         # TODO: try load the message_read the same way as in model T
         if br.pages is not None:
@@ -243,6 +248,10 @@ class InputFlowSignMessagePagination(InputFlowBase):
                     self.debug.swipe_up()
         self.debug.press_yes()
 
+        # confirm message
+        yield
+        self.debug.press_yes()
+
 
 class InputFlowShowAddressQRCode(InputFlowBase):
     def __init__(self, client: Client):
@@ -789,12 +798,18 @@ class InputFlowEthereumSignTxScrollDown(InputFlowBase):
         self.debug.wait_layout()
         self.debug.press_yes()
 
+        yield  # confirm data
+        self.debug.press_info()
+
         br = yield  # paginated data
         assert br.pages is not None
         for _ in range(br.pages):
             self.debug.wait_layout()
             self.debug.swipe_up()
 
+        yield  # confirm data
+        self.debug.press_yes()
+
         yield  # confirm amount
         self.debug.wait_layout()
         self.debug.press_yes()