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

[Bug] up/down chevron alignment #894

Merged
merged 29 commits into from
Jan 20, 2023
Merged

Conversation

langermank
Copy link
Contributor

Closes https://github.com/github/primer/issues/1673

This PR adjusts where the SVG sits within the frame to improve horizontal alignment between these related icons.

Before

image

After

image

Concerns

  • Will this break how the icons behave individually? This might be considered an edge case where they appear side by side
  • Should we take another approach to handle this at the component level?

@langermank langermank requested a review from a team as a code owner January 11, 2023 20:33
@changeset-bot
Copy link

changeset-bot bot commented Jan 11, 2023

🦋 Changeset detected

Latest commit: fadb376

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@primer/octicons Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@langermank langermank temporarily deployed to github-pages January 11, 2023 20:36 — with GitHub Actions Inactive
@gavinmn
Copy link
Contributor

gavinmn commented Jan 13, 2023

Going to add this to the Octicons agenda for next Wednesday.

Copy link
Contributor

@tallys tallys left a comment

Choose a reason for hiding this comment

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

Haven't heard any concerns about aligning the icon. If we can bundle this with addressing any visual regressions in buttons (if that was the original reason for the misalignment) let's do it.

@langermank langermank temporarily deployed to github-pages January 19, 2023 22:15 — with GitHub Actions Inactive
@langermank langermank force-pushed the chevron-vertical-alignment-bug branch from 80eac50 to 9fee24d Compare January 19, 2023 22:44
@langermank langermank force-pushed the chevron-vertical-alignment-bug branch from 9fee24d to 8aee6b6 Compare January 19, 2023 22:44
@github-actions github-actions bot force-pushed the chevron-vertical-alignment-bug branch from b4bc3f4 to c890516 Compare January 19, 2023 22:49
@github-actions github-actions bot force-pushed the chevron-vertical-alignment-bug branch 3 times, most recently from 84452a3 to 984e506 Compare January 19, 2023 22:50
@langermank langermank temporarily deployed to github-pages January 19, 2023 23:21 — with GitHub Actions Inactive
@langermank langermank temporarily deployed to github-pages January 20, 2023 00:04 — with GitHub Actions Inactive
@langermank
Copy link
Contributor Author

@langermank langermank merged commit 410831b into main Jan 20, 2023
@langermank langermank deleted the chevron-vertical-alignment-bug branch January 20, 2023 19:23
@primer-css primer-css mentioned this pull request Jan 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants