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

Documentation font changes #2139

Merged
merged 12 commits into from
Aug 20, 2024
Merged

Documentation font changes #2139

merged 12 commits into from
Aug 20, 2024

Conversation

CobusT
Copy link
Contributor

@CobusT CobusT commented Aug 19, 2024

As per design's guidelines.

  • Spline Sans Mono for the h1 headers
  • Inter for the body text
  • Font weight 500 for both

Also made a small fix to the footer (removed an old link)

@CobusT CobusT requested review from Fil and mbostock August 19, 2024 22:15
CobusT and others added 5 commits August 19, 2024 15:22
See  https://vitepress.dev/reference/site-config#example-adding-google-fonts

Note that we're using the (slightly faster?) "preload as stylesheet" strategy —as we do in Framework— over "preconnect" that Vitepress suggests.
(a.button is not used anywhere else; introduced in #1757)
Copy link
Contributor

@Fil Fil left a comment

Choose a reason for hiding this comment

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

I've changed a few things. I have just one pending question about the additional margin-bottom

.vp-doc h1 {
font-family: "Spline-Sans Mono", monospace;
font-weight: 500;
margin-bottom: 8px;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not seeing where this margin-bottom operates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am removing it.

Comment on lines 40 to 42
.vp-doc a {
color: var(--vp-c-text-1);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Makes the links same color as text (except on hover), instead of being "brand" (purple).

}
.vp-doc h1 {
font-family: "Spline-Sans Mono", monospace;
font-weight: 500;
Copy link
Contributor

Choose a reason for hiding this comment

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

These two font-weight: 500; make the the normal text a bit fatter, and the headers skinny.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was actually a bug in the font name and it was showing monospace instead of Spline Sans Mono. Fixed.

@@ -21,7 +22,22 @@
--vp-c-brand-2: var(--vp-c-purple-2);
--vp-c-brand-3: var(--vp-c-purple-3);
--vp-c-brand-soft: var(--vp-c-purple-soft);
--vp-font-family-base: -apple-system, BlinkMacSystemFont, "avenir next", avenir, helvetica, "helvetica neue", ubuntu, roboto, noto, "segoe ui", arial, sans-serif;
--vp-font-family-base: Inter, -apple-system, BlinkMacSystemFont, "avenir next", avenir, helvetica, "helvetica neue",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't intend the change the base font. Only the content font. This changes the left nav and the ToC also, which was not the intent of this PR.

Copy link
Contributor

@Fil Fil Aug 20, 2024

Choose a reason for hiding this comment

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

OK I'll revert that. At this size the fonts are almost identical, I can only differentiate them with the inspector :) [Or by switching "live" because Inter is wider]

Copy link
Member

Choose a reason for hiding this comment

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

I don’t think it looks good to have a mix of Inter for the body and San Francisco for the sidebar. We should use the same sans-serif font everywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will run it by design.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @CobusT. I asked Kyna and Alex earlier this evening, and also about consolidating Framework’s accent colors (since we’re also using a blue and a red in addition to phosphate there). I put up observablehq/framework#1591 for Framework and if there’s agreement there I expect we can propagate some of those changes to Plot. Like maybe the Plot should use phosphate for its accent color instead of purple, too…

@Fil Fil merged commit e50254d into main Aug 20, 2024
1 check passed
@Fil Fil deleted the cobus/branding-changes branch August 20, 2024 15:17
@CobusT CobusT restored the cobus/branding-changes branch August 20, 2024 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants