-
Notifications
You must be signed in to change notification settings - Fork 597
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
add option to pass scwOnboardMode to eth_requestAccounts #1383
Conversation
✅ Heimdall Review Status
|
const scwOnboardMode = Array.isArray(args?.params) | ||
? (args.params[0]?.scwOnboardMode as ScwOnboardMode) | ||
: undefined; | ||
const options: { scwOnboardMode: ScwOnboardMode } = scwOnboardMode | ||
? { scwOnboardMode } | ||
: { scwOnboardMode: 'default' }; | ||
const signerType = await this.requestSignerSelection(options); |
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.
hmm, this should work. potentially more clear
const scwOnboardMode = Array.isArray(args?.params) | |
? (args.params[0]?.scwOnboardMode as ScwOnboardMode) | |
: undefined; | |
const options: { scwOnboardMode: ScwOnboardMode } = scwOnboardMode | |
? { scwOnboardMode } | |
: { scwOnboardMode: 'default' }; | |
const signerType = await this.requestSignerSelection(options); | |
const scwOnboardMode = args?.params?.[0]?.scwOnboardMode; | |
const options = { scwOnboardMode: scwOnboardMode ?? 'default' as ScwOnboardMode }; | |
const signerType = await this.requestSignerSelection(options); |
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.
Ah yeah this is much cleaner! For line 58 though, TS complains because expression of type '0' can't be used to index type 'object | readonly unknown[]'.
Even though in practice eth request params are usually (always?) structured as an object inside an array, the eip-1193 provider spec say params
is an array or object so we gotta respect the possibility than an object could be passed as params
Line 59 is nice! Here's that change. 5c9c0ac
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.
Looks good, one comment to consider. Could you run a manual test on the final version for each:
- create passed in
- default passed in
- nothing passed in
and cover each by unit tests as well?
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.
Looks great
…)" This reverts commit 5d54a0a.
* add option to pass scwOnboardMode to eth_requestAccounts * readability * add default / no params test cases
Summary
Adding the option for dapps to take users directly to Smart Wallet creation by passing a custom
scwOnboardMode
param to eth_requestAccountsHow did you test your changes?
unit tests & locally
Create Button manual test
Screen.Recording.2024-09-04.at.6.50.26.PM.mov
Manual test of all 3 param possibilities
Screen.Recording.2024-09-05.at.10.23.15.AM.mov