Use tidytemplate styles instead of quillt template for pkgdown

hadley commented 3 years ago

This is a work in progress — I'm hoping that we can figure out a few tweaks to tidytemplate to match the primary visual appearance of quillt, while reducing the amount of custom css. The end goal is to come up with a compromise style so that all tidyverse, r-lib, tidymodels, and rmarkdown sites look like they are part of the family family.

No need to look at this yet, and it currently probably only works on my machine. I'll touch base when I'm a bit further along to figure out if this is going to be feasible or not.

hadley commented 3 years ago

To try it out

devtools::install_github(c(
  "r-lib/pkgdown", 
  "tidyverse/tidytemplate", 
  "rstudio/quillt#42"
))

Then set this in _pkgdown.yml:

template:
  package: tidytemplate
  bootstrap: 5
  bslib:
    primary: "#096B72"
    navbar-background: "#e6f3fc"

cderv commented 3 years ago

Do you plan to compare all the tweak that was made in the CSS by Alison ? I can look into too - I am not sure what she tweaked explicitly.

Otherwise, we have modified the template for

trailing redirect, now supported
Modified footer: we can add that through config now right ?
navbar reorganization - but we can maybe ignore now.

That is what I remember.

hadley commented 3 years ago

I didn't inspect every component of the CSS, but I did look at the overall visual appearance of the site and attempt to bring it all into alignment. I do know that a lot of the tweaks are no longer needed because they fixed problems that no longer exist in tidytemplate.

cderv commented 3 years ago

I do know that a lot of the tweaks are no longer needed because they fixed problems that no longer exist in tidytemplate.

That confirms what I thought. Thank you. the Bootstrap theme looks good to me.

hadley commented 3 years ago

Fonts that were specific: unless you feel really strongly about this, I'd really prefer to keep the font the same across all sites (to match rstudio.com)
border on hover for example page thumbnails: I'm happy to add that to tidytemplate
no background on inline code in headers: I'm happy to add that to pkgdown
h3 uppercase: I'd prefer to not to use this as it general makes text harder to read (because there's less variation in letter forms)
.snippet CSS for flexdashboard ...: I'm happy to take any extra stuff into tidytemplate

cderv commented 3 years ago

Let's use the same fonts.
border on hover is specific for the example page using the thumbnails structure for which quillt has helpers to create. I can definitely add this only in the article page using css there. But if you want to add globally, ok.
let's put the css for background inline code in pkgdown then
Ok to remove h3 uppercase
I need to check this snippet css usage. Flexdashboard pkgdown website was a rewrite of previous website and specific content (ace like snoppet) was kept. https://pkgs.rstudio.com/flexdashboard/articles/using.html quillt also contains the JS lib for that that was moved from previous site. https://github.com/rstudio/quillt/tree/main/inst/pkgdown/assets

I will really need to test each website to be sure all specific thing are ok. But flexdashboard was the main one because of previous website.

hadley commented 3 years ago

I think the thumbnails structure is very nice, and it would be great to be able to use it in more places than quillt
You can also have pkgdown/extra.css just for specific sites too. That might be easier for flexdashboard.

gadenbuie commented 3 years ago

You can also have pkgdown/extra.css just for specific sites too. That might be easier for flexdashboard.

Is there a similar mechanism for including extra JavaScript libraries at the site level?

Both flexdashboard and learnr (not on a pkgdown site yet, but we want to be) use Ace editors for displaying snippets of code. Personally, since those snippets are static, I'm okay with changing the display to static <pre> elements for that code. But flexdashboard also includes other JS dependencies, e.g. for building the placeholder elements .

hadley commented 3 years ago

@gadenbuie yes, it's pkgdown/extra.js 😉

More details at https://pkgdown.r-lib.org/dev/articles/customise.html#additional-html-and-files

cderv commented 2 years ago

@gadenbuie for flexdashboard and learnr, it would mean that the JS and CSS for the Ace snippet would be moved in local extra.css and extra.js files ?

Currently, they are included into quillt template https://github.com/rstudio/quillt/blob/e9f7e49a28fc880e35ec4dca81ff358b01e78a27/inst/pkgdown/templates/head.html#L95-L98

