-
Notifications
You must be signed in to change notification settings - Fork 1k
USWDS - Card: Enable $theme-card-font-family setting #5974
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
Conversation

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.
Looks good to me! Thanks for fixing this @cathybaptista.
thought: The only hesitation I have here is whether we should expect the button styles to change as well, since the footer and button are included in usa-card__container
and do not have an explicit font family setting like header does. I ultimately decided that it would be too opinionated to force that, and users can easily access the container font for button by adding a font-family: inherit
style rule for usa-button
if desired.
I was able to:
- Confirm that there were no visual regressions with the default component
- Confirm that I could update the value for
$theme-card-font-family
and see the font update in the body
Note
I added the related issue (#5824) to the 3.9.0 milestone since this seems like a nice quick win to include.

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.
Working like a charm! Great simple fix here. Thank you for including testing instructions as it made this PR easy to review and see the changes take place.
Since we're likely to include this in 3.9.0
it would be great to have a changelog item for this on site! I'll reach out directly about walking you through this process 👍
Changelog entry is pending, but that doesn't have to block the feature PR review. @cathybaptista please create a changelog entry on the website and add the link to your PR description. |
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.
Love a nice focussed change like this one
Summary
Fixed a bug that caused the component to ignore the
$theme-card-font-family
setting.Breaking change
This is potentially a breaking change.
Prior to this PR, the card component disregarded the value set in the
$theme-card-font-family
setting. Because the component will now respect the value of that setting, your implementation of the card component might display with an unexpected font family. If it does, update the value of$theme-card-font-family
in your USWDS settings configuration with your expected font family.Related issue
Closes #5824
Related pull requests
Changelog uswds/uswds-site#2805
Preview link
Card component preview
This preview won't show any changes until the style itself changes, refer to the screen captures below in the "solution" section to see what happens when this change is included.
Problem statement
$theme-card-font-family is a value that is set in _settings-components.scss. However, the typeset mixin that is used by the style doesn't currently have this value passed to it, so if the style were to change it would not display.
Solution
Added the value of $theme-card-font-family to the @typeset mixin so that whatever value is set in settings is honored in the component. Now, when the $theme-card-font-family is set the component will show the appropriate font-family.
change
@include typeset;
to@include typeset($theme-card-font-family);
so that the value of the style is passed to the mixin.both of these screen captures use
$theme-card-font-family: "mono" !default;
before:


after:
This approach was chosen because if this style changes it should be available in the component.
Testing and review
To reproduce:
git pull origin [base branch]
to pull in the most recent updates from your base and check for merge conflicts. (Often, the base branch isdevelop
).npm run prettier:sass
to format any Sass updates.npm test
and confirm that all tests pass.-->