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

layouts: altcoin apps #1538

Merged
merged 23 commits into from
Jun 23, 2021
Merged

layouts: altcoin apps #1538

merged 23 commits into from
Jun 23, 2021

Conversation

mmilata
Copy link
Member

@mmilata mmilata commented Mar 19, 2021

This is a follow-up of #1480 which modifies all altcoin apps to use the layout abstraction. To support various multiple page confirms it adds the paginate_paragraphs function which is a generalization of paginate_text that allows mixing fonts in the paginated output, with the restriction that different font text must start on a new line.

In the previous PR we discussed the truncate= parameter which is used here to emulate the old behavior wrt. text that doesn't fit on a single screen. It's used quite a lot to keep the UI diff small (or skip the pagination logic if we know the text fits), even in some cases where pagination might be desirable. Ideas?

My aim was to keep the UI diff reasonably small even though there are some differences especially for Eos and Monero.

Not covered in this PR, to be done in the next (final) one:

  • apps.management and apps.webauthn
  • splitting trezor.ui.layouts.tt to multiple modules to make it smaller
  • review of br_type values to have them consistent & useful to Suite

@mmilata mmilata marked this pull request as ready for review March 22, 2021 11:58
@mmilata mmilata requested review from tsusanka and removed request for onvej-sl March 22, 2021 11:58
Base automatically changed from mmilata/layouts-more to master March 30, 2021 20:34
@mmilata mmilata removed the request for review from andrewkozlik April 13, 2021 20:19
@mmilata
Copy link
Member Author

mmilata commented Apr 13, 2021

Rebased to master. UI diff.

@tsusanka tsusanka added this to the 21.07 milestone May 20, 2021
@tsusanka tsusanka self-assigned this May 20, 2021
@matejcik matejcik force-pushed the mmilata/layouts-3 branch 3 times, most recently from 6dcb057 to a595b02 Compare June 14, 2021 13:29
@matejcik
Copy link
Contributor

before this is merged, I want to do two things:

  • Implement auto-swiping. I'll do that via a separate PR and then rebase this on top of it. This will improve the UI diff, because right now a lot of screens are not correctly scrolled.

  • Finish the TODO with putting property label and value always on the same screen

@matejcik
Copy link
Contributor

I rebased this on today's master (incl. auto-pagination) and finished the key+value display above.

@mmilata please review my commits (starting at 85cdddc)

@mmilata
Copy link
Member Author

mmilata commented Jun 22, 2021

Thanks for finishing confirm_properties, it looks very good! Seems like device tests failed, probably due to the ButtonRequest changes. Am curious about the final UI diff, but the screens I've seen look OK.

@matejcik
Copy link
Contributor

CI passing now

@mmilata
Copy link
Member Author

mmilata commented Jun 23, 2021

Reviewed UI diff for the last time and found no regressions, only improvements. Let's merge it.

@matejcik matejcik merged commit 47c2be9 into master Jun 23, 2021
@matejcik matejcik deleted the mmilata/layouts-3 branch June 23, 2021 09:51
@bosomt
Copy link

bosomt commented Jul 5, 2021

QA OK

Trezor model T version 2.4.1 revision 01c1ae42

Wallets:
Monero,
SimpleStaking
Cardano,
MyeEth + MyCrypto,
Electrum, Electrum Multisig,
Mycellium - Android, Blockstream GREEN - Andorid
and of course Trezor Wallet /standard + shamir backup and recovery / and Trezor Suite /standard + shamir backup and recovery /

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants