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 site themes #185

Merged
merged 10 commits into from
Oct 6, 2020
Merged

Add site themes #185

merged 10 commits into from
Oct 6, 2020

Conversation

apreshill
Copy link
Contributor

Hi @jjallaire,

I'm reasonably happy with these additional CSS variables, and needed to adapt some of the base styles to make them all work well with the blog listing pages. I'm unsure where to actually put these new variables, because I think users may be confused and/or overwhelmed if they create an article and see the additional website variables populate in the theme.css. Marking this PR as a draft if you can advise!

Alison

@jjallaire
Copy link
Member

I think it's fine to include the site variables if you:

a) Keep them at the bottom; and

b) Put a comment above them that say they only affect sites not standalone articles

Unrelated question: are we going to allow customization of fonts here as well?

@apreshill
Copy link
Contributor Author

Do you think there could be a lightweight way to link up to Google fonts? Some related packages:

And it looks like @rich-iannone has a google font helper function in gt:

I could see specifying a header font, body font, and code font, if you wanted to go that far. Here is an example from a fonts.toml file for a Hugo theme:

# Font style metadata
name = "sharing-short-notice"


# Optional Google font URL
google_fonts = "Noto+Serif+JP|Sen|Roboto+Mono"


# Font families
heading_font = "Noto Serif JP"
body_font = "Sen"
nav_font = "Sen"
mono_font = "Roboto Mono"

https://github.com/apreshill/share-blogdown/blob/f34d889ea7992bbaa4a222be444acdb11e5efe03/data/fonts/ssn.toml#L1-L11

@jjallaire
Copy link
Member

Could the user just import those fonts right in their theme file and then assign them to variables?

@jjallaire
Copy link
Member

If that's a bit unwieldy for any reason we could also support a "theme.fonts" file alongside the "theme.css", but I think unless it's really problematic to include the fonts inline it would be cleaner to have it in one file.

@apreshill
Copy link
Contributor Author

Thinking through this more, this is "easy" to add to the base-variables.css:

:root {
  /* fonts */
  --header-font: "Merriweather", serif;
  --mono-font: "DM Mono", monospace; 
  --body-font: "Merriweather Sans", sans-serif;
}

And then use them in the CSS by adding to the base-style.css:

/* FONT FAMILIES */


body {
  font-family: var(--body-font);
}

h1, h2, h3, h4, h5, h6 {
  font-family: var(--header-font);
}

d-article div.sourceCode code, 
d-article pre code {
	font-family: var(--mono-font);
}

The harder bit is figuring out how to get each font imported. We may have to ask users to add that to the CSS themselves, like:

<link href="https://fonts.googleapis.com/css2?family=DM+Mono&family=Merriweather+Sans:ital@0;1&family=Merriweather:ital@0;1&display=swap" rel="stylesheet">

harder b/c fonts with spaces have a + in between, and you have to deal with weights and styles. @rich-iannone said this works in gt, but we only have CSS variables to work with if we go this direction:

@import url('https://fonts.googleapis.com/css2?family={font_name}:ital,wght@0,100;0,200;0,300;0,400;0,500;0,600;0,700;0,800;0,900;1,100;1,200;1,300;1,400;1,500;1,600;1,700;1,800;1,900&display=swap');

@jjallaire
Copy link
Member

I think users can get pretty far with just generally available fonts (I believe but could be wrong that Distill itself doesn't import any Google fonts). We could just point to some documentation on what fonts can be generally relied on. Also Google has this font import tool that should build the right @import directive: https://fonts.google.com/.

@apreshill apreshill marked this pull request as ready for review October 5, 2020 19:49
@apreshill
Copy link
Contributor Author

apreshill commented Oct 5, 2020

Hi @jjallaire,

I edited the structure of the theme file to use less global CSS properties and try to locally scope some of them. Happy to return to previous structure too, as this one looks a lot longer, but may be easier to explain/document:

https://github.com/rstudio/distill/blob/182d55393cc7c5667a1c03b963b7e20f2d23cfed/inst/rmarkdown/templates/distill_article/resources/base-variables.css

@jjallaire jjallaire merged commit 056f24f into rstudio:master Oct 6, 2020
@apreshill apreshill linked an issue Oct 8, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve ability to theme distill sites
2 participants