Skip to content

[Bug] up/down chevron alignment - #894

Merged
langermank merged 29 commits into
mainfrom
chevron-vertical-alignment-bug
Jan 20, 2023
Merged

langermank merged 29 commits into
mainfrom
chevron-vertical-alignment-bug

Conversation

@langermank

Copy link
Copy Markdown
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
@langermank
langermank requested a review from tallys January 11, 2023 20:33
@changeset-bot

changeset-bot Bot commented Jan 11, 2023 •

Copy link
Copy Markdown

🦋 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

gavinmn commented Jan 13, 2023

Copy link
Copy Markdown
Contributor

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

@tallys tallys left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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

This branch was successfully deployed

1 active deployment
github-pages — fadb3761 Deployed Jan 20, 2023 by langermank
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