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

[REFACTOR AND UPGRADE] Rainbowkit -> Web3Modal #1354

Closed
wants to merge 10 commits into from

Conversation

Da-Colon
Copy link
Contributor

@Da-Colon Da-Colon commented Feb 16, 2024

Pull Request: Wallet Provider Upgrade and Dependency Updates

Description

This pull request introduces a significant update and upgrade to our wallet provider interface, aiming to enhance the WalletConnect functionality for users. The integration of the latest dependencies required extensive updates across multiple files due to the breaking nature of these changes. Notably, this overhaul necessitated an update to the Node.js version, alongside modifications to the .nvmrc file and the engines field in package.json.

Efforts were made to retain the existing settings and behavior of rainbowkit to ensure that users experience no perceptible difference in the interface's feel.

I will highlight additional specifics in a separate code review within this pull request.

Notes

  • WalletConnect has been tested using Mobile MetaMask. Although the connection process can be slow, it functions correctly without the need for a page refresh. Occasionally, the 'create proposal' action may require a second attempt to trigger.
  • No issues have been observed with MetaMask's functionality.
  • After executing a Multisig proposal, there is a known issue where the proposal page does not automatically refresh.

Testing

Comprehensive testing will need to be done:

  • Ability to create a proposal on various wallet interfaces, use the Settings page for convenience.
  • Voting functionality on proposals.
  • Compatibility and functionality tests for WalletConnect across different wallets and platforms.
  • Specific tests for Multisig DAO and Azorious DAO functionalities.

For best experience I'd test using #1355

Screenshots


- remove @rainbow-me/rainbowkit
- added @tanstack/react-query
- added @web3modal/wagmi
- updated next @13.3.0 -> @14.0.3
- Updates node version in nvmrc and engine to compatible version
Copy link

netlify bot commented Feb 16, 2024

Deploy Preview for fractal-framework-dev ready!

Name Link
🔨 Latest commit 46dd276
🔍 Latest deploy log https://app.netlify.com/sites/fractal-framework-dev/deploys/65df46be28b7d5000780b065
😎 Deploy Preview https://deploy-preview-1354.app.dev.fractalframework.xyz
📱 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.

</WagmiConfig>
</FractalErrorBoundary>
</ChakraProvider>
<Providers>{children}</Providers>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was prompted by seeing why the Header was seemingly mounting differently. While this didn't solve the issue I was looking for. pulling the imports out of the app directly seems to speed up a little how the app fast the app is ready

@@ -15,6 +15,5 @@ module.exports = {
},
experimental: {
esmExternals: false,
appDir: true,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer required as of version 13.4

"@safe-global/safe-deployments": "^1.23.0",
"@safe-global/safe-ethers-lib": "^1.9.2",
"@safe-global/safe-service-client": "^1.5.2",
"@sentry/react": "^7.42.0",
"@sentry/react": "^7.101.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sentry needed to be updated because of the node version update

Comment on lines +128 to +129
const wagmiChainId = useChainId();
const chainId = wagmiChainId ? wagmiChainId : disconnectedChain.id;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just in case this gets called out, I didn't want to mess with the current functionality and didn't want to miss with how the app interacts with the provider in this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't want to spend the time upgrading. If we need to create a new test wallet using Web3Modal tooling


function Header() {
return (
<ClientOnly>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See more about this change in #1355

@@ -32,7 +33,6 @@ export default function useDAOName({
const { data: ensName } = useEnsName({
address: address as Address,
chainId: networkId,
cacheTime: 1000 * 60 * 30, // 30 min
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems that the cacheTime property was removed from all the wagmi hooks

@adamgall adamgall changed the title [REFACTOR AND UPGRADE] Rainbowkit -> Web3Modal [TSK-2] [REFACTOR AND UPGRADE] Rainbowkit -> Web3Modal Feb 19, 2024
Copy link

Untitled

@adamgall adamgall changed the title [TSK-2] [REFACTOR AND UPGRADE] Rainbowkit -> Web3Modal [REFACTOR AND UPGRADE] Rainbowkit -> Web3Modal Feb 19, 2024
@Da-Colon Da-Colon marked this pull request as draft February 23, 2024 18:56
@adamgall adamgall force-pushed the dc/upgrade-rainbowkit branch from b5d2f6c to 46dd276 Compare February 28, 2024 14:44
@Da-Colon Da-Colon closed this Mar 4, 2024
@adamgall adamgall added maintenance Keep the lights on and removed enhancement New feature or request labels Mar 4, 2024
@adamgall adamgall deleted the dc/upgrade-rainbowkit branch March 25, 2024 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Keep the lights on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants