-
-
Notifications
You must be signed in to change notification settings - Fork 276
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
feat(suite-native): add DeviceCompromisedModal for FW revision check #16722
feat(suite-native): add DeviceCompromisedModal for FW revision check #16722
Conversation
80124af
to
8cae3e0
Compare
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
error Error: http://10.0.0.28:4873/@trezor%2feslint: no such package available WalkthroughThis pull request introduces a comprehensive implementation of a device compromised modal for firmware revision checks in the Suite Native application. The changes span multiple files across different modules, including the creation of a new Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
✅ Files skipped from review due to trivial changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (17)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
🚀 Expo preview is ready!
|
suite-native/module-authenticity-checks/src/screens/DeviceCompromisedModalScreen.tsx
Outdated
Show resolved
Hide resolved
suite-native/module-authenticity-checks/src/screens/DeviceCompromisedModalScreen.tsx
Show resolved
Hide resolved
suite-native/module-authenticity-checks/src/screens/DeviceCompromisedModalScreen.tsx
Outdated
Show resolved
Hide resolved
suite-native/module-authenticity-checks/src/screens/DeviceCompromisedModalScreen.tsx
Outdated
Show resolved
Hide resolved
I just realized that the device authenticity check screen that @yanascz implemented recently looks almost the same (the copy is identical). Might be good idea to unify it? Or is there any reason why we do not want it? |
8cae3e0
to
0b7f605
Compare
Ad similar screens for authenticity checks: discussed personally, but here it's for visibility - there are multiple similar screens like this one, but since they are mostly WIP, it's still too early to try to unify it. |
// TODO: this hook is getting very complex, and it's hard to understand the logic when it navigates there and back again. | ||
// Ideally there'd be a single source of truth, a function returning "where we should be as per current state" | ||
// rather than multiple useEffects with imperative instructions "go there when X changes" | ||
if (isDeviceCompromisedModalFocused) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at first I tried to use shouldNavigateToDeviceCompromisedModal
for this condition, but then I realized - even if you don't dismiss the modal, this variable still becomes false when disconnecting device, because whole state.device
is anulled when you disconnect device.
lastRoute
might not be an ideal way to determine current state of UI, but I didn't think of anything else without overengineering it..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done, thanks for adjusting the code to my comments ;).
/rebase |
Start rebasing: https://github.com/trezor/trezor-suite/actions/runs/13174129482 |
816f368
to
358df52
Compare
Merging despite broken E2E tests, they're broken globally (slack discussion) |
Description
FW revision check part II.
Related Issue
Resolve #16448
Screenshots / QA instructions
2-main
or1-main
) to continue.IsFwRevisionCheckEnabled
: no visible changePressing the Contact Support button opens browser with chatbot flow:
![hal](https://private-user-images.githubusercontent.com/5287903/408110885-119abfa2-c291-4198-ad07-2cc929996d24.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk1NTAyNDYsIm5iZiI6MTczOTU0OTk0NiwicGF0aCI6Ii81Mjg3OTAzLzQwODExMDg4NS0xMTlhYmZhMi1jMjkxLTQxOTgtYWQwNy0yY2M5Mjk5OTZkMjQucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIxNCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMTRUMTYxOTA2WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9MGE0ODVkYzg3NGQ4ZWUwMjYzNmU2MTFmMWM3ZTFkZjE4M2ZkNmI5NzEwMjQ5YzFkM2NhYjcyOGZiYzI5YTZlMyZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.h9hKmtuKILhiKRBB3Sk_ZB6WL_NK43uBYoIbo0nAuLs)
Pressing back returns back to normal dashboard
![dashboard](https://private-user-images.githubusercontent.com/5287903/408111915-038b4f2f-8618-4202-ad39-da791a554c11.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk1NTAyNDYsIm5iZiI6MTczOTU0OTk0NiwicGF0aCI6Ii81Mjg3OTAzLzQwODExMTkxNS0wMzhiNGYyZi04NjE4LTQyMDItYWQzOS1kYTc5MWE1NTRjMTEucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIxNCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMTRUMTYxOTA2WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9NDNjZGUxMzY3YjBiYjViODNkYTI3YzJjNmE1NDc4NDQ5MzBjNjZmMDhhOWQ2ZDUyYmMxNjc0MDA2NmZiNzg5NSZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.L0isWeGIBs0z1gQLJUiJHSIjDhS2JZrDyLwMiB7KeZY)