-
Notifications
You must be signed in to change notification settings - Fork 30
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
Use card headline fonts for card byline #12873
base: main
Are you sure you want to change the base?
Conversation
Size Change: -211 B (-0.02%) Total Size: 957 kB
ℹ️ View Unchanged
|
const sublinkStyles = css` | ||
display: block; | ||
/* See: https://css-tricks.com/nested-links/ */ | ||
${getZIndex('card-nested-link')} | ||
/* The following styles turn off those provided by Link */ | ||
color: inherit; | ||
text-decoration: none; | ||
/* stylelint-disable-next-line property-disallowed-list */ | ||
font-family: inherit; | ||
font-size: inherit; | ||
line-height: inherit; | ||
|
||
const textSansSize = { | ||
xxxlarge: textSans20, | ||
xxlarge: textSans20, | ||
xlarge: textSans20, | ||
large: textSans20, | ||
medium: textSans20, | ||
small: textSans20, | ||
xsmall: textSans20, | ||
xxsmall: textSans17, | ||
xxxsmall: textSans15, | ||
tiny: textSans12, | ||
}; | ||
/* This css is used to remove any underline from the kicker but still | ||
* have it applied to the headline when the kicker is hovered */ | ||
:hover { | ||
color: inherit; | ||
text-decoration: none; | ||
.show-underline { | ||
text-decoration: underline; | ||
text-underline-offset: auto; | ||
text-underline-position: auto; | ||
} | ||
} | ||
`; |
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.
this has been moved higher up the file, it is not a new addition
return 'medium'; | ||
} | ||
}; | ||
|
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.
we no longer need to convert the sizes here as we pass font styles directly to byline
648bb70
to
3a2ae43
Compare
Hello 👋! When you're ready to run Chromatic, please apply the You will need to reapply the label each time you want to run Chromatic. |
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.
Had a look at this locally and all looks great. Nice work! ✨
7d7e102
to
777525c
Compare
What does this change?
Allows the byline to use the same font styles as the headline by passing through the font styles as a prop.
It also extends the font offering in card headline to include headline light as this is used by opinion cards. A card is considered an opinion card if it matches one of the following format criteria:
It also refactors the card headline component with the intent of tightening the types to make it easier to select the correct font family and size.
Why?
Recent redesigns determine that card bylines should inherit the card headline styles (except colour). As the byline is not always a child of the headline, we cannot simply inherit the styles so we pass them through instead.
Screenshots