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

[Issue #2565] network config extended #2609

Merged

Conversation

Da-Colon
Copy link
Contributor

@Da-Colon Da-Colon commented Dec 6, 2024

Merges with #2595
Discussed in this comment #2595 (comment)

This PR will cover any work needed to ensure that the network is properly being used in the App.

@Da-Colon Da-Colon self-assigned this Dec 6, 2024
Copy link

netlify bot commented Dec 6, 2024

Deploy Preview for decent-interface-dev ready!

Name Link
🔨 Latest commit 89b2989
🔍 Latest deploy log https://app.netlify.com/sites/decent-interface-dev/deploys/67604703f1b18f000857f071
😎 Deploy Preview https://deploy-preview-2609.app.dev.decentdao.org
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Da-Colon Da-Colon marked this pull request as draft December 6, 2024 21:43
Comment on lines +12 to +21
const { switchChain } = useSwitchChain({
mutation: {
onError: () => {
if (addressPrefix !== urlAddressPrefix && urlAddressPrefix !== undefined) {
const chainId = getChainIdFromPrefix(urlAddressPrefix);
switchChain({ chainId });
}
},
},
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Funny enough, this kinda reverts back to how the file was before only this now we are attempting the switch chain again onError.

While, not ideal. this does work and fixes the current network switcher issues we've been having. Ideally I think we would have completely seperate publicClient by passing the id directly in. and then move all network switching to when a user clicks to send a transaction.

But I'm not sure how that scales at the moment and want to discuss further before we make that change.

So this is the current path of least resistance. currently there are two spots that this is going to be required.

  • useAutomaticSwitchChain
  • EstablishEssentials where a new network switcher will exist.

Copy link
Contributor Author

@Da-Colon Da-Colon left a comment

Choose a reason for hiding this comment

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

Another. I'll create any other PRs for anything pre-existing issues off develop I find while testing so keep an eye out for those.

And one PR branched from here to updates the DAO search to be cross-chain per @adamgall suggestion.

  • I have another commit to add the SwitchChain to the network switcher in DAO creation coming and this will be good for other testers starting tomorrow

@Da-Colon Da-Colon marked this pull request as ready for review December 12, 2024 14:09
@Da-Colon
Copy link
Contributor Author

Okay, I feel like this is good as its going to get in its current form. There is a flash of the network error screen, but that was always there before, but now it actually loads. I think the state refactor will help this get rid of the flash.

Copy link
Contributor

@mudrila mudrila left a comment

Choose a reason for hiding this comment

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

1 very minor comment, but otherwise - looks good!

transports: supportedNetworks.reduce(transportsReducer, {}),
batch: {
multicall: true,
},
});

if (walletConnectProjectId) {
createWeb3Modal({ wagmiConfig, projectId: walletConnectProjectId });
createWeb3Modal({ wagmiConfig, projectId: walletConnectProjectId, metadata: metadata });
Copy link
Contributor

Choose a reason for hiding this comment

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

Can use prop shorthand here as well

@Da-Colon Da-Colon merged commit c7b41c5 into issue/2565-network-config-updates Dec 16, 2024
7 checks passed
@Da-Colon Da-Colon deleted the issue/2565-network-config-extended branch December 16, 2024 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants