Option to select xverse as default browser wallet in settings #774
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
victorkirov
commented
Jan 29, 2024
| }); | ||
| }); | ||
| } | ||
|
|
||
| removeItem(key: string): Promise<any> { | ||
| removeItem(key: string): Promise<void> { |
There was a problem hiding this comment.
I tried changing this and the setItem one to boolean, but the seedVault stuff started to complain. Checking the rest of the code, nothing was actually using the return value, so I removed it.
victorkirov
commented
Jan 29, 2024
| }); | ||
| }); | ||
| } | ||
|
|
||
| getItem<T = any>(key: string): Promise<T> { | ||
| getItem<T = any, D = undefined>(key: string, defaultValue?: D): Promise<T | D> { |
There was a problem hiding this comment.
By default, the storage interface returns an empty object if the key is not present, resulting in a response of undefined. The Seed Vault expects null though, so I had to add this default value.
…s-default-browser-wallet-in-the
victorkirov
requested review from
m-aboelenein
and removed request for
m-aboelenein
|
Test with build here: https://github.com/secretkeylabs/xverse-web-extension/actions/runs/7738374490 |
m-aboelenein
approved these changes
Feb 1, 2024
m-aboelenein
deleted the
victor/eng-3649-option-to-select-xverse-as-default-browser-wallet-in-the
branch
|
@m-aboelenein merged before it was tested! |
I tested this one, and the change is pretty minimal we can retest with the RC |
This was referenced Feb 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🔘 PR Type
What kind of change does this PR introduce?
📜 Background
When a user connects to an extension wallet, if there are multiple wallets implementing the sats connect interface, then the last wallet loaded would get all event messages. A user may want to force Xverse as the priority wallet to intercept these messages if the app using sats connect is using the default providers.
🔄 Changes
This adds a new setting toggle to set Xverse as the priority sats-connect wallet.
Note that the setting is stored in chrome storage so that it's available from the content-script directly without having to send messages around.
🖼 Screenshot / 📹 Video
✅ Review checklist
Please ensure the following are true before merging: