Skip to content
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

add custom web fonts guidance #1905

Merged
merged 30 commits into from
Jan 18, 2023
Merged

add custom web fonts guidance #1905

merged 30 commits into from
Jan 18, 2023

Conversation

bonnieAcameron
Copy link
Contributor

@bonnieAcameron bonnieAcameron commented Nov 16, 2022

Updating Custom Fonts Guidance

Preview link →

Description

Based on a high number of repeat questions asking for custom (web) fonts, I've created this guidance with review from core devs.

Before you hit Submit, make sure you’ve done whichever of these applies to you:

  • Follow the 18F Front End Coding Style Guide and Accessibility Guide.
  • Run npm test and make sure the tests for the files you have changed have passed.
  • Run your code through HTML_CodeSniffer and make sure it’s error free.
  • Title your pull request using this format: [Website] - [UI component]: Brief statement describing what this pull request solves.

@bonnieAcameron bonnieAcameron self-assigned this Nov 16, 2022
@bonnieAcameron bonnieAcameron added Affects: Content Relates to content Affects: Guidance Relates to guidance labels Nov 16, 2022
@bonnieAcameron bonnieAcameron linked an issue Nov 16, 2022 that may be closed by this pull request
@bonnieAcameron
Copy link
Contributor Author

Looks like the failure is in my html

@mejiaj
Copy link
Contributor

mejiaj commented Nov 22, 2022

@bonnieAcameron I'll take a look.


For reference, if you click on the Details of first failed test (test_build) you can see where exactly the error is occurring.

image

image

Copy link
Contributor

@mejiaj mejiaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bonnieAcameron pushed up some changes. Guidance should look like this now:

image

Copy link
Contributor

@amyleadem amyleadem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bonnieAcameron I added a few comments with some questions and suggestions.

Please let me know if you have questions!

bonnieAcameron and others added 5 commits December 2, 2022 13:33
Co-authored-by: Amy Leadem <93996430+amyleadem@users.noreply.github.com>
Co-authored-by: Amy Leadem <93996430+amyleadem@users.noreply.github.com>
Co-authored-by: Amy Leadem <93996430+amyleadem@users.noreply.github.com>
Co-authored-by: Amy Leadem <93996430+amyleadem@users.noreply.github.com>
Co-authored-by: Amy Leadem <93996430+amyleadem@users.noreply.github.com>
@amyleadem
Copy link
Contributor

@bonnieAcameron @mejiaj
Created a PR with some possible edits to both the "Customizing family tokens" and "Setting custom fonts" sections: #1937

Some of the changes are just to bring some clarity to the copy, while some are dev fixes because I couldn't get the example code to work.

Feel free to take any, all, or none of the changes. Let me know if you have questions.

bonnieAcameron and others added 2 commits December 7, 2022 11:07
USWDS-Site - Font family: Custom fonts content edits
Copy link
Contributor

@amyleadem amyleadem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bonnieAcameron Looks good to me! (FYI I pushed up a small typo fix in 093d75c)

@mejiaj Can you give a sanity check on the changes made, particularly the new values for $theme-font-role-?

@@ -163,9 +165,9 @@ Role-based tokens set the font family value based on the _role_ the face plays i
**Note:** It is possible to add **custom font metadata**, **custom font stacks**, and **custom font source files** in your USWDS settings. This documentation is coming soon. See the inline documentation in `_uswds-theme-typography` for more details.
Copy link
Contributor

@amyleadem amyleadem Jan 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slightly out of scope, but this file name is out of date. We can also link to the file directly for easy reference. Also thinking this note might be better placed at the bottom of the page, below the new "Creating a custom typeface token" section. But now I might be getting out of hand, scope-wise ;)

Suggested change
**Note:** It is possible to add **custom font metadata**, **custom font stacks**, and **custom font source files** in your USWDS settings. This documentation is coming soon. See the inline documentation in `_uswds-theme-typography` for more details.
**Note:** It is possible to add **custom font metadata**, **custom font stacks**, and **custom font source files** in your USWDS settings. This documentation is coming soon. See the inline documentation in [_settings-typography.scss](https://github.com/uswds/uswds/blob/develop/packages/uswds-core/src/styles/settings/_settings-typography.scss) for more details.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related, should we create documentation for these? If not, maybe we can remove the coming soon line?

@amyleadem
Copy link
Contributor

@bonnieAcameron I updated this branch and added another couple of comments for your review. Let me know if you have questions or comments!

@mejiaj Can you give a sanity check on the changes made, particularly the new values for $theme-font-role-?

@amyleadem amyleadem requested a review from mejiaj January 10, 2023 16:29
bonnieAcameron and others added 2 commits January 10, 2023 12:04
Co-authored-by: Amy Leadem <93996430+amyleadem@users.noreply.github.com>
@amyleadem
Copy link
Contributor

Note: This guidance is only one of two options available for adding a custom font. We should add instructions for adding a custom self-hosted font. I can work on writing that and will submit it for review shortly.
@bonnieAcameron

@amyleadem
Copy link
Contributor

@bonnieAcameron @mejiaj
I reformatted the custom fonts section to include instructions for adding a self-hosted font in this PR: #1972

I also added a changelog entry (we'll need to update the merge date here when we have it).

Let me know if you have any questions or want to change up the formatting.

USWDS-Site: Add instructions for adding self-hosted font
@thisisdano thisisdano merged commit 1fb4416 into main Jan 18, 2023
@thisisdano thisisdano deleted the bc-custom-font-guidance branch January 18, 2023 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Affects: Content Relates to content Affects: Guidance Relates to guidance
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Update Custom Fonts guidance on Site
4 participants