xaringan icon indicating copy to clipboard operation
xaringan copied to clipboard

Add shadow_clone() to use themes

Open tcgriffith opened this issue 4 years ago • 8 comments

To contribute to this repo, please make sure to:

  • [x] Sign the CLA http://www.clahub.com/agreements/yihui/xaringan (if you haven't signed it before).
  • [x] Add a news item to NEWS.md and your name to DESCRIPTION (by alphabetical order) unless you have already been listed there.
  • [x] Provide a URL to a live example (and even better, a couple of screenshots) if you are contributing a new theme.

Thank you!


The proposed function shadow_clone() is a product from discussions in #232

In short, we propose to separate the xaringan themes from the main repo, so that users can host their own xaringan themes in a separate repo.

Summary:

  • The shadow_clone() function works with existing built-in themes and remotely-hosted themes
  • For remote themes, git hash is resolved with a message addressing how to reproduce the exact theme.

Live example xaringan source

tcgriffith avatar Oct 31 '19 01:10 tcgriffith

Hi, @pat-s, your proposed alternative approach looks interesting and sound. After reading your review, I have more thoughts on this.

Downloads the CSS from the requested theme and stores it locally under assets/CSS adds the new CSS file to the YAML header with the correct name Adds a comment into the "setup" chunk stating which revision of the theme is used

I'm sold on your approach. Downloading the CSS and modify the yaml seems to be more consistent with the way xaringan handles other assets. My current approach embeds the theme CSS in the html. So if the user wants to make small adjustments to the theme, it will be hard.

have a single function called use_xaringan_theme() which is called once in the console

I think this is a bit inconvenient because it breaks the 1-click "knit" tradition in all types of rmarkdown documents. Instead, we can include the use_xaringan_theme() in the setup chunk and follow the 1-click knit tradition.

I would remove all built-in themes from xaringan and just have a local dictionary stored in the package which tracks the upstream URLs of themes

I disagree. xaringan can ship with some ready-to-use themes which will always work. There are many factors that may affect the individually hosted themes that make them unavailable (changing the GitHub account name/deleting, renaming the repo, etc.) Thus, I tend to keep the existing themes as-is.

then there is your showcase site with all themes and this which shows how to define a function use_theme_github() and then apply it And there is the showcase site which also contains some code

The showcase website can be adjusted accordingly after the use_theme functionality is settled down I think it provides a better way to introduce themes to the users but for now, let's just forget about that.

tcgriffith avatar Oct 31 '19 15:10 tcgriffith

I'm sold on your approach. Downloading the CSS and modify the yaml seems to be more consistent with the way xaringan handles other assets. My current approach embeds the theme CSS in the html. So if the user wants to make small adjustments to the theme, it will be hard.

I'm with you there. Also I do not like having a huge CSS chunk at the bottom of the presentation. Theme adjustments could to into a separate CSS (e.g. extra.css) also stored in assets/css. Then it is very clear what the theme does and what user modifications have been added.

I think this is a bit inconvenient because it breaks the 1-click "knit" tradition in all types of rmarkdown documents. Instead, we can include the use_xaringan_theme() in the setup chunk and follow the 1-click knit tradition.

I disagree with you on this. I see no difference between calling use_xaringan_theme() once in the console (with all the side effects above) and manually storing the call in the setup chunk. In contrast, I would not want to have use_xaringan_theme() running on every "knit" of the file. Imo adding a theme should be a one time procedure. This also makes sure that the theme does not get updated in the background but only when the user again calls use_xaringan_theme() (implying that there were upstream changes to the theme meanwhile). Maybe we misunderstood each other on this because I do not see how this approach would break the "1-click knit approach". Maybe you can clarify this one? And in general: What do you think?

I disagree. xaringan can ship with some ready-to-use themes which will always work. There are many factors that may affect the individually hosted themes that make them unavailable (changing the GitHub account name/deleting, renaming the repo, etc.) Thus, I tend to keep the existing themes as-is.

Yes, I see the advantage of built-in themes. The question is just: How do we decide which ones are built-in? Simply all the one up to now? We could do that but the approach would then not be as 100% clean as to say: "We outsource all themes" and just store a dictionary of upstream theme repos. But I am also not the one do make such decisions here :smile: .


Even though we have a wall of text now, maybe @yihui can shortly give a summary of his preferences for all of our approaches on this. This way we avoid starting something without the karma of the creator :)

pat-s avatar Oct 31 '19 15:10 pat-s

I see no difference between calling use_xaringan_theme() once in the console (with all the side effects above) and manually storing the call in the setup chunk. In contrast, I would not want to have use_xaringan_theme() running on every "knit" of the file. Imo adding a theme should be a one-time procedure.

I'll explain. My concern is that when the xaringan creator shares the source Rmd (but not the assets). To reproduce the slide, the reader will need to run use_xaringan_theme() first, then knit.

I've seen similar issues on SO that the posted xaringan source failed to compile on my first try. (Apparently I need to run an independent command summon_remark() first.)

OK let's wait for yihui's comment since there are many ways to move forward :smile:

tcgriffith avatar Nov 01 '19 01:11 tcgriffith

I'll explain. My concern is that when the xaringan creator shares the source Rmd (but not the assets). To reproduce the slide, the reader will need to run use_xaringan_theme() first, then knit.

Valid concern! Haven't thought of this one. This is a good example why it is great having more people think about a problem.

Still I think we should find a middle way. Running the whole process of getting the CSS online and storing it on every knit is not efficient and might even cause problems at some point (because internet connectivity would be mandatory all the time if one wants to use themes).

A simple solution could be: Assert that the CSS file from the theme exists in assets/css and then exit the function. And yes, then you are absolutely correct that we need use_xaringan_theme() in the setup chunk which would do exactly this check. Then, if the Rmd is passed around, it would install the CSS on the first "knit".

pat-s avatar Nov 01 '19 06:11 pat-s

Still I think we should find a middle way. Running the whole process of getting the CSS online and storing it on every knit is not efficient and might even cause problems at some point (because internet connectivity would be mandatory all the time if one wants to use themes).

Yeah you are right. The logic of use_theme() should be

  • local theme CSS exists: do nothing.
  • No local theme CSS exist: download&setup

tcgriffith avatar Nov 01 '19 12:11 tcgriffith

@tcgriffith This idea got a bit stale now. Unfortunately I have no resources to tackle such a big task right now - how do we proceed here? I think it would be important to have Yihui's opinion on this before starting anything in more detail.

pat-s avatar Mar 14 '20 09:03 pat-s

@tcgriffith This idea got a bit stale now. Unfortunately I have no resources to tackle such a big task right now - how do we proceed here? I think it would be important to have Yihui's opinion on this before starting anything in more detail.

I was working on my thesis so I tend to come back for this PR when I have time. On the other hand, yihui said he'd take a look at this thread (personal communication, 2019)but apparently are busy at the moment.

tcgriffith avatar Mar 14 '20 09:03 tcgriffith

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Sep 22 '20 20:09 CLAassistant