-
Notifications
You must be signed in to change notification settings - Fork 33
fix(card): card title typography in asset and title-bar patterns #2695
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
base: main
Are you sure you want to change the base?
fix(card): card title typography in asset and title-bar patterns #2695
Conversation
🦋 Changeset detectedLatest commit: 4b540fd 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 |
✅ Deploy Preview for red-hat-design-system ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
6f6a09a to
9702aa1
Compare
…verrides Removes !important flags from #container #header/#body ::slotted heading selectors in rh-card.css. This allows asset and title-bar card patterns to override heading typography with body-text tokens as specified in their inline styles. Fixes RedHat-UX#2164
9702aa1 to
2dcc5d4
Compare
| font-weight: var(--rh-font-weight-heading-regular, 300); | ||
| font-size: var(--rh-font-size-body-text-md, 1rem); | ||
| font-family: var(--rh-font-family-body-text); | ||
| color: var(--rh-color-text-brand-on-light, #ee0000); |
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.
@marionnegp can you confirm that we'd need to use the theming token light-dark() variation here: --rh-color-text-brand.
var(--rh-color-brand-red,
light-dark(
var(--rh-color-brand-red-on-light, #ee0000),
var(--rh-color-brand-red-on-dark, #ee0000)
)
);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.
See my reply to the other comment.
| font-weight: var(--rh-font-weight-heading-regular, 300); | ||
| font-size: var(--rh-font-size-body-text-md, 1rem); | ||
| font-family: var(--rh-font-family-body-text); | ||
| color: var(--rh-color-text-brand-on-light, #ee0000); |
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.
--rh-color-text-brand likely here as well @marionnegp
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.
Yep! I think this code predates the switch to light-dark. However, we need to adjust the font weight for the brand red text to have enough color contrast at this font size. I'll make a separate comment for that.
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.
Actually, is it possible to use --rh-color-text-brand for the title text in light scheme and --rh-color-text-primary in dark scheme?
A medium font weight still isn't bold enough to satisfy the color contrast requirements, and I don't think we want these card headings to be bold or much bigger. I think switching the color to white in dark scheme is our best bet.
6a24ed4 to
4b540fd
Compare
Fixes #2164
Changes
!importantflags from#container #header/#body ::slottedheading selectors inrh-card.cssProblem
Card titles in asset and title-bar patterns were not matching Figma specs because heading styles from the constructed stylesheet were using
!important, which prevented pattern-level CSS from applying body-text tokens.Solution
Removed
!importantto allow patterns to override typography with their inline styles using body-text tokens as designed.Testing
Pattern demos already have correct inline styles with body-text tokens - they were just being blocked by
!important.