[READY] filter out Nova Wallet option if not in the Nova app#635
Conversation
✅ Deploy Preview for pendulum-pay ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Sharqiewicz
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR filters out the Nova Wallet connection option when the dApp is not opened in the Nova app.
- Introduces a global declaration for window.walletExtension with the isNovaWallet property.
- Adds helper functions filterNovaWallet and getFilteredWallets to control wallet visibility.
- Replaces the direct wallet filtering logic with the new getFilteredWallets function.
ebma
left a comment
ebma
left a comment
There was a problem hiding this comment.
Let's address the nit from copilot but rest looks good to me ✅
| } else { | ||
| localStorage.setItem(BRLA_KYC_FORM_STORAGE_KEY, JSON.stringify({ taxId })); | ||
| } | ||
| }, [setValue, taxId]); |
There was a problem hiding this comment.
Maybe we could ignore here reacting to taxId and setting? I think the effect bellow (with the subscription) should handle already setting the taxId, or do we need both effects for different scenarios?
There was a problem hiding this comment.
The first useEffect is used for the form initialization with the data stored in the LocalStorage. I see it might look confusing so I reorganized the code in a better way now to reduce the cognitive load. let me know if it looks better for you now
There was a problem hiding this comment.
Yes I was just confused about the else branch of the first one (where the storage is set), and the second useEffect that is also subscribed to changes to the form via watch. But I see now that the second one will set the storage not only when savedData doesn't exists but on any refresh of the form. Nevermind then, let's go!
|
Sentry timeout error failed the pipeline |
…f the kyc process
…wser' of github.com:pendulum-chain/vortex into 545-check-flag-if-vortex-app-is-opened-in-nova-dapp-browser
|
@pendulum-chain/devs Can I merge? |
ebma
left a comment
ebma
left a comment
There was a problem hiding this comment.
I tested a bit and seems to do what it's supposed to do. I think the localStorage logic is a bit scattered but I'm fine if we keep it that way.
ebma
deleted the
545-check-flag-if-vortex-app-is-opened-in-nova-dapp-browser
branch
Show Nova Wallet connection option only when dApp is open in Nova app.