Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Allow Disconnect of MWA #960

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Allow Disconnect of MWA #960

Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
3cdb221
can disconnect
alex-fung Apr 25, 2024
4ea7236
add back setting unloading to flase
alex-fung Apr 26, 2024
f61090c
default to mwa if no other adapters around
alex-fung Apr 26, 2024
5642ba7
always disconect
alex-fung Apr 26, 2024
c72209f
preserve old logic, but only if adapters with standard wallet adapter…
alex-fung May 9, 2024
1651cbd
need to flip the boolean
alex-fung May 9, 2024
af496bd
remove MWA checks outright
alex-fung May 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 14 additions & 4 deletions packages/core/react/src/WalletProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ export function WalletProvider({
}, [adaptersWithStandardAdapters, mobileWalletAdapter]);
const [walletName, setWalletName] = useLocalStorage<WalletName | null>(
localStorageKey,
getIsMobile(adaptersWithStandardAdapters) ? SolanaMobileWalletAdapterWalletName : null
adaptersWithStandardAdapters.length === 0 && getIsMobile(adaptersWithStandardAdapters)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. This has the effect of, if any adapter is included, to not select MWA. The problem with this in practice is that almost all dapps will include adapters, even ones they don't need because they've been deprecated by Standard Wallets, or ones that simply don't work at all on the platform. This is especially true because most dapps I've seen don't use separate mobile builds with conditional compilation of included adapters.

This basically means that it's always going to be false, and we're never even going to do the getIsMobile check. Now, I'm probably fine with eliminating all preference for MWA, but I want to be clear that's what we're doing here, and if so, we might as well just do it outright and remove these checks.

? SolanaMobileWalletAdapterWalletName
: null
);
const adapter = useMemo(
() => adaptersWithMobileWalletAdapter.find((a) => a.name === walletName) ?? null,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One possibility for this: if the only item in adaptersWithMobileWalletAdapter is MWA, could we select it by default?

This preserves the existing behavior if the user is on Android mobile (so no other wallets are being injected), the app is passing wallets={[]}, and no other Standard Wallets are registered. I don't know how common this is, but it might not hurt?

Expand Down Expand Up @@ -106,7 +108,12 @@ export function WalletProvider({
function handleDisconnect() {
if (isUnloadingRef.current) return;
// Leave the adapter selected in the event of a disconnection.
if (walletName === SolanaMobileWalletAdapterWalletName && getIsMobile(adaptersWithStandardAdapters)) return;
if (
walletName === SolanaMobileWalletAdapterWalletName &&
adaptersWithStandardAdapters.length === 0 &&
getIsMobile(adaptersWithStandardAdapters)
)
return;
setWalletName(null);
}
adapter.on('disconnect', handleDisconnect);
Expand Down Expand Up @@ -150,11 +157,14 @@ export function WalletProvider({
};
}, [adaptersWithStandardAdapters, walletName]);
const handleConnectError = useCallback(() => {
if (adapter && adapter.name !== SolanaMobileWalletAdapterWalletName) {
if (
adapter &&
(adapter.name !== SolanaMobileWalletAdapterWalletName || adaptersWithStandardAdapters.length !== 0)
) {
// If any error happens while connecting, unset the adapter.
changeWallet(null);
}
}, [adapter, changeWallet]);
}, [adapter, changeWallet, adaptersWithStandardAdapters]);
const selectWallet = useCallback(
(walletName: WalletName | null) => {
hasUserSelectedAWallet.current = true;
Expand Down