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

refactor(core/mercury): unify confirm TOS #4051

Merged
merged 1 commit into from
Jul 28, 2024

Conversation

obrusvit
Copy link
Contributor

This PR unifies confirming Terms of Service (TOS) screen on T3T1. It saves only less than 1KB of debug flash (I expected more but..).

In the future, it wouldn't be bad to replace it with ConfirmAction but there are difficulties: 1) TOS has different text style, 2) choosing text for cancel button in menu. We do not want to pollute the implementation of confirm action with additional params.

@obrusvit obrusvit added code Code improvements T3T1 Trezor Safe 5 labels Jul 22, 2024
@obrusvit obrusvit self-assigned this Jul 22, 2024
@obrusvit obrusvit requested review from mmilata and removed request for prusnak and matejcik July 22, 2024 15:43
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.

utACK, big fan of any PR that deletes more lines than it adds

core/embed/rust/src/ui/model_mercury/flow/confirm_reset.rs Outdated Show resolved Hide resolved
@obrusvit obrusvit force-pushed the refactor/ui-t3t1/unify-tos-confirm branch from 68dd3c2 to 374330b Compare July 28, 2024 17:11
@obrusvit obrusvit force-pushed the refactor/ui-t3t1/unify-tos-confirm branch from 374330b to 1357142 Compare July 28, 2024 20:09
@obrusvit
Copy link
Contributor Author

Rebased again because of forgotten use crate::ui::component::text::paragraph::ParagraphSource.

@obrusvit obrusvit merged commit 008490b into main Jul 28, 2024
84 checks passed
@obrusvit obrusvit deleted the refactor/ui-t3t1/unify-tos-confirm branch July 28, 2024 21:14
@bosomt
Copy link

bosomt commented Jul 30, 2024

QA OK

Info:

  • Suite version: desktop 24.7.3 (19fd64f5b2ab1bc265821e34d4c4b9c910864d0e)
  • Browser: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) TrezorSuite/24.7.3 Chrome/118.0.5993.159 Electron/27.3.8 Safari/537.36
  • OS: MacIntel
  • Screen: 1512x982
  • Device: Trezor T3T1 2.8.1 regular (revision 33f5d47)
  • Transport: BridgeTransport 2.0.33

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code Code improvements T3T1 Trezor Safe 5
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants