-
Notifications
You must be signed in to change notification settings - Fork 155
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
feat: Update colors for warnings in status-related components #1340
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1340 +/- ##
=======================================
Coverage 93.47% 93.47%
=======================================
Files 623 623
Lines 16770 16772 +2
Branches 5551 5551
=======================================
+ Hits 15675 15677 +2
Misses 1022 1022
Partials 73 73
☔ View full report in Codecov by Sentry. |
@@ -548,6 +553,11 @@ const metadata: StyleDictionary.MetadataIndex = { | |||
public: true, | |||
themeable: true, | |||
}, | |||
colorTextNotificationYellow: { |
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.
Why do we need to expose this token? It is already themeable by theming the colorTextNotificationDefault
in the newly introduced flashvar-warning
context
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 you're right, removed the token from the metadata.
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.
The changes are looking good to me.
When checking the visual differences, I saw the Code Editor does not have a permutation page. Do you know if this is by purpose? If not, I think this PR is a good opportunity for adding the permutation page.
I don't know if the Code editor not having a permutations page is by purpose. I didn't add coverage for Code editor because I noticed that the warning icon is covered by this existing page, even though it could be a good idea to change its name and title (because they suggest that the code editor doesn't show any warning, though it does). It's true that having a permutations page for Code editor would help catch potential regressions at the PR stage. But I think I'd rather do that separately. So in short I'd do the following:
|
Description
(Updated replica of #1231)
A visual update to components that use "warning" status: status indicator, alert, flashbar, and code editor.
Currently, warning status doesn't have a dedicated color and instead either uses warning or info colors. This change now introduces a new set of yellow tones for warning specifically.
Design POC is Adam.
Related links, issue #, if available: AWSUI-8420,
Zb5hAZRq0yk8/Warning-Colour-Update
How has this been tested?
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md
.CONTRIBUTING.md
.Security
checkSafeUrl
function.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.