-
Notifications
You must be signed in to change notification settings - Fork 231
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/nft checkout plugin #2018
base: master
Are you sure you want to change the base?
Feat/nft checkout plugin #2018
Conversation
@@ -27,6 +27,7 @@ export type PluginNamespace = (typeof PLUGIN_NAMESPACES)[keyof typeof PLUGIN_NAM | |||
|
|||
export const EVM_PLUGINS = { | |||
WALLET_SERVICES: "wallet-services", | |||
NFT_CHECKOUT: "nft-checkout", |
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 nft checkout limited to evm?
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 about wallet services?
@@ -398,13 +398,13 @@ export class Web3AuthNoModal extends SafeEventEmitter<Web3AuthNoModalEvents> imp | |||
private connectToPlugins(data: { adapter: WALLET_ADAPTER_TYPE }) { | |||
Object.values(this.plugins).map(async (plugin) => { | |||
try { | |||
if (!plugin.SUPPORTED_ADAPTERS.includes(data.adapter)) { | |||
if (!plugin.SUPPORTED_ADAPTERS.includes("all") && !plugin.SUPPORTED_ADAPTERS.includes(data.adapter)) { |
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 think it may be better to list out all possible adapters instead of a separate value called "all". We can put it in a constant
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.
we have dynamic adapters (e.g. injected wallet extensions like Metamask...), so cannot list them out.
return; | ||
} | ||
if (plugin.status === PLUGIN_STATUS.CONNECTED) return; | ||
const { authInstance } = this.walletAdapters[this.connectedAdapterName] as AuthAdapter; | ||
const { options, sessionId, sessionNamespace } = authInstance || {}; | ||
await plugin.initWithWeb3Auth(this, options.whiteLabel); | ||
await plugin.initWithWeb3Auth(this, options?.whiteLabel); | ||
await plugin.connect({ sessionId, sessionNamespace }); |
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.
considering that nft checkout plugin connect function takes no arguments should we update IPlugin interface to make it optional?
Motivation and Context
Jira Link: https://toruslabs.atlassian.net/browse/PD-3995
Description
How has this been tested?
Screenshots (if appropriate):
Types of changes
Checklist: