-
Notifications
You must be signed in to change notification settings - Fork 2
oko 372 #143
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
base: main
Are you sure you want to change the base?
oko 372 #143
Conversation
- and add SignerAddressOrEmailForSiwe to be applied other style
- oko 504
- oko-503
| "oko-sandbox-evm": { | ||
| path: path.join(paths.root, "sandbox/sandbox_evm"), | ||
| }, |
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’m not sure we need to deploy sandbox_evm to Vercel. What are your thoughts?
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.
First, the reason this was added to the current code was that I needed to deploy sandbox_evm for QA purposes.
For EVM features like EIP-712 and SIWE signature UI, using sandbox_evm is definitely more convenient, so I think it could be useful in the future as well.
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.
Understood
| @@ -0,0 +1,35 @@ | |||
| import type { Theme } from "@oko-wallet/oko-common-ui/theme"; | |||
|
|
|||
| import { OKO_PUBLIC_S3_BUCKET_URL } from "@oko-wallet-attached/requests/endpoints"; | |||
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.
How about using static assets instead of fetching from S3?
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.
For some reasons (actually I don't remember detailed) we store image in S3.
so @eldenpark How about putting it in the codebase instead of S3?
| <picture> | ||
| <source srcSet={imageUrl.webp} type="image/webp" /> | ||
| <img | ||
| className={styles.image} | ||
| src={imageUrl.png} | ||
| alt="sign siwe title image" | ||
| /> | ||
| </picture> |
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.
It would also be good to use the newly updated ImageWithAlt component.
Pull Request
CONTRIBUTING.mdand followed the guidelines.Summary
Links (Issue References, etc, if there's any)