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

UI Polish #1815

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

UI Polish #1815

wants to merge 9 commits into from

Conversation

mindapivessa
Copy link

@mindapivessa mindapivessa commented Feb 21, 2025

What changed? Why?

  • Wallet Pictogram Update: Updated the wallet pictogram to ensure consistency across the application.
  • Builders Container Spacing: Cleaned up the spacing within the Builders container to enhance layout aesthetics.
  • Menu Desktop Border Radius: Adjusted the border radius of the desktop menu for a more polished look.
  • Navigation Description Letter Spacing: Improved the letter spacing for navigation descriptions to enhance readability.
  • Search Modal Spacing: Refined the spacing within the search modal for better visual alignment.
  • Button Bottom Padding Fix: Fixed the bottom padding of buttons to ensure consistent spacing.
  • Footer Horizontal Line Color: Adjusted the color of the horizontal line above the footer for better visual contrast.
  • "Docs" Button Padding in Component Preview: Modified the padding of the docs button in preview mode for improved user interaction.

These changes collectively aim to improve the user interface and maintain a consistent design language throughout the application. Please review the changes and provide feedback.

Notes to reviewers

How has it been tested?

Locally - both on mobile and desktop

Copy link

vercel bot commented Feb 21, 2025

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

Name Status Preview Comments Updated (UTC)
web-base-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 21, 2025 9:29pm

Copy link

vercel bot commented Feb 21, 2025

@mindapivessa is attempting to deploy a commit to the Coinbase Team on Vercel.

A member of the Team first needs to authorize it.

@cb-heimdall
Copy link
Collaborator

cb-heimdall commented Feb 21, 2025

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 1/2
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 1
Sum 2

@mindapivessa mindapivessa changed the title [DO NOT MERGE] update wallet pictogram for consistency [DO NOT MERGE] UI Polish Feb 21, 2025
@mindapivessa mindapivessa changed the title [DO NOT MERGE] UI Polish UI Polish Feb 21, 2025
</Link>
.
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo? think this is an extraneous period

Copy link
Author

Choose a reason for hiding this comment

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

can remove

Copy link
Collaborator

Choose a reason for hiding this comment

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

let's either remove or put on line 39 so it's part of the link

Builders
</Title>
<span className="text-sm text-dark-palette-foregroundMuted ">
{/* use tracking-wide to temporary offset the odd tight letter-spacing */}
<p className="text-sm text-dark-palette-foregroundMuted tracking-wide">
Copy link
Collaborator

Choose a reason for hiding this comment

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

why p tags here instead of spans?

Copy link
Author

Choose a reason for hiding this comment

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

I was hoping to fix the tracking using it since p was used for minicard that has the correct tracking. It doesn't change anything. I can change it back to span if you think that's better.

Copy link
Collaborator

Choose a reason for hiding this comment

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

gotcha. don't have a strong feeling, was just curious. if the p tags are working for you fine to keep that way

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants