-
Notifications
You must be signed in to change notification settings - Fork 2
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
Update: Add all CE Colors and Storybook Docs #294
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes introduce a substantial addition of CSS variables and gradients in the Changes
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (3)
Additional comments not posted (18)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
not sure if we need the gradients in there. i will add some verbiage to describe the "magic numbers" contrast-based system for accessibility |
} | ||
|
||
:root { | ||
/* Light mode questions */ |
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.
Is this comment accurate? I'm not sure what it means. Should there be a "dark mode" counterpart?
|
||
:root { | ||
/* BASE-COLORS */ | ||
--mdhui-grey-1-base: rgb(251, 252, 252); |
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'm curious why we have the base variables? What does having a "base" color variable and then a 1:1 mapping with the actual color variables do for us?
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.
It's for enabling dark/light mode when we want to do that. The non "base" color variable gets assigned with whatever the current color palette is.
src/helpers/globalCss.ts
Outdated
--mdhui-gradient-early-bird: linear-gradient(180deg, 0% rgba(34, 159, 185), 100% rgba(143, 130, 246)); | ||
--mdhui-gradient-evergreen-petrichor: linear-gradient(180deg, 0% rgba(34, 159, 185), 100% rgba(0, 123, 108)); | ||
}` | ||
|
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.
Minor: Extra newline and we lost the semicolon 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 echo the same question from Garret about having base variables.
@dylanbox Can the gradients be added to the story?
Yeah we can add the gradients to the story, I'll push that up in a sec |
Additional help text
@reneefromhold @greinard @n400 that's up |
@dylanbox Wil this be merged? |
@dylanbox - Any reason to hold off merging this? I think I could take advantage of some of these new colors once they are available. Thanks! |
Overview
Adds all of the CE colors to the MDHUI library. Colors are namespaced as
--mdhui-[color]-[shade]
to keep with current naming scheme. Additionally, adds a page on the storybook for documentation purposes.Security
REMINDER: All file contents are public.
Only adds css variables, no additional data added or endpoints.
Checklist
Testing
Documentation
Consider "Squash and merge" as needed to keep the commit history reasonable on
main
.Reviewers
Assign to the appropriate reviewer(s). Minimally, a second set of eyes is needed ensure no non-public information is published. Consider also including:
Summary by CodeRabbit
New Features
Documentation