-
Notifications
You must be signed in to change notification settings - Fork 10
fix(components): [DSYS-139] improve dark mode Alert warning colors #1910
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@launchpad-ui/components': patch | ||
| --- | ||
|
|
||
| Adjust dark mode Alert warning variant colors to read more yellow/gold and distinguish from error. Use theme-aware `--alert-color-icon-warning` token for the warning icon fill. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| --alert-color-bg-success: var(--lp-color-green-0); | ||
| --alert-color-border-warning: #fad88f; | ||
| --alert-color-bg-warning: #fdfae4; | ||
| --alert-color-icon-warning: var(--lp-color-brand-orange-base); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Primitive token used instead of semantic alias for iconLow Severity
Additional Locations (1)Triggered by project rule: Bugbot Instructions — launchpad-ui Reviewed by Cursor Bugbot for commit 5d5a8cb. Configure here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just a local fix for the Alert component. Will follow up to add a global |
||
| } | ||
| [data-theme='dark'] { | ||
| --alert-color-border-neutral: var(--lp-color-gray-800); | ||
|
|
@@ -21,8 +22,9 @@ | |
| --alert-color-bg-info: #192142; | ||
| --alert-color-border-success: var(--lp-color-green-700); | ||
| --alert-color-bg-success: #14260d; | ||
| --alert-color-border-warning: #932c00; | ||
| --alert-color-bg-warning: #3c170c; | ||
| --alert-color-border-warning: #946020; | ||
| --alert-color-bg-warning: #34260e; | ||
| --alert-color-icon-warning: var(--lp-color-brand-orange-base); | ||
| } | ||
|
|
||
| /* Shared styles for both the block and inline variants */ | ||
|
|
@@ -46,7 +48,7 @@ | |
| } | ||
|
|
||
| &.warning .icon { | ||
| fill: #e65d00; | ||
| fill: var(--alert-color-icon-warning); | ||
| } | ||
| } | ||
|
|
||
|
|
||


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.
Light mode warning icon color unintentionally changed
Medium Severity
The warning icon fill changed from
#e65d00(dark burnt orange,rgb(230, 93, 0)) tovar(--lp-color-brand-orange-base)(rgb(255, 157, 41), a much lighter golden orange) in both light and dark mode. The PR intends to fix only dark mode colors, and the test plan item "Confirm light mode warning appearance is unchanged" is explicitly unchecked. The light-mode--alert-color-icon-warningdefinition could use the original#e65d00(or an equivalent token) to preserve light-mode appearance, while only the dark-mode block uses the new value.Additional Locations (1)
packages/components/src/styles/Alert.module.css#L50-L51Reviewed by Cursor Bugbot for commit 5d5a8cb. Configure here.
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.
I think this is fine, it looks better: