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

Long-term support for themes #232

Open
pat-s opened this issue Oct 20, 2019 · 18 comments
Open

Long-term support for themes #232

pat-s opened this issue Oct 20, 2019 · 18 comments
Labels

Comments

@pat-s
Copy link
Collaborator

pat-s commented Oct 20, 2019

Due to #229 I was wondering if there would be the need for a different approach of updating/contributing themes.

Rather than adding individual CSS files for a theme to xaringan directly, xaringan could import themes from an upstream repo via git submodules.

As submodules are somewhat complicated, one could come up with a wrapper function named install_theme(update = TRUE/FALSE) (use_theme()) which takes care of this in the background.

In general modular approaches have a lot of advantages seem from a maintenance point of view. However, it is hard to change the system if one did not start this way.

There haven't been many new theme contributions lately and most of the time themes won't be updated. So maybe I am trying to use sledgehammer to crack a nut here...

Advantages

  • No need to review theme contributions/updates anymore as these would happen in a separate repo
  • No need to duplicate changes (PR to xaringan and the upstream repo) if there is a dedicated upstream repo
  • less files shipped with the package when being installed (CSS files are very small but things might become messy at some point (total number of files))

Disadvantages

  • possible need to write a convenience functions for user to simplify git-submodule stuff
  • existing themes would need to be ported to an external repo if none exists yet
@pat-s pat-s added the question label Oct 20, 2019
@tcgriffith
Copy link
Collaborator

tcgriffith commented Oct 20, 2019

Love it.

I like the way hugo themes are organized. I think we could borrow some ideas from them. Like having a master repo for showcase and organize themes in different repo.
https://github.com/gohugoio/hugoThemes

But I think the submodule stuff is likely to be an overkill. Just download the theme file should be enough. To make my point clear, my case is that all my slides sit under "projectA/slides/". They are individual Rmd files, so adding a submodule to the project repo feels strange to me.

