Model R - smaller screen #2610

grdddj · 2022-11-05T18:53:00Z

Replaces #2277 as the main PR for model R - this one already contains the smaller screen.

Below are the current screens for the user onboarding and input methods.

It does not completely correspond to Figma, some screens are missing - based on #2609 - we may want to add these to TT's design as well.

It also doesn't yet have the "arrow signaling content continues from the previous page". I am not even sure we want to have this, together with the "arrow signalling content continues" - the scrollbars could be enough (scrollbar is just not yet implemented for the tutorial).

All screens to be seen under https://satoshilabs.gitlab.io/-/trezor/trezor-firmware/-/jobs/3279593101/artifacts/test_ui_report/all_unique_screens.html

Tutorial:

Seed backup:

PIN:

Num of words + recovery:

Passphrase

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

grdddj · 2023-05-12T09:21:14Z

Rebased on master. Will be pushing fixups with the review feedback

Hannsek · 2023-05-12T10:18:54Z

Good catch. In 9a72cc4 I am cancelling the whole wallet creation when users presses cancel on this screen. I cannot easily "not show" the cancel button in this configuration (ButtonPage component), but can make a custom layout for it. Do we want it?

Let's just cancel the whole wallet creation. This is the original behaviour how it was meant to be.

core/embed/rust/src/trace.rs

core/embed/rust/src/trezorhal/wordlist.rs

mmilata · 2023-05-15T22:43:10Z

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

+    Big,     // O
+    Middle,  // o
+    Small,   // .
+}


Nit: Model T uses the same style of scrollbars, feel free to reuse the simpler rendering algorithm in ui/model_tt/component/scroll.rs.

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

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

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

core/embed/rust/src/ui/display/toif.rs

mmilata

Overall the PR looks alright and seems to work, my comments are mostly re: style and code dupliction. Done for now, might give it once-over after the comments are addressed.

core/src/apps/management/apply_settings.py

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

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

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

core/src/apps/base.py

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

core/embed/rust/src/ui/model_tr/component/input_methods/choice.rs

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

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

core/embed/rust/src/ui/display/font.rs

core/embed/rust/src/ui/display/mod.rs

core/embed/rust/src/ui/model_tr/component/input_methods/wordlist.rs

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

core/embed/rust/src/trezorhal/wordlist.rs

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

core/src/apps/management/apply_settings.py

core/src/apps/management/recovery_device/layout.py

core/src/apps/management/apply_settings.py

tests/upgrade_tests/test_passphrase_consistency.py

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

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

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

core/embed/rust/src/ui/model_tr/bootloader/confirm.rs

core/embed/rust/src/ui/model_tr/bootloader/mod.rs

tests/device_tests/test_msg_applysettings.py

core/embed/rust/src/ui/display/mod.rs

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

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

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

TychoVrahe

few minor things with intention to make adding new models as easy and clean as possible

core/src/apps/base.py

core/src/apps/common/request_pin.py

core/src/apps/management/apply_settings.py

core/src/apps/management/change_wipe_code.py

core/src/apps/management/recovery_device/layout.py

grdddj · 2023-05-29T07:11:22Z

few minor things with intention to make adding new models as easy and clean as possible

Thanks for the suggestions.

I did all the changes in cd2b55c so we see how they look like. I am not sure I like the empty functions more than conditionals in the main code, but have no problem with that if we all agree.

Maybe name these functions "XXX_possibly_empty" so we see on first glance they can be unsupported for some models? This is my little worry that it will look like they are called everywhere even if they are not

TychoVrahe · 2023-05-29T07:34:10Z

It looks better to me, but lets see what others think. I don't really see the need for a suffix, as the implementation is one click away, but i don't mind it either.

Btw. Is there some technical limitation why passphrase on device would not be supported on T1? Since its using TR gui in core we might as well get rid of these conditions too, unless there is another reason which i dont know of.

grdddj · 2023-05-29T07:41:33Z

It looks better to me, but lets see what others think. I don't really see the need for a suffix, as the implementation is one click away, but i don't mind it either.

