-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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: QR hardware signing in new designs #13261
base: main
Are you sure you want to change the base?
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
Bitrise❌❌❌ Commit hash: 86a442b Note
Tip
|
minHeight: '68%', | ||
maxHeight: '68%', |
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.
is there a reason for the fixed height? I was able to remove this in this PR and it seems to work. This way no excess space for confirmations with less content.
I do have a PR that is open that applies the BottomSheet and scroll container here. With this, the header and footer are also fixed whilst having scrollable content. We will need to resolve the conflict in one of our PRs
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.
min height helpe confirmation page not look too short if there is less content
yep we will need to resolve conflicts
<Title /> | ||
<View style={styles.scrollWrapper}> | ||
<ScrollView> | ||
<View style={styles.scrollableSection}> |
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.
ScrollView accepts style, contentContainerStyle, and other style props that can be used to avoid adding extra View components around or within ScrollView
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.
good idea, I could remove inner view, but not outer one.
Bitrise QA builds https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/106d1a13-fdd2-4671-9391-1ca33b3930af?tab=workflows iPhone 15, Samsung S24+ All QR signatures work as expected on both platforms, except for the Android: android.moviOS: ios.mov |
Hey @sleepytanya : typed sign works for me using Airgap vault: can you plz check if you notice this in old designs also, this may be a thing I need to check with hardware team |
Quality Gate passedIssues Measures |
Description
QR hardware signing in new designs
Related issues
Fixes: https://github.com/MetaMask/MetaMask-planning/issues/4058
Manual testing steps
Screenshots/Recordings
TODO
Pre-merge author checklist
Pre-merge reviewer checklist