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

Try to left align text of navbar links #508

Merged
merged 4 commits into from
Nov 15, 2023
Merged

Conversation

rly
Copy link
Collaborator

@rly rly commented Nov 12, 2023

The text on the left nav bar is not left-aligned. This PR adjusts the margins of the SVGs to correct for that. I assume there is a reason the icons are different sizes. I think it does look better at these sizes, so I did not touch that.

Before:
Screenshot 2023-11-12 at 12 56 37 AM

After:
Screenshot 2023-11-12 at 12 53 25 AM

It's not pixel perfect if you zoom in - I'm not sure why - but it does look better to me now.

@rly
Copy link
Collaborator Author

rly commented Nov 12, 2023

PS This felt too small of an issue to create an issue ticket for, but let me know if that would be preferred.

@garrettmflynn garrettmflynn self-requested a review November 12, 2023 12:58
@garrettmflynn
Copy link
Member

I also tried a CSS-based solution—but this is the best approach given the inconsistent widths of the icons themselves.

When setting things to a uniform 25px outside of this PR, this is what it would look like:
Screenshot 2023-11-12 at 8 00 30 AM

Not great balance between the original SODA icons and the new ones for the GUIDE. Could change them for new Material Icons, but that's a broader discussion.

Copy link
Member

@garrettmflynn garrettmflynn left a comment

Choose a reason for hiding this comment

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

This is great. Thank you @rly!

@CodyCBakerPhD CodyCBakerPhD merged commit 8ebc77b into main Nov 15, 2023
11 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the adjust_nav_alignment branch November 15, 2023 15:44
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