-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: add loading state to useWeb3ModalState hook #1911
feat: add loading state to useWeb3ModalState hook #1911
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
@boidushya we'd need some docs update on this right? |
yep! This relevant to React, Next & JS right? I'll go ahead and open a PR. |
f3a8420
to
745b668
Compare
745b668
to
4835511
Compare
@tomiir @svenvoskamp @ignaciosantise I think not it's handling loading better. Could you review it again? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ignaciosantise for a double check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 looking good!
just left you a question
Can we add a small description on what this state is tracking? It's not 100% clear to me |
5986bfb
to
f018d16
Compare
Breaking Changes
We are storing the
loading
state inside our scaffold which is managed inPublicStateController
to make conditional renderings in UI depending on the client's (ethers/wagmi) loading statuses. Whileopen
/close
states are exposed to hooks for developers, thisloading
state not exposed.This PR introduces changes to share the
loading
state to the hooks.Changes
loading
state touseWeb3ModalState
hook