Bugfix: Fix dropdown caret position in CSS styles for clean borders#4061
Bugfix: Fix dropdown caret position in CSS styles for clean borders#4061lesliecdubs merged 6 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 0168aa2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
There was a problem hiding this comment.
Pull request overview
Fixes a visual bug in the Primer::Alpha Dropdown caret “arrow” rendering by adjusting the caret pseudo-element positioning so borders align cleanly across dropdown directions.
Changes:
- Adjusted
::aftercarettopoffset for west/east dropdown menus to correct a 1px misalignment. - Adjusted horizontal offsets (
left/right) for::aftercaret positioning in NE/SW/SE directions to remove a 1px offset.
Show a summary per file
| File | Description |
|---|---|
| app/components/primer/alpha/dropdown.pcss | Tweaks caret pseudo-element offsets for multiple dropdown directions to correct arrow border alignment. |
Copilot's findings
Comments suppressed due to low confidence (2)
app/components/primer/alpha/dropdown.pcss:296
- Same concern as the other directional carets:
left: 9pxis a hard-coded adjustment that likely representsvar(--base-size-8) + 1px. Using token-based calc (e.g.,calc(var(--base-size-8) + var(--borderWidth-thin))) makes the intended relationship clearer and reduces reliance on raw px values.
top: -14px;
/* stylelint-disable-next-line primer/spacing */
left: 9px;
}
app/components/primer/alpha/dropdown.pcss:198
- Same as the
.dropdown-menu-wcaret: switching from a token to a hard-coded11pxintroduces a magic number and needs a stylelint suppression. Prefer a token-based expression (e.g.,calc(var(--base-size-12) - var(--borderWidth-thin))) so the offset remains tied to the design system scale.
/* stylelint-disable-next-line primer/spacing */
top: 11px;
/* stylelint-disable-next-line primer/spacing */
left: -14px;
border-color: transparent;
- Files reviewed: 1/1 changed files
- Comments generated: 4
…eractive snapshots Agent-Logs-Url: https://github.com/primer/view_components/sessions/21064b25-d326-406d-afc9-e158fe24f157 Co-authored-by: lesliecdubs <3902488+lesliecdubs@users.noreply.github.com>
|
@copilot add a changeset to this PR. This is a patch. Changelog entry: "Fix Dropdown component caret position in CSS styles for clean borders" |
Agent-Logs-Url: https://github.com/primer/view_components/sessions/1518ebf1-0bff-47cb-8434-6fbc1832fe87 Co-authored-by: lesliecdubs <3902488+lesliecdubs@users.noreply.github.com>
…ath instead of magic numbers
|
@copilot autofix any of the stylelint failures that you can. Then look at the other failures and see if you can address them. |
Agent-Logs-Url: https://github.com/primer/view_components/sessions/a244c473-8a12-4c20-9380-eaf9cfdf56c2 Co-authored-by: lesliecdubs <3902488+lesliecdubs@users.noreply.github.com>
Fixed in 0168aa2. Changes made to
Verified with |
|
👋🏻 Hey @llastflowers, looks like you got pulled in as reviewer. Do you know if there's a way to get a Lookbook preview for this PR so we can manually verify these changes cross-browser? Or is there another way to test? |
llastflowers
left a comment
There was a problem hiding this comment.
Looks so much better, yay! ✨
Nevermind, we found it! But happy to have an extra set of 👀 s on this before we run with it 🙏🏻 |
|
Looks good on Safari too! |
jonrohan
left a comment
There was a problem hiding this comment.
Looks good thanks for fixing it 👍🏻
What are you trying to accomplish?
In the PRC Dropdown component, most of the borders on the arrows are broken. South works, but the rest do not. This is an attempt to manually adjust the CSS controlling arrow position to fix the bad 1px offset.
Screenshots
Look closely, we're adjusting the small 1px border on the side of the dropdown menu's arrow.
List the issues that this change affects.
Closes https://github.com/github/primer/issues/5370
Risk Assessment
What approach did you choose and why?
CSS is fun
Anything you want to highlight for special attention from reviewers?
Check the Lookbook stories for Dropdown, click each dropdown, and ensure the arrow borders are correct. Best to check cross-browser too.
Accessibility
Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.