docs(card): customizing theme colors #1200
Conversation
|
✅ Deploy Preview for red-hat-design-system ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Size Change: -2 B (0%) Total Size: 188 kB
ℹ️ View Unchanged
|
|
Before we merge this, it's worth considering weather it would be wiser to advise users to change the surface color tokens for a given card or class of cards, rather than providing an overriding background variable. My reasoning is as follows: if we expose the background color variable to users, then users who wish to implement for example a dark background will skip the use of our color context system all together. That is likely to lead to maintenance difficulties, feature creep, and design and accessibility violations. On the other hand, if we advise users who wish to apply a custom background to explicitly change the surface token values for a given card or class of cards, then they will have to explicitly participate in the context system. Because the context surface tokens have the words "on light" and "on dark" in their names, the user will be subtly encouraged to provide values for both theme variants. This would have the added advantage of inheriting the provided values to the card's context-consuming children e.g. accordions, which in most cases would be desirable, and could be explicitly opted out of with a child selector |
That's a good point, I was debating whether or not updating the token for the cards was the way to go or if giving them a background variable would be "easier" vs more correct. If we did go with the token route we'd just want to document it as a pattern or in the element docs letting the user know how they could go about updating it for light, dark, or both contexts. |
Sounds like the example in the patterns section where this issue originated is incorrect instead. If we do not want people overriding the background due to color context. https://github.com/RedHat-UX/red-hat-design-system/pull/1188/files#diff-74187edef1126c20210dc56031164bf56de11f133eacd202a89143cca266baf1R133 If i understand correctly @bennypowers you are asking for something like this instead which would be strictly pattern. rh-card.alt {
--rh-color-surface-lightest: rebeccapurple;
--rh-color-surface-lighter: #bada55;
--rh-color-surface-light: cornsilk;
--rh-color-surface-darkest: indigo;
--rh-color-surface-darker: aliceblue;
--rh-color-surface-dark: #FF91AF;
}In this example as long as the |
|
@brianferry @zeroedin I agree, we should update the docs. WDYT if we pivot this PR by reverting the css changes and then updating the docs? |
|
Makes sense to me, I'll take a look at updating these docs |
|
@marionnegp @coreyvickery would either of you be able to review the content under "Custom Theming" https://deploy-preview-1200--red-hat-design-system.netlify.app/patterns/card/#custom-theming . I added two examples in there of customizing the background, border color, and the font color. |
|
I added a |
|
@coreyvickery, should people be able to add a headline to the header, like in the slotted title card pattern? I think it makes the spacing look off, but I'm not sure if other teams have asked for it. |
This should probably be discussed in another issue, yes? @brianferry What do you think? |
yes, please open a new issue for this |
bennypowers
changed the title
Co-authored-by: Benny Powers <[email protected]>
Co-authored-by: Benny Powers <[email protected]>
|
leaving this open for @marionnegp to approve before merging |
|
@marionnegp - Created a bug for the issue you raised #1209 |
Issue
#1198
What I did
--rh-card-background-coloras a css variable to rh-card for docs(card): explain element vs pattern #1188