The "click to see the implementation" is not so straightforward in case of model-specific UI functions, as there is multiple of them. In my case, it always gets the TT's implementation, so then I need to manually check the TR (and other models in the future)

Btw. Is there some technical limitation why passphrase on device would not be supported on T1? Since its using TR gui in core we might as well get rid of these conditions too, unless there is another reason which i dont know of.

I think the T1 code in core does not even exist anymore, it was all replaced by TR. So the action here might be to remove all mentions of model == 1 in core

mmilata · 2023-05-29T11:29:41Z

It looks better to me, but lets see what others think. I don't really see the need for a suffix, as the implementation is one click away, but i don't mind it either.

👍 from me, if MODEL conditions tend to make code hard to read.

Maybe name these functions "XXX_possibly_empty" so we see on first glance they can be unsupported for some models? This is my little worry that it will look like they are called everywhere even if they are not

Or a comment at the call site.

matejcik · 2023-05-29T13:30:55Z

let's not forget that ui.layout namespace is about UI layout and not a model abstraction layer.

things like "confirm reenter pin" we discussed at standup. let's leave them in layout as-is, there's a bunch of things that shouldn't be there either but that's going to happen Soon(tm) anyway as part of the big layout unification.

please move the homescreen validation back to apply_settings -- it's clearly not a layout function, but there is currently no better place for it.

grdddj · 2023-05-29T13:56:20Z

please move the homescreen validation back to apply_settings -- it's clearly not a layout function, but there is currently no better place for it.

Done in cf99167

grdddj · 2023-05-30T13:32:07Z

CI is nicely green just with changelog check. BTW, should we put the changelog into code and python?

For the master difference, there are some small changes CC @Hannsek - https://satoshilabs.gitlab.io/-/trezor/trezor-firmware/-/jobs/4373819631/artifacts/master_diff/differing_screens.html

Hannsek · 2023-05-30T13:46:02Z

For the master difference, there are some small changes CC

From where these changes come from for model T?

grdddj · 2023-05-30T13:54:11Z

For the master difference, there are some small changes CC

From where these changes come from for model T?

Coming from changes we did to TR but are common. Should I revert some of those (3) changes? (the last one is just UI test change, the screen was already there)

Hannsek · 2023-05-30T14:00:05Z

Ouch, ok, we will tackle the last one later.

change to this please

Put there Trezor Model T, please

Or we can do it later with larger copy fixup.

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

matejcik · 2023-05-30T14:53:07Z

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

+) -> None:
+    from ubinascii import hexlify
+
+    def handle_bytes(prop: PropertyType):


@grdddj please create a separate issue so we can resolve this

grdddj · 2023-05-30T16:11:17Z

Ouch, ok, the last one we would tackle it later.

change to this please

Put there Trezor Model T, please

Or we can do it later with larger copy fixup.

Both the Coinjoin screen and the Trezor Model T(R) changes done

grdddj mentioned this pull request Nov 8, 2022

T2B1 design - Tutorial #2620

Closed

grdddj force-pushed the grdddj/model_r_smaller branch from 4ea394e to 6141ca7 Compare November 13, 2022 14:39

grdddj mentioned this pull request Nov 14, 2022

T2B1 design - Recovery #2628

Closed

grdddj removed a link to an issue Dec 1, 2022

TR design #2601

Closed

13 tasks

grdddj linked an issue Dec 1, 2022 that may be closed by this pull request

T2B1 design - Send #2664

Closed

grdddj force-pushed the grdddj/model_r_smaller branch from 5d5309b to e1f7d34 Compare January 1, 2023 13:22

grdddj mentioned this pull request Jan 1, 2023

Change Trezor emulator screen of TR to 128x64 px #2465

Closed

grdddj removed a link to an issue Jan 9, 2023

T2B1 design - header animation #2635

Closed

grdddj mentioned this pull request Jan 11, 2023

integrate marquee into model R pages #2726

Closed

grdddj force-pushed the grdddj/model_r_smaller branch 2 times, most recently from 1ea7e79 to 3abb737 Compare January 17, 2023 10:56

grdddj mentioned this pull request Jan 20, 2023

T2B1: unite the Rust API with redesigned model T #2440