@hadley would it be something to include in tidytemplate otherwise ?

hadley commented 2 years ago

@cderv hmmmm, I think it's better to keep the site specific stuff out of the overhaul template.

cderv commented 2 years ago

Having them in quillt was a way to share them between sites. But it is only 2 and I don't think other will use. So would have those components in learn and flexdashboard directly. @gadenbuie does it seems ok ?

gadenbuie commented 2 years ago

@cderv Yeah, I think it'd be fine for these components to live only in learnr and flexdashboard

gadenbuie commented 2 years ago

I think the thumbnails structure is very nice, and it would be great to be able to use it in more places than quillt

@hadley are you working moving the thumbnails to tidytemplate? I'd be happy to help move the feature there if you haven't started on it yet.

hadley commented 2 years ago

I think the thumbnails should probably go into pkgdown itself. Any help doing that would be much appreciated 😄

gadenbuie commented 2 years ago

@hadley sounds great, I'll work on submitting a PR there. I should have something together either today or by the end of the week.

hadley commented 2 years ago

I've added brief notes at https://github.com/tidyverse/tidytemplate#rmarkdown--quillt; let me know if you think there's anything else that needs to be done here.

cderv commented 2 years ago

I think everything is there. I just need to take the time to deal with the element not provided by tidytemplate

Example gallery which is currently made from a quillt package function
JS libraries - available through pkgdown quillt template currently

First try was learnr website which is now live. It handled the 2 above points specifically. I would like to make them more generic for others website. But overall, website looks good !

I think the plan will be for quillt to still exists as a package containing helpers for setup and storing common functions accross those website but no more as a pkgdown template package.

I'll work on that change while updating our website based on this PR. Thanks Hadley for starting this experiment ! Everything is on me now.

gadenbuie commented 2 years ago

One thing that I just ran into is that, for package docs that aren't hosted at the root folder (everything on GitHub Pages), you need to use the full URL to resources that are included via includes: in_header or other places. The side effect is that this breaks local preview.

This came up for me in learnr with the Ace and snippets assets. I originally included the following in the _pkgdown.yml:

url: https://pkgs.rstudio.com/learnr/
template:
  includes:
    in_header: >
      <script src="/snippets/ace/ace.js" type="text/javascript" charset="utf-8"></script>
      <script src="/snippets/snippets.js" type="text/javascript" charset="utf-8"></script>

This worked locally but didn't work once I deployed to GitHub pages since learnr is at rstudio.github.io/learnr.

The fix I applied (for now) is to fully specify the complete and final URL to those assets

url: https://pkgs.rstudio.com/learnr/
template:
  includes:
    in_header: >
      <script src="https://pkgs.rstudio.com/learnr/snippets/ace/ace.js" type="text/javascript" charset="utf-8"></script>
      <script src="https://pkgs.rstudio.com/learnr/snippets/snippets.js" type="text/javascript" charset="utf-8"></script>

but as you can imagine, this makes local preview (or deploy previews, too, for that matter) hard. I'm going to open an issue upstream in pkgdown suggesting that we allow {{#site}}{{root}}{{/site}} (or other templating variables?) in the includes HTML, with the goal of allowing the following

url: https://pkgs.rstudio.com/learnr/
template:
  includes:
    in_header: >
      <script src="{{#site}}{{root}}{{/site}}snippets/ace/ace.js" type="text/javascript" charset="utf-8"></script>
      <script src="{{#site}}{{root}}{{/site}}snippets/snippets.js" type="text/javascript" charset="utf-8"></script>

cderv commented 2 years ago

@gadenbuie I included your solution as a in-header.html template that we copy in pkgdown/templates when doing the setup so that we override the one in tidytemplate which hopefully is not provided. If it does we would need to change it.

Other solution would be to include this into tidytemplate directly conditionally with a variable for website that requires it; But it would add some external assets into tidytemplate.

For now, this will work fine.

rstudio / quillt

Use tidytemplate styles instead of quillt template for pkgdown #42