To keep things simple, I also propose

  • One CSS file for one theme (instead of separating fonts/ other styles into two files, Yihui suggested to do this so people can use other "fonts", but I don't think people will do this in practice.)

  • do not include materials (like logos) which might pose copyright issue.

@pat-s
Copy link
Collaborator Author

pat-s commented Oct 22, 2019

One CSS file for one theme

I'd go with that.


But I think the submodule stuff is likely to be an overkill.

Yes probably. What about a install_theme() function which just scrapes the css file from raw.github (because it knows the upstream url of the theme), then puts it into assets/css of the current working dir and also links it in the css: field of the YAML header?

This function could run in the setup chunk of the Rmd file and external people looking at the source Rmd would directly see that there is this call for installing an external theme.

@tcgriffith
Copy link
Collaborator

I like it and it's straightforward to implement. It should be flexible to deal with existing themes and new themes hosted individually

@tcgriffith
Copy link
Collaborator

tcgriffith commented Oct 23, 2019

I suggest using use_theme() as a more proper function name. Since it's pure CSS, we could embed the code directly instead of modifying the css field (I imagine this might be a bit complicated because you need to correctly interact with the rstudio editor api?)

A user-friendly way to use a theme should be something like dropping xaringan::use_theme("aaa/bbb") in the setup chunk and that's all.

@tcgriffith
Copy link
Collaborator

tcgriffith commented Oct 24, 2019

Just found a convenient way to include CSS into the RMD using htmltools, which is an existing dependency of xaringan.

Add the following chunk into the Rmd, There's no need to modify the yaml header or to download the CSS file and it will always up to date with the online CSS version.

htmltools::includeCSS("https://cosx.org/css/style.css")

Then the only work left to do is to organize a repo for hosting themes and maybe write a wrapper function in xaringan.

@pat-s
Copy link
Collaborator Author

pat-s commented Oct 24, 2019

Then the only work left to do is to organize a repo for hosting themes and maybe write a wrapper function in xaringan.

You mean in detail that use_theme() would essentially paste the correct theme url from the upstream repo into the setup chunk?
This way users do not have to look up the upstream repo url.
And if one wants to add a theme, he only needs to add a key:value pair of theme:url to use_theme().

Additionally we should state that xaringan is not responsible for glitches in the themes.

@mschilli87
Copy link
Contributor

@tcgriffith: I'm not sure if (always) directly using the (latest) online version is the right thing to do.
Not so much because it makes offline presenting even harder. If you manage to make everything else self-contained you won't have an issue downloadign and including the CSS.
I'm more concerned about digging out an old presentation to find that something has changed about the CSS and now things don't align anymore. Maybe a parameter to choose between "latest" (i.e. master) or "current" (i.e. commit pointed at by master at the time of running use_theme()) could solve this issue.

@tcgriffith
Copy link
Collaborator

@pat-s yes, exactly.

Additionally we should state that xaringan is not responsible for glitches in the themes.

Separating the theme stuff(contributing/updating themes) from the xaringan package. I think that's your initial purpose right?

@mschilli87
I'm with you on making the slide self-contained and reproducible. Actually, using the current built-in themes should have this reproducibility problem (such as metropolis theme updated by @pat-s in #229 ). But how do we know the "current" version of the theme in the xaringan source Rmd?

@tcgriffith
Copy link
Collaborator

tcgriffith commented Oct 25, 2019

I quickly came up with this demo repo to demonstrate how I would like a theme to be

The css add three classes to change the background color:

.bg-red{
  background-color: red;
}

.bg-bisque{
  background-color: bisque;
}

.bg-blue{
  background-color: blue;
}

the theme is xaringan_theme_example.css, which uses the same name as the repo.
In the same repo,

The use_theme() function:

```{r echo=TRUE}
use_theme_github = function(theme = "tcgriffith/xaringan_theme_example", ref =
                              "master") {
  bn = basename(theme)
  
  css_remote = paste0("https://raw.githubusercontent.com/",
                      theme,
                      "/",
                      ref,
                      "/",
                      bn,
                      ".css")
  
  css_name = basename(css_remote)
  xfun::download_file(css_remote)
  
  htmltools::includeCSS(css_name)
}

use_theme_github("tcgriffith/xaringan_theme_example", "83908f")
```

The ref parameter should be sufficient to make the slide reproducible. @mschilli87

@mschilli87
Copy link
Contributor

@tcgriffith: If I read this correctly, the CSS is downloaded and the local copy is included in the HTML. In that case, as long as I don't re-render my RMarkdown file, I'd get the same result even with ref = "master". I understood the earlier proposal to directly include the online version of the CSS by URL (i.e. htmltools::includeCSS(css_remote)). In such a sceanrio I'd like some code that resolves master to the corresponding commit (at render time) such that if I open an old HTML it looks the same as it used to. I'm not sure if this would require more dependencies and would be worth the effort though.

@tcgriffith
Copy link
Collaborator

tcgriffith commented Oct 25, 2019

@mschilli87

It's possible

I think there are several ways to approach this problem.

  • if local theme.CSS exists, use it and don't download. It requires users to share the Rmd together with the CSS to make the slide reproducible.

  • explicitly record the theme's git hash or md5sum in the yaml header(or somewhere else) and use that information when calling use_theme()

=== update:

I embedded the info in the author's note in the presentation mode

source

@tcgriffith
Copy link
Collaborator

tcgriffith commented Oct 25, 2019

Maybe this is a little off-topic but why not…

I deployed a blogdown site as a theme showcase (hugo theme: xmag by @yihui )
each theme is added as a submodule under content/themes/

xaringan-themes
repo

The showcase is not compulsory, people can host their own themes at their own will.

The structure of a theme is pretty flexible: only that CSS is required, I also added an index.md, which works in both hugo+netlify and jekyll+gh pages

@pat-s
Copy link
Collaborator Author

pat-s commented Oct 25, 2019

That escalated quickly 😄

I like the current approach @tcgriffith. Havin a "ref" arg gives control for versioning.

each theme is added as a submodule under content/themes/

And we're back at submodules 😃 How do you want to keep track of upstream changes? Automate the submodule update process somehow?

@tcgriffith
Copy link
Collaborator

tcgriffith commented Oct 25, 2019

The showcase repo is only intended for the blogdown site, not for actually using them so submodule is totally fine in this case. So only a handful of showcase maintainers and the theme author are responsible to keep the showcase up to date.
AND THEMES INCOMPATIBLE WITH THE LATEST XARINGAN WILL BE PURGED INTO OBLIVION!

@tcgriffith
Copy link
Collaborator

So shall we propose a PR or gather more discussions for a few days ?

@pat-s
Copy link
Collaborator Author

pat-s commented Oct 26, 2019

Your approach looks fine for me. More comments when the PR is ready then.

@yihui's opinion would be important before you get started to not waste resources and time.

@tcgriffith
Copy link
Collaborator

Update:
The 18 built-in themes

and the Hugo theme is also changed.

@tcgriffith
Copy link
Collaborator

PRed, please have a look! @pat-s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants