-
Notifications
You must be signed in to change notification settings - Fork 52
Use bgColor-default
for overlay
background in dark themes
#1220
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
Conversation
🦋 Changeset detectedLatest commit: 3551710 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 |
Design Token Diff (CSS)
|
Design Token Diff (StyleLint)
|
Design Token Diff (Figma)
|
bgColor-default
for overlay
background in dark themes
Can we make the BG darker so that we still see layers? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the overlay background in dark themes by removing the muted shade override so that the default background is used, and bumps the primitives package version.
- Remove the
dark
override ofbgColor.muted
inoverlay.json5
- Rely on the default background in dark mode
- Add a changeset to bump
@primer/primitives
to a minor version
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/tokens/component/overlay.json5 | Removed the dark: '{bgColor.muted}' override |
.changeset/ten-adults-matter.md | Added a changeset to bump version and describe change |
Comments suppressed due to low confidence (2)
src/tokens/component/overlay.json5:12
- Removing the explicit
dark
override might lead to ambiguity; consider adding an explicitdark: '{bgColor.default}'
entry to make the intended default background clear and guard against future changes.
dark: '{bgColor.muted}',
src/tokens/component/overlay.json5:12
- We should add or update a visual regression or snapshot test for the overlay component in dark mode to ensure the background change behaves as expected.
dark: '{bgColor.muted}',
Use
bgColor-default
for dark overlays. I'm really not sure how I feel about this solution as we rely on a slightly lighter shade to show dimension for dark mode where shadows aren't very useful. But we're lacking the ability to layer inside overlays by usingmuted
as the background.Closes https://github.com/github/primer/issues/4175
Before
After