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

[Bugfix | Issue #2565] Network Switching and context to store update #2595

Merged

Conversation

Da-Colon
Copy link
Contributor

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

Closes #2565

Updates the Network Provider to zustand store

Now switching networks don't cause a massive tree re-render

Fixes App Crash when navigating to a DAO on a different network via links, refresh or external link

We will need to test with other wallets, but now that 'being connected to a network' is decoupled from the choice in the wallet we don't need to use switchchain anymore

Screenshots

DAO switching

image

image

image

@Da-Colon Da-Colon added the bug Something isn't working label Dec 5, 2024
@Da-Colon Da-Colon self-assigned this Dec 5, 2024
@Da-Colon Da-Colon changed the title [Bugfix | Issue #2565] Network Switching and Context to Store update [Bugfix | Issue #2565] Network Switching and context to store update Dec 5, 2024
Copy link

netlify bot commented Dec 5, 2024

Deploy Preview for decent-interface-dev ready!

Name Link
🔨 Latest commit 63c1575
🔍 Latest deploy log https://app.netlify.com/sites/decent-interface-dev/deploys/6761c0d94ada6d00084fb0bf
😎 Deploy Preview https://deploy-preview-2595.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.


// Create the Zustand store
export const useNetworkConfigStore = create<NetworkConfigStore>(set => ({
...sepoliaConfig,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

defaults to sepolia...though probably should set this up pending on env? the only place this would be noticeable would be the network switcher...

Speaking of....we can probably get rid of that.....

and if we get rid of that we should probably add something somewhere to indicate what network you are on more explictly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@decentdao/design

Copy link
Member

@adamgall adamgall left a comment

Choose a reason for hiding this comment

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

Maybe it's just me, but selecting a network in the in-app network switcher doesn't work. I start on Sepolia, try to switch to Base, but when I open the dropdown again I'm still on Sepolia.

@Da-Colon
Copy link
Contributor Author

Da-Colon commented Dec 5, 2024

I didn't update the network switcher to work with this setup. The app no longer cares what network the wallet is 'connected' to

@adamgall adamgall self-requested a review December 5, 2024 21:56
@Da-Colon
Copy link
Contributor Author

Da-Colon commented Dec 5, 2024

Discussed further with @nicolaus-sherrill the current plan is:

  • Rip out Network Selector from Wallet Menu
  • Add Network Selector to DAO Essentials.
  • Add Network indictor (icon?) to DAO Home

@nicolaus-sherrill
Copy link

@Da-Colon

  • Network switcher removed from top nav components.
Screenshot 2024-12-05 at 4 26 33 PM
  • Network icon added to DAO dashboard info tile. Component updated with padding reduced to 1rem and favorite icon shifted to right, next to settings. Component linked here
Screenshot 2024-12-05 at 4 30 53 PM
  • Create/edit a DAO workflow updated to include the network selector at the beginning of the flow. Ready in Handoff.
  • Selector dropdown pill component updated with a token and a network variant here.
    • (this will be consolidated with other component classes in the near future)

@Da-Colon
Copy link
Contributor Author

Da-Colon commented Dec 5, 2024

@Da-Colon

  • Network switcher removed from top nav components.
Screenshot 2024-12-05 at 4 26 33 PM * Network icon added to DAO dashboard info tile. Component updated with padding reduced to 1rem and favorite icon shifted to right, next to settings. [Component linked here](https://www.figma.com/design/ur5hElxplkmlY9gnGKlwrm/Components-%26-Styles---Decent-Design-System-1.1.1?m=auto&node-id=2248-25539&t=u0dzpMgPBzXdWZzc-1) Screenshot 2024-12-05 at 4 30 53 PM * Create/edit a DAO workflow updated to include the network selector at the beginning of the flow. [Ready in Handoff.](https://www.figma.com/design/9hUYtvCKjOyB92vt8ukYtD/Product-Handoff?m=auto&node-id=7208-30777&t=rTmswoFYVnJHrYhy-1) * Selector dropdown pill component updated with a token and a network variant [here](https://www.figma.com/design/ur5hElxplkmlY9gnGKlwrm/Components-%26-Styles---Decent-Design-System-1.1.1?m=auto&node-id=4329-422&t=u0dzpMgPBzXdWZzc-1). * * (this will be consolidated with other component classes in the near future)

fantastic, thank you sir

@Da-Colon Da-Colon marked this pull request as draft December 5, 2024 22:43
Copy link
Member

@adamgall adamgall left a comment

Choose a reason for hiding this comment

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

Have an issue loading a DAO name on Base, in the search menu.

Here's how I got to this situation:

  • fresh instance of https://deploy-preview-2595.app.dev.decentdao.org/
  • searched for "decentdao.eth" -> it popped up and the name loaded -> clicked it to go to the DAO. Saved it as a "Favorite".
  • went back to home page
  • pasted in 0x374D5a17D2433415C48C735A493F0523DaB99041 to the search bar, which is the Base Myosin DAO address
  • screenshot 👇

Screenshot 2024-12-16 at 2 24 36 PM

@Da-Colon
Copy link
Contributor Author

@Da-Colon
Copy link
Contributor Author

Da-Colon commented Dec 16, 2024

weird locally its working fine and can't reproduce. @adamgall, but I can reproduce on the deploy preview

image

hmmm preview is on the correct commit hmm

I can't switch directly into searching for decentdao.eth as well

image

Finds both locally.

@Da-Colon Da-Colon requested a review from adamgall December 16, 2024 21:30
Copy link
Contributor

@DarksightKellar DarksightKellar left a comment

Choose a reason for hiding this comment

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

Approving, but something's off with Base Myosin DAO. Navigating directly to https://deploy-preview-2595.app.dev.decentdao.org/home?dao=base:0x374D5a17D2433415C48C735A493F0523DaB99041 results in a white screen of death. Similar weird behaviour on dev and prod actually. However clicking into the DAO from saved list (after searching if on fresh cache) works fine.

@adamgall
Copy link
Member

https://deploy-preview-2595.app.dev.decentdao.org/home?dao=base:0x374D5a17D2433415C48C735A493F0523DaB99041

I'm not getting any issues with this direct link... @DarksightKellar can you take a video of what you're experiencing

@Da-Colon
Copy link
Contributor Author

@DarksightKellar Thanks, I can reproduce. On it

@DarksightKellar
Copy link
Contributor

@DarksightKellar Thanks, I can reproduce. On it

Weird lol. Curious what's going on there

@Da-Colon
Copy link
Contributor Author

@DarksightKellar I figured it out. Soon I saw the white screen I suspected something was up with around the wallet and it checking the connection. if it tries to switch chain prior to being ready, I believe it catches it self in a loop. I think the first attempt (first commit I just did) adding isFetched worked and I got eager and tested before it was deployed, I switched to isFetchedAfterMount which makes sense anyway here.

Anyway, fixed

@Da-Colon Da-Colon merged commit fec54ef into issue/noIssue-state-loading-cleanup Dec 17, 2024
7 checks passed
@Da-Colon Da-Colon deleted the issue/2565-network-config-updates branch December 17, 2024 18:51
@DarksightKellar
Copy link
Contributor

Works!

@Da-Colon In other news, would you know anything about this intermittent toast? Also only on myosin:

Screenshot 2024-12-17 at 18 36 54

@Da-Colon
Copy link
Contributor Author

yeah preexisting. I had made some updates, but I guess there is still something trying to load in state and not getting reset. I'll take a look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug Network Autoswitch not working properly
5 participants