-
-
Notifications
You must be signed in to change notification settings - Fork 273
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): onboarding unitialized device landing screen #16616
base: develop
Are you sure you want to change the base?
Conversation
🚀 Expo preview is ready!
|
Beware, the "seedless" term can be confusing. Check https://trezor.io/learn/a/seedless-setup and try to search for "seedless" in our codebase... I believe it would be better to rename it to "uninitialized". |
@matejkriz Good point 👀, I will rename it |
|
!isDeviceInitialized && | ||
isDeviceConnected && | ||
isOnboardingFinished && | ||
!isNoPhysicalDeviceConnected && |
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.
Wouldnt !isPortfolioTrackerDevice
be better here? Or what is the reason for checking this?
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.
I tested this version originally and I had some problem with it. But now I retested that and it works as expected. Using this version in 94fe6b8. Good point.
suite-native/device/src/utils.ts
Outdated
case DeviceModelInternal.UNKNOWN: | ||
return false; | ||
default: { | ||
const exhaustiveCheck: never = model; |
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.
fyi you can also use UnreachableCaseError
from #16396
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.
const trezorModelImageMap = { | ||
[DeviceModelInternal.T2B1]: require('../assets/trezorSafe3.png'), | ||
[DeviceModelInternal.T3B1]: require('../assets/trezorSafe3.png'), | ||
[DeviceModelInternal.T3T1]: require('../assets/trezorSafe5.png'), |
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.
Please, name those images with model. E.g. T2B1
Also those images are pretty huge (2MB), is there a way to optimise the size?
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.
there is just a single image for two models (T2B1 and T3B1) so naming it like that does not make sense to me. I at least compressed the images in 94fe6b8
const trezorImageStyle = prepareNativeStyle<{ hasDeviceFirmwareInstalled: boolean }>( | ||
(_, { hasDeviceFirmwareInstalled }) => ({ | ||
width: '100%', | ||
height: hasDeviceFirmwareInstalled ? 280 : 360, |
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.
Will this work with system font size changed?
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.
What do you mean? The height will be the same on every device, but it has maximumHeight specified right below. Also the screen is scrollable, so if the content overflows on some super small device with an enlarged font, user will be able to scroll to see the content. Do I answer your question?
76c6f6b
to
94fe6b8
Compare
Description
Related Issue
Resolve #16278
Screenshots:
Firmware-less device
Device with firmware installed