Closed

grdddj force-pushed the grdddj/model_r_smaller branch from e4e3d92 to d5db922 Compare January 23, 2023 14:49

Hannsek reviewed Jan 24, 2023

View reviewed changes

core/embed/rust/src/ui/model_tr/component/show_more.rs Outdated Show resolved Hide resolved

TychoVrahe mentioned this pull request Jan 25, 2023

T2B1 rust bootloader #2772

Closed

This was referenced Jan 26, 2023

T2B1 design - arrows #2760

Closed

T2B1: improve one-time screens #2400

Closed

implement loaders for T2B1 #2742

Merged

grdddj force-pushed the grdddj/model_r_smaller branch from a4eb5c9 to 0d4db93 Compare January 31, 2023 14:56

Base automatically changed from grdddj/tt_stuff_from_tr to master May 11, 2023 19:02

grdddj force-pushed the grdddj/model_r_smaller branch from dd1777a to eb4075f Compare May 12, 2023 09:20

mmilata reviewed May 15, 2023

View reviewed changes

mmilata reviewed May 17, 2023

View reviewed changes

matejcik reviewed May 22, 2023

View reviewed changes

matejcik mentioned this pull request May 23, 2023

Fix off-by-one in rust geometry methods #2993

Open

matejcik reviewed May 23, 2023

View reviewed changes

core/embed/rust/src/ui/model_tr/component/share_words.rs Outdated Show resolved Hide resolved

TychoVrahe reviewed May 29, 2023

View reviewed changes

grdddj force-pushed the grdddj/model_r_smaller branch from e1f2d1b to bda3e17 Compare May 30, 2023 12:17

matejcik approved these changes May 30, 2023

View reviewed changes

grdddj force-pushed the grdddj/model_r_smaller branch from df70201 to f1a7970 Compare May 30, 2023 16:04

feat(all): add UI for Model R

f8e3ce1

grdddj force-pushed the grdddj/model_r_smaller branch from f1a7970 to f8e3ce1 Compare May 31, 2023 07:36

grdddj merged commit da14c22 into master May 31, 2023

grdddj deleted the grdddj/model_r_smaller branch May 31, 2023 08:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Model R - smaller screen #2610

Model R - smaller screen #2610

grdddj commented Nov 5, 2022

grdddj commented May 12, 2023

Hannsek commented May 12, 2023 •

edited

Loading

mmilata May 15, 2023

mmilata left a comment

TychoVrahe left a comment

grdddj commented May 29, 2023

TychoVrahe commented May 29, 2023

grdddj commented May 29, 2023

mmilata commented May 29, 2023

matejcik commented May 29, 2023

grdddj commented May 29, 2023

grdddj commented May 30, 2023

Hannsek commented May 30, 2023 •

edited

Loading

grdddj commented May 30, 2023

Hannsek commented May 30, 2023 •

edited

Loading

matejcik May 30, 2023

grdddj commented May 30, 2023

Model R - smaller screen #2610

Model R - smaller screen #2610

Conversation

grdddj commented Nov 5, 2022

Tutorial:

Seed backup:

PIN:

Num of words + recovery:

Passphrase

grdddj commented May 12, 2023

Hannsek commented May 12, 2023 • edited Loading

mmilata May 15, 2023

Choose a reason for hiding this comment

mmilata left a comment

Choose a reason for hiding this comment

TychoVrahe left a comment

Choose a reason for hiding this comment

grdddj commented May 29, 2023

TychoVrahe commented May 29, 2023

grdddj commented May 29, 2023

mmilata commented May 29, 2023

matejcik commented May 29, 2023

grdddj commented May 29, 2023

grdddj commented May 30, 2023

Hannsek commented May 30, 2023 • edited Loading

grdddj commented May 30, 2023

Hannsek commented May 30, 2023 • edited Loading

matejcik May 30, 2023

Choose a reason for hiding this comment

grdddj commented May 30, 2023

Hannsek commented May 12, 2023 •

edited

Loading

Hannsek commented May 30, 2023 •

edited

Loading

Hannsek commented May 30, 2023 •

edited

Loading