feat(suite-native): device onboarding tutorial #17638
Conversation
WalkthroughThe pull request introduces several configuration and code modifications across the suite-native modules. Key changes include the addition of new dependencies such as Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (17)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (13)
⏰ Context from checks skipped due to timeout of 90000ms (15)
🔇 Additional comments (5)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
suite-native/module-dev-utils/tsconfig.json (1)
8-8: Formatting improvement for consistencyThe change from multi-line to single-line format for the trading reference improves consistency with other entries in the file. This is a good practice for maintaining clean configuration files.
Consider applying this single-line format to the other multi-line reference entries in this file (lines 5-7 and 9-11) for complete consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
suite-native/device/package.json(1 hunks)suite-native/device/src/components/ContinueOnTrezorScreenContent.tsx(1 hunks)suite-native/device/src/index.ts(1 hunks)suite-native/device/tsconfig.json(1 hunks)suite-native/intl/src/en.ts(1 hunks)suite-native/module-dev-utils/tsconfig.json(1 hunks)suite-native/module-device-onboarding/package.json(1 hunks)suite-native/module-device-onboarding/src/navigation/DeviceOnboardingStackNavigator.tsx(2 hunks)suite-native/module-device-onboarding/src/screens/ConfirmFirmwareUpdateScreen.tsx(2 hunks)suite-native/module-device-onboarding/src/screens/DeviceTutorialScreen.tsx(1 hunks)suite-native/module-device-onboarding/src/screens/FirmwareInstallationScreen.tsx(1 hunks)suite-native/module-device-onboarding/tsconfig.json(1 hunks)suite-native/module-device-settings/src/screens/ContinueOnTrezorScreen.tsx(1 hunks)suite-native/navigation/src/navigators.ts(1 hunks)suite-native/navigation/src/routes.ts(1 hunks)
🔇 Additional comments (24)
suite-native/navigation/src/routes.ts (1)
44-44: LGTM! Good addition to support the device tutorial feature.This addition to the
DeviceOnboardingStackRoutesenum introduces a new route that aligns with the PR objective of adding a device onboarding tutorial. The placement at the end of the enum is appropriate, and the naming follows the established convention.suite-native/device/package.json (1)
35-35: LGTM! Dependency correctly added.The addition of
@trezor/env-utilsas a workspace dependency is properly formatted and positioned alphabetically in the dependencies list. This dependency is likely required for the newContinueOnTrezorScreenContentcomponent functionality.suite-native/device/src/index.ts (1)
13-13: LGTM! Appropriate export for the new component.The export of the
ContinueOnTrezorScreenContentcomponent allows it to be used by other modules in the codebase. This is consistent with the pattern of other component exports and supports the device tutorial feature being added.suite-native/device/tsconfig.json (1)
33-33: LGTM! TypeScript reference correctly added.The addition of the reference path to
../../packages/env-utilsis consistent with the new dependency in package.json and follows the pattern of other package references. This ensures proper TypeScript compilation support for the new dependency.suite-native/module-device-onboarding/tsconfig.json (1)
10-10: References properly updated for new dependencyThe addition of the connect package reference aligns with the corresponding package.json dependency addition, allowing proper TypeScript compilation for the device tutorial feature.
suite-native/module-device-onboarding/package.json (1)
23-23: Dependency correctly added for tutorial functionalityThe
@trezor/connectdependency has been properly added using workspace syntax, which is required for the device tutorial feature mentioned in the PR objectives.suite-native/intl/src/en.ts (1)
862-864: Added localization key for new tutorial screenThe new
deviceTutorialScreenentry with a clear, descriptive title matches the PR objective of adding a device tutorial to the onboarding flow.suite-native/navigation/src/navigators.ts (1)
131-131: New route added for device tutorialThe addition of
[DeviceOnboardingStackRoutes.DeviceTutorial]: undefined;to theDeviceOnboardingStackParamListtype correctly integrates the new tutorial screen into the navigation flow, consistent with the PR objective of adding a device onboarding tutorial.suite-native/device/src/components/ContinueOnTrezorScreenContent.tsx (3)
1-9: Import organization looks goodYour imports are well-organized, separating external libraries from internal modules.
11-13: Props interface is clear and minimalGood job keeping the props interface simple with an optional translation key.
22-44: Component implementation is clean and responsiveThe component is well-structured and follows responsive design principles:
- Uses translation keys for internationalization
- Properly applies styles
- Has responsive sizing based on screen height
- Uses Redux for device model information
suite-native/module-device-onboarding/src/screens/DeviceTutorialScreen.tsx (1)
28-32: Component rendering is clean and modularThe component properly leverages the
DeviceOnboardingScreenWithExitButtonandContinueOnTrezorScreenContentcomponents, providing a consistent UI experience with the rest of the application.suite-native/module-device-onboarding/src/navigation/DeviceOnboardingStackNavigator.tsx (2)
10-10: Import for new screen added correctlyThe import for the new
DeviceTutorialScreenis appropriately placed with the other screen imports.
43-46: New screen added to navigatorThe
DeviceTutorialScreenis correctly added to theDeviceOnboardingStack.Navigatorwith the proper route name.suite-native/module-device-onboarding/src/screens/FirmwareInstallationScreen.tsx (3)
2-6: Appropriate import updates for navigation supportThe imports have been updated to include navigation-related types from '@suite-native/navigation', replacing the previously used toast notifications. This aligns with the new navigation-based flow.
10-15: Well-typed navigation prop implementationThe component now properly accepts a navigation prop with appropriate TypeScript types, ensuring type safety when navigating between screens in the onboarding flow.
16-18: Flow improvement: Direct navigation to tutorialInstead of showing a toast notification after firmware installation, the component now navigates directly to the DeviceTutorial screen, creating a more streamlined user experience for device onboarding.
suite-native/module-device-onboarding/src/screens/ConfirmFirmwareUpdateScreen.tsx (5)
4-5: Proper import for focus effectThe useFocusEffect import is correctly maintained while other navigation-related imports have been updated.
15-16: Appropriate StackProps importThe StackProps type is properly imported to support the component's navigation prop typing.
20-25: Well-structured component with navigation propThe component now accepts navigation as a prop with correct TypeScript typing, which is more explicit and testable than using the useNavigation hook.
32-35: Improved flow for users with latest firmwareFor users who already have the latest firmware, the component now directly navigates to the tutorial screen rather than showing a notification. The dependency array has been properly updated to include navigation.
41-43: Consistent navigation to tutorialThe handleSkipUpdate function now navigates directly to the DeviceTutorial screen, maintaining a consistent navigation flow regardless of whether the user updates firmware or skips it.
suite-native/module-device-settings/src/screens/ContinueOnTrezorScreen.tsx (2)
1-1: Good component reuseImporting the extracted ContinueOnTrezorScreenContent component promotes code reuse across the application.
5-9: Clean component refactoringThe component has been simplified to a functional expression that directly returns JSX. This refactoring improves readability and maintainability by delegating the content rendering to the specialized ContinueOnTrezorScreenContent component.
suite-native/device/src/components/ContinueOnTrezorScreenContent.tsx
Outdated
Show resolved
Hide resolved
suite-native/module-device-onboarding/src/screens/DeviceTutorialScreen.tsx
Show resolved
Hide resolved
|
/rebase |
|
Start rebasing: https://github.com/trezor/trezor-suite/actions/runs/13833141952 |
trezor-ci
force-pushed
the
feat/native/device-onboarding-device-tutorial
branch
from
776dc30 to
18da698
Compare
PeKne
force-pushed
the
feat/native/device-onboarding-device-tutorial
branch
from
18da698 to
4875d2d
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
suite-native/module-device-onboarding/src/screens/DeviceTutorialScreen.tsx (1)
14-26:⚠️ Potential issueIncomplete implementation with missing error handling
There are two issues in the tutorial implementation:
- There's a TODO comment indicating that navigation to an authentication check screen is not implemented.
- The useEffect lacks error handling for the
TrezorConnect.showDeviceTutorialcall.Consider adding error handling and implementing the navigation:
useEffect(() => { const showTutorial = async () => { - await TrezorConnect.showDeviceTutorial({ device }); - showToast({ - message: 'TUTORIAL COMPLETED. TODO: navigate to auth. check screen.', - variant: 'success', - }); + try { + await TrezorConnect.showDeviceTutorial({ device }); + showToast({ + message: 'Tutorial completed', + variant: 'success', + }); + // Navigate to the security check screen + navigation.navigate(DeviceOnboardingStackRoutes.SecurityCheck); + } catch (error) { + showToast({ + message: 'Failed to display tutorial', + variant: 'error', + }); + } }; showTutorial(); // This use effect should be triggered only during the first render // eslint-disable-next-line react-hooks/exhaustive-deps }, []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
suite-native/device/package.json(1 hunks)suite-native/device/src/components/ContinueOnTrezorScreenContent.tsx(1 hunks)suite-native/device/src/index.ts(1 hunks)suite-native/device/tsconfig.json(1 hunks)suite-native/intl/src/en.ts(1 hunks)suite-native/module-dev-utils/tsconfig.json(1 hunks)suite-native/module-device-onboarding/package.json(1 hunks)suite-native/module-device-onboarding/src/navigation/DeviceOnboardingStackNavigator.tsx(2 hunks)suite-native/module-device-onboarding/src/screens/ConfirmFirmwareUpdateScreen.tsx(2 hunks)suite-native/module-device-onboarding/src/screens/DeviceTutorialScreen.tsx(1 hunks)suite-native/module-device-onboarding/src/screens/FirmwareInstallationScreen.tsx(1 hunks)suite-native/module-device-onboarding/tsconfig.json(1 hunks)suite-native/module-device-settings/src/screens/ContinueOnTrezorScreen.tsx(1 hunks)suite-native/navigation/src/navigators.ts(1 hunks)suite-native/navigation/src/routes.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- suite-native/module-device-onboarding/tsconfig.json
- suite-native/device/package.json
- suite-native/module-dev-utils/tsconfig.json
- suite-native/device/tsconfig.json
- suite-native/navigation/src/routes.ts
- suite-native/device/src/index.ts
- suite-native/navigation/src/navigators.ts
- suite-native/module-device-onboarding/package.json
- suite-native/intl/src/en.ts
- suite-native/device/src/components/ContinueOnTrezorScreenContent.tsx
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: PR-check / node-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / web-authorizeCoinjoin cancelCoinjoinAuthorization passphrase unlockPath setBusy checkFirmwareAuthenticity keepSession cancel.test info.test resetDevice-api
- GitHub Check: PR-check / node-override init-api-flaky
- GitHub Check: PR-check / web-override init-api-flaky
- GitHub Check: run-e2e-suite-desktop-tests (@group=wallet, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-e2e-suite-desktop-tests (@group=other, trezor-user-env-unix)
- GitHub Check: run-e2e-suite-desktop-tests (@group=passphrase, trezor-user-env-unix)
- GitHub Check: run-e2e-suite-desktop-tests (@group=settings, trezor-user-env-unix bitcoin-regtest)
- GitHub Check: run-e2e-suite-desktop-tests (@group=device-management, trezor-user-env-unix)
- GitHub Check: run-e2e-suite-desktop-tests (@group=suite, trezor-user-env-unix)
- GitHub Check: build-deploy
- GitHub Check: build-deploy
- GitHub Check: Setup and Cache Dependencies
- GitHub Check: transport-e2e-test
- GitHub Check: build-web
- GitHub Check: test
- GitHub Check: prepare_android_test_app
- GitHub Check: Analyze with CodeQL (javascript)
- GitHub Check: EAS Update
🔇 Additional comments (8)
suite-native/module-device-onboarding/src/navigation/DeviceOnboardingStackNavigator.tsx (1)
43-46: LGTM! The DeviceTutorial screen is properly integratedThe implementation correctly adds the new DeviceTutorialScreen to the navigation stack with the appropriate route name.
suite-native/module-device-settings/src/screens/ContinueOnTrezorScreen.tsx (1)
1-9: Good refactoring to improve code reuseThe changes simplify the implementation by extracting the rendering logic to a reusable component. This promotes better code organization and maintainability.
suite-native/module-device-onboarding/src/screens/ConfirmFirmwareUpdateScreen.tsx (3)
20-25: Improved component signature using StackPropsThe component now properly accepts navigation props via StackProps, which is a more type-safe approach.
30-35: Improved flow by replacing toast with direct navigationThe code now navigates directly to the DeviceTutorial screen instead of showing a toast message, creating a smoother user experience.
41-43: Consistent navigation to DeviceTutorial screenThe skip update handler correctly navigates to the DeviceTutorial screen, maintaining consistency with the other navigation logic.
suite-native/module-device-onboarding/src/screens/FirmwareInstallationScreen.tsx (3)
2-6: Good addition of navigation-related imports.The new imports from
@suite-native/navigationprovide the necessary types for proper navigation handling in the component. This is a good practice for type safety in TypeScript.
10-15: Well-typed component signature with navigation prop.The updated component signature now properly accepts a navigation prop with appropriate TypeScript typing. This enhances type safety and makes the component's dependencies explicit.
16-18: Improved user flow with direct navigation.Replacing the toast notification with direct navigation to the tutorial screen provides a more seamless user experience during the device onboarding process. This change aligns well with the PR objective of enhancing the device onboarding flow with a tutorial.
suite-native/module-device-onboarding/src/screens/DeviceTutorialScreen.tsx
Show resolved
Hide resolved
|
🚀 Expo preview is ready!
|
PeKne
force-pushed
the
feat/native/device-onboarding-device-tutorial
branch
from
4875d2d to
fc4b427
Compare
Description
Related Issue
Resolve #17628
Screenshots:
Screen.Recording.2025-03-13.at.11.08.43.AM.mov