-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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
explorer: use wallet kit #6520
explorer: use wallet kit #6520
Conversation
7ef90ea
to
e8676e9
Compare
e8676e9
to
69bf4d9
Compare
69bf4d9
to
4cd5eb2
Compare
4cd5eb2
to
c6ca12d
Compare
c6ca12d
to
5527b25
Compare
Looks like the wallet kit backdrop isn't visible, maybe needs a higher z-index? |
connected ? st.connected : st.connect, | ||
st.button | ||
)} | ||
type="button" |
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.
We should add this to the underlying wallet kit buttons
} | ||
size="md" | ||
className={clsx( | ||
connected ? st.connected : st.connect, |
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.
Can we use inline tailwind?
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.
I've used inline before but was't working properly when building in prod mode. Then moved it to css modules to apply !important
. I'll see if I can do something better
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.
Done (inline and important)
5527b25
to
b52ce99
Compare
b52ce99
to
07b7486
Compare
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.
Some nits but not blocking, LGTM overall
@@ -25,6 +25,15 @@ const argsSchema = z.object({ | |||
params: z.array(z.object({ value: z.string().trim().min(1) })), | |||
}); | |||
|
|||
const connectButtonStyles = cva('!text-bodySmall !rounded-md', { |
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.
FWIW for cases like this, I think it's often cleaner to just use clsx
inline. CVA is really for component definitions where we want to extract variant props, not for arbitrary application of classes.
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.
Makes sense thanks! I will update it
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.
Done
* the account modal was visible for a bit after connecting to a wallet from the Connect modal before it closes. This fixes it.
* addresses PR comment * also fixes hover styling
07b7486
to
ef119db
Compare
* fixes form validation
* wallet-kit: prevent showing Account modal after connecting * the account modal was visible for a bit after connecting to a wallet from the Connect modal before it closes. This fixes it. * explorer: migrate to wallet-kit * wallet-kit: dialog backdrop visible and clickable * explorer: use clsx instead of cva * addresses PR comment * also fixes hover styling * explorer: fixes executing tx for functions with no arguments * fixes form validation * wallet-kit: connect button defaults to type button
the issue
Screen.Recording.2022-12-01.at.20.58.52.mov
and makes wallet-kit backdrop clickable and visible
Screen.Recording.2022-12-02.at.11.52.10.mov
Screen.Recording.2022-12-02.at.11.52.58.mov
Screen.Recording.2022-12-02.at.11.55.57.mov
Closes: APPS-245