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.

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

fix: only display provided chains in ChainModal #1156

Merged

Conversation

greg-schrammel
Copy link
Contributor

@greg-schrammel greg-schrammel commented Apr 8, 2023

I have a specific use case where the app is optimism only, but we provide a embedded socket.tech bridge widget
I don't want to confuse users enabling all chains to be connected thru the modal, but they must be supported and only switch when bridge asks to

if I do this:

const { chains, provider, webSocketProvider } = configureChains(
  [optimism, optimismGoerli, mainnet, polygon, arbitrum],
  [publicProvider()]
);

...

<RainbowKitProvider chains={[optimism, optimismGoerli]}>

I expect to see this
Screenshot 2023-04-08 at 18 20 26

but get this instead
Screenshot 2023-04-08 at 18 16 33

we could replace the provideRainbowKitChains in useRainbowKitChains with a map like

import { rainbowKitChainMetadata } from '...'

const chains = useRainbowKitChains()
// if the chain passed in the provider doesn't have metadata check if rainbowkit provides a default
const icons = chains.map(chain => chain.iconUrl || rainbowKitChainMetadata[chain.id]?.iconUrl) 

this refactor (that's not in this pr) would make the second screenshot have the chain logos

@greg-schrammel greg-schrammel requested a review from a team as a code owner April 8, 2023 21:36
@vercel
Copy link

vercel bot commented Apr 8, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
rainbowkit-example ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 6, 2023 6:40pm
rainbowkit-site ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 6, 2023 6:40pm

@vercel
Copy link

vercel bot commented Apr 8, 2023

@greg-schrammel is attempting to deploy a commit to the rainbowdotme Team on Vercel.

A member of the Team first needs to authorize it.

Comment on lines -37 to -68
const stopSwitching = useCallback(() => {
setSwitchingToChain(null);
onClose();
}, [onClose]);

useEffect(() => {
if (!activeConnector) {
return;
}

const stopSwitching = () => {
setSwitchingToChain(null);
onClose();
};

let provider: any;
activeConnector?.getProvider?.().then((provider_: any) => {
provider = provider_;
provider.on('chainChanged', stopSwitching);
});

return () => {
provider?.removeListener('chainChanged', stopSwitching);
};
}, [activeConnector, onClose, stopSwitching]);

useEffect(() => {
if (networkError && networkError.name === 'UserRejectedRequestError') {
stopSwitching();
}
}, [networkError, stopSwitching]);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty sure this is the same as this

const { chains, pendingChainId, reset, switchNetwork } = useSwitchNetwork({
    onSettled: () => {
      reset(); // reset mutation variables (eg. pendingChainId)
      onClose();
    },
  });

didn't find any differences in my testing, but easy to revert if the useEffects where proposital

@DanielSinclair
Copy link
Contributor

Looking good 👌
Would just need you to create patch changeset from pnpm changeset that explains the bug fix. This may also be a good first component unit test since we had missed the chains mismatch scenario. Think you can spin up a @testing-library/react setup and test?

@socket-security
Copy link

socket-security bot commented Apr 13, 2023

No dependency changes detected. Learn more about Socket for GitHub ↗︎

👍 No dependency changes detected in pull request

@greg-schrammel
Copy link
Contributor Author

done! I'm not good with changeset messages, open to rephrasing
also first time setting up tests with wagmi, lmk what you think

@DanielSinclair DanielSinclair changed the title fix(ChainModal): only display provided chains fix: only display provided chains in ChainModal Jul 6, 2023
@DanielSinclair DanielSinclair added the ready to merge Ready for final PR review and merge label Jul 6, 2023
@DanielSinclair DanielSinclair linked an issue Jul 6, 2023 that may be closed by this pull request
1 task
@DanielSinclair DanielSinclair merged commit 08e3f4c into rainbow-me:main Jul 7, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Ready for final PR review and merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug] Error disconnecting Trust Wallet
2 participants