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

chore: Update Wallet disconnect SVG #1103

Merged
merged 14 commits into from
Aug 21, 2024
Merged

Conversation

cpcramer
Copy link
Contributor

@cpcramer cpcramer commented Aug 19, 2024

What changed? Why?
https://linear.app/coinbase/issue/BOE-326/[wallet]-update-the-disconnect-icon-to-an-updated-icon

We are upgrading to a slightly larger SVG image.
Storybook:
Screenshot 2024-08-19 at 9 29 32 PM

Screenshot 2024-08-20 at 9 22 17 PM
Before After
Screenshot 2024-08-19 at 4 52 51 PM Screenshot 2024-08-19 at 4 52 59 PM
Before After
Screenshot 2024-08-19 at 4 53 18 PM Screenshot 2024-08-19 at 4 53 27 PM
**Notes to reviewers**

How has it been tested?

Copy link

vercel bot commented Aug 19, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
onchainkit-coverage ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 21, 2024 4:24am
onchainkit-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 21, 2024 4:24am
onchainkit-routes ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 21, 2024 4:24am

@@ -6,16 +6,16 @@ export const disconnectSvg = (
aria-label="ock-disconnect-svg"
width="100%"
height="100%"
viewBox="0 0 20 20"
viewBox="0 0 16 20"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a story for this component on Storybook?

Copy link
Contributor Author

@cpcramer cpcramer Aug 20, 2024

Choose a reason for hiding this comment

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

No, we do not have a storybook for any of our internal SVGs or our Wallet Components. Should I try to make one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a Fix Storybook task on Linear but it's getting deprioritized for tasks such as fix landing page and the upcoming Swap Component changes https://linear.app/coinbase/issue/BOE-313/fix-storybook

Copy link
Contributor

Choose a reason for hiding this comment

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

My comment was more about. Go make a storybook page for every component you touch :)

So in this case you touch this component, let's have a story for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL you can add a story for an SVG! Added!

@Zizzamia Zizzamia merged commit 3f703c2 into main Aug 21, 2024
16 checks passed
@Zizzamia Zizzamia deleted the paul/update-disconnect-icon branch August 21, 2024 04:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants