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

core/ui: Improve semantics of layout functions #1731

Merged
merged 9 commits into from Jul 26, 2021
Merged

Conversation

matejcik
Copy link
Contributor

confirm_hex was replaced by a more appropriate confirm_blob.

There are now two main "confirm property" functions:

  • confirm_blob should be used when the property is not human-readable text. So for public keys, hashes, passphrase, etc.
  • confirm_text should be used when the property is human-readable text. So flag names, message texts, etc.

Two specializations exist for common cases: confirm_address and confirm_amount.

All screens using the new functions are auto-paginated. This produces part of the UI diff.

The other part is using semantically appropriate layouts from the above set in altcoins, which in some cases changes rendering from bold font to monospace font.

A very significant part of the PR is complete overhaul of Stellar UI.

@andrewkozlik please take a look at 9de567b and the resulting UI diff of get_ownership_proof

@matejcik matejcik requested a review from onvej-sl as a code owner July 23, 2021 08:12
@matejcik matejcik removed the request for review from onvej-sl July 23, 2021 08:12
@matejcik matejcik added this to To be reviewed in Firmware Pull Requests via automation Jul 23, 2021
Copy link
Contributor

@andrewkozlik andrewkozlik left a comment

Choose a reason for hiding this comment

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

@andrewkozlik please take a look at 9de567b and the resulting UI diff of get_ownership_proof

9de567b LGTM. I haven't looked at the rest.

Copy link
Member

@mmilata mmilata left a comment

Choose a reason for hiding this comment

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

Awesome! UI diff ACK.

core/src/trezor/ui/layouts/tt.py Show resolved Hide resolved
Firmware Pull Requests automation moved this from To be reviewed to Finalizing Jul 23, 2021
Truncation options were removed.

Subtitle distinct from description was removed.

confirm_hex was replaced by confirm_blob. You should use confirm_blob
when displaying data that is not human readable and can be broken at any
character.

Also it is now possible to pass bytes, which are automatically converted
to hex.

For displaying addresses, a separate confirm_address is introduced,
which simply delegates to confirm_blob, but has a more limited
signature.

Analogously, there is confirm_text for text data (should maybe be used
in many places where we currently use confirm_metadata) and a
specialized confirm_amount.
@matejcik matejcik merged commit 0e14291 into master Jul 26, 2021
Firmware Pull Requests automation moved this from Finalizing to Merged Jul 26, 2021
@matejcik matejcik deleted the matejcik/confirm-hex branch July 26, 2021 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants