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

Fix/check seed links #16144

Merged
merged 5 commits into from
Jan 3, 2025
Merged

Fix/check seed links #16144

merged 5 commits into from
Jan 3, 2025

Conversation

komret
Copy link
Contributor

@komret komret commented Jan 2, 2025

I noticed that in case deviceWillBeWiped during firmware update, user is sent to settings to create or check backup, but these settings are not available for firmwares unsupported by Suite. The links should lead right to the create/check backup modals instead. While this is only a problem with unsupported firmware (currently 1.6.1), redirecting right to the create/check backup modal is more convenient than having to go to settings first.

Plus some cleanup and fixes along the way, in separate commits. For example, Suite warned users that device will be wiped even though it was not initialized.

TODO:

  • update Crowdin

Related: #16094 - this PR adds Check backup button to CheckSeedStep for some cases. I decided to add the button to create or check backup to CheckSeedStep for every case so that we are consistent.

QA:
Some testing scenarios I can think of:

  • Check with a fresh device
  • Check with an initialized device
  • Check required firmware update
  • Check with a backed-up device
  • Check with a device without backup
  • Check firmware switch on Safe series
  • Check firmware update from 1.6.1, this is the only fw version guaranteed to wipe and should show the warning (no need to proceed to update itself, just the screens before that)

Screenshots

Before:

image
image
image
Notice how clicking on creating a wallet backup in Settings takes you to the empty page. You can click on the Check wallet backup button in the red banner, but:

  • The banner originates in message system and might not be there in the future. EDIT: it is not there anymore.
  • Checking backup in a wallet without backup makes no sense. Creating backup is not possible at this point because there is no button to do it.

After:

I don't have a 1.6.1 device so testing with firmware type switch on a TS3.

Without backup:

Screen.Recording.2025-01-03.at.10.22.11.mov

Backed up:

Screen.Recording.2025-01-03.at.10.25.08.mov

Copy link

github-actions bot commented Jan 2, 2025

🚀 Expo preview is ready!

  • Project → trezor-suite-preview
  • Platforms → android, ios
  • Scheme → trezorsuitelite
  • Runtime Version → 21
  • More info

Learn more about 𝝠 Expo Github Action

@komret komret marked this pull request as ready for review January 3, 2025 09:29
@komret komret requested a review from peter-sanderson January 3, 2025 09:29
@komret komret added bug Something isn't working as expected settings Settings page and removed settings Settings page labels Jan 3, 2025
const installationWillChangeFirmwareVendorHeader =
!!shouldSwitchFirmwareType &&
deviceModelInternal !== undefined &&
![DeviceModelInternal.T1B1, DeviceModelInternal.T2T1].includes(deviceModelInternal);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we shall add come explanation. I know there is some logic that some models have this change of vendor and other don't but I don't know any details and from the code itself its not obvious to me how this is designed (or reasons why it is the way it is)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@peter-sanderson peter-sanderson left a comment

Choose a reason for hiding this comment

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

Good job, its very elegant actually. 👏

@komret
Copy link
Contributor Author

komret commented Jan 3, 2025

/rebase

Copy link

github-actions bot commented Jan 3, 2025

@trezor-ci trezor-ci force-pushed the fix/check-seed-links branch from 48e164d to bd94a01 Compare January 3, 2025 12:05
@komret komret merged commit 24512a0 into develop Jan 3, 2025
34 checks passed
@komret komret deleted the fix/check-seed-links branch January 3, 2025 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as expected
Projects
Status: 🤝 Needs QA
Development

Successfully merging this pull request may close these issues.

2 participants