Make compatible with remote_theme

[gpotter2]

Add support for remote_theme:

• move all css/js/img to assets
• update all references accordingly
• make all data files optional
• add default config values
• doc

This is the "easy", low effort way of making it work. Eventually you could use scss...

@daattali
fix #339

[skalee]

Data files are not available. This breaks social icons in the footer, for instance. Can be workarounded by copying these files to the target repo.

[daattali]

@skalee is there no way to fix this without requiring more work by the end user?

[skalee]

@daattali No idea. I'm a Jekyll beginner.

---

Also, some parts of theme's _config.yml need to be copied to the project's _config.yml as well. In particular, this applies to following colour definitions:

beautiful-jekyll/_config.yml

Lines 39 to 49 in a04be27

	# Personalize the colors in your website. Colour values can be any valid CSS colour
	navbar-col: "#F5F5F5"
	navbar-text-col: "#404040"
	navbar-children-col: "#F5F5F5"
	page-col: "#FFFFFF"
	link-col: "#008AFF"
	hover-col: "#0085A1"
	footer-col: "#F5F5F5"
	footer-text-col: "#777777"
	footer-link-col: "#404040"

Otherwise, blank strings are used, and resulting CSS is slightly broken, for example this rule:

beautiful-jekyll/css/main.css

Lines 73 to 77 in a04be27

	::selection {
	color: white;
	text-shadow: none;
	background-color: {{ site.hover-col }};
	}

becomes:

::selection {
  color: white;
  text-shadow: none;
  background-color: ;
}

In the result, selected text is white font on white background, at least in my browser.

---

Having said that, remote-theme capability is a HUGE improvement, even if a few additional steps must be done by user.

[daattali]

Thanks both

This is indeed something I procrastinated with for way too long. I just spent some time reading over the documentation for this to understand it better.

I'm 100% onboard with merging this PR! But I do want to work out the little quirks so that it'll be a smooth experience for the user. It seems like there are two issues currently:

1. The _data folder doesn't get used in a gem-based theme. Do you either of you @skalee or @gpotter2 know how other themes handle this? If we use the idea of moving these files into assets/, do you know if assets/ is capable of serving any type of file (rather than only css/js/etc)?

2. There are some _config.yml values that must be defined for the theme to work properly. What do you think of changing any code that uses such a value to use a default value?

[daattali]

Also, would merging this PR mean that the master branch would be a proper ruby gem and #269 would be solved organically, correct?

[daattali]

I'd be very happy to get this PR merged if you get some time to continue the discussion

[gpotter2]

I think the user will need to copy _data anyways. Assets isn't able to provide its data and it seems adding "default _data" is on the roadmap for jekyll 4.1.

• no this isn't enough to make beautiful-jekyll a gem theme (you'll need a .gemspec....)

[gpotter2]

@daattali This PR will not break anything: forking the repo will still work. However it will enable us to use remote theme by only providing _data. Please merge asap

[daattali]

Thanks for letting me know about the 4.1 Jekyll roadmap - it does look like soon enough the _data issue won't be an issue anymore.

I do want to merge this PR ASAP, but only when it is complete. I know it won't break anything, but I don't want to have a feature that's not fully working.

From my current understanding all that needs to happen to make this PR work 100% is:

1. Add a couple sentences in the README about how to use this theme as a remote theme. This should include instructions for copying the _data folder! (this must be part of this PR - every feature that gets added must have its associated documentation)

2. Any CSS values, or any other values on the site, that require config values from the config file should use some sensible default value, to accommodate the case of when the config file is empty (this can be a separate PR/issue, but it does need to happen before this is merged)

[gpotter2]

I've added some doc. Feel free to review it.
I'm not up for replacing all the values with default ones..

For people who are in my case, you can use gpotter2/beautiful-jekyll until this gets merged.

[daattali]

In the README the first step is " have a working jekyll project" - do you mean having a jekyll project locally? Or do you mean just have a github repository? I was hoping this would work with github pages, and the documentation should make it clear how to use this on a GH repo

I completely understand you not wanting to deal with the default values issue - I'm doing a lot of work for free these days analyzing COVID for hospitals and research groups that ask me for help, so I don't have much free time to dedicate to this, hopefully someone else can find some time to do that

[gpotter2]

I've looked into it:

• staticman uses _data/ui-text, but already has default values in place. No changes are required
• the footer uses _data/SocialNetworks.yml, only to get the icons & link templates.

The thing is that there's no real default value that would work for the footer links. Should we disable the links entirely when the file isn't available?
If so, should we also move the social network configs from _config.yml to SocialNetworks.yml for consistency? Otherwise people wouldn't realize it's not being displayed when the SocialNetworks.yml is missing.

I'll try to work on this. What do you think @daattali ?. Thanks for your help
Stay safe

[daattali]

@gpotter2 (I deleted the previous comments in favour of putting all the info here)

1. You created 3 commits, the first one is specifically relevant to this PR, the second one (adding default values) is related to this PR but can also be standalone, and the third one (round avatar) is not related to the PR. Do you mind submitting the last 2 commits as separate PRs? The default values commit should make a reference to Use sensible default values for any value that's read from the config file #600 . This PR shouldn't have those two commits in it.

2. According to your documentation, does the remote_theme not work with github pages? Will it only work in local ruby installations? From my (limited) understanding, I thought remote_theme is supported by github pages and should be very similar to how it works with local installations

3. Regarding staticman: we can just add a single sentence in the readme under the remote_theme section that says if you're using staticman, you should copy the _data/ui-text folder

4. Regarding social network links: because in the near future this problem will be solved automatically (when jekyll includes the _data folder in the next version), I would say that we shouldn't touch the code too much - leave everything related to this feature where it currently is. Add a simple check in the code that checks if this data is available and if it isn't then just don't try to render this section. In the README under the remote_theme section, add a sentence that says that you need to copy the _data/SocialNetworks.yml folder

Does this all make sense?
Thanks for all your work

[gpotter2]

Hi,

1. Makes sense, will do
2. Yes the goal is first and foremost to add support for remote-theme. I'm using https://github.com/gpotter2/secdev.github.io/blob/gh-pages/_config.yml as test platform, to give you an idea of what it'll look like. Build is currently failing but I'll fix it soon enough
3. ok
4. I prefer my changes because i don't believe users ever read the doc (e.g. the massive amount of PRs you get that are mistakes, despite the template). If i were you i would make it so that it either works, either not at all. Anything in between will bring you even more issues of confused users... In the end, you're the one providing end user support so it's up to you

[daattali]

Sorry I'm confused by the answer to 2. "Yes" means that GitHub Pages is supported, or not?

I agree that people won't really read the full docs. What do you think of having a sentence inside the config file , above the start of the social networks list, that says "If you're using remote_theme, you need to copy the xxx folder for these to work"

[gpotter2]

Github pages is supported through remote_theme. see the example i linked above

[daattali]

Great! The first goal of this theme is to be GitHub Pages-first, so I would prefer to say in the readme how to do this in a GitHub Pages site (like how most of the documentation focuses on github pages) rather than saying "create a jekyll project".

[gpotter2]

@daattali I think I've addressed all your points. I've linked to the Github pages quickstart guide because that's exactly what people are supposed to do, and copy pasting it would be useless.
I also squashed the commits when necessary

[gpotter2]

Also note you shouldn't include Gemfile.lock in a gem (especially when the only dependency is github-pages....). see https://yehudakatz.com/2010/12/16/clarifying-the-roles-of-the-gemspec-and-gemfile/

I've "only" updated it here because I assume it's here for a reason.

[daattali]

I trust you on the gemfile thing - again, I don't really deal with ruby/gems, so I have to take your word for it.

I did see a PR for the round avatar, but I didn't see a separate PR for the config default values, those changes still seem to be in this PR

[skalee]

Thanks for letting me know about the 4.1 Jekyll roadmap - it does look like soon enough the _data issue won't be an issue anymore.

FYI github-pages gem is still using Jekyll 3.x, and it doesn't look like they're going to upgrade soon.

[daattali]

That's ok - at some point in the future it will be upgraded, and until then it's fairly simple to have one sentence in the config.yml in the social icons section saying that if you're using remote_theme, you just need to copy this file

[gpotter2]

but I didn't see a separate PR for the config default values, those changes still seem to be in this PR

The commits are smashed together. It makes sense IMHO for it to be included here. (also it's a pain to split commits)

[gpotter2]

Also it seems the data feature was removed from the 4.1 roadmap 6 days ago.. honnestly it could also be me that doesn't exactly understand their roadmap.. I wouldn't expect anything from there

[gpotter2]

All done.

[daattali]

Is there any way for me to test this? Do you have a github repo set up with this that I can use as a remote?

[gpotter2]

Yes, I'm using the master branch of https://github.com/gpotter2/beautiful-jekyll to host this PR.
remote_theme: gpotter2/beautiful-jekyll works.

I'm also using https://gpotter2.github.io/secdev.github.io/ as my debug website. It uses this PR as theme

[daattali]

How are you using this PR as a theme? What value do I use as remote_theme in the config file to point to this pull request? I couldn't figure it out easily

[gpotter2]

This PR is the master branch of https://github.com/gpotter2/beautiful-jekyll.
remote_theme: gpotter2/beautiful-jekyll

[daattali]

Is there a way to automatically copy over the index.md file? Because right now if I create a repo and use remote_theme and add the config file, I'll get a 404. An index file is required, unless there's something I'm missing?

[gpotter2]

A theme isn't really supposed to add an index page :/ no its not possible (nor should it be), at least as far as i know.

[daattali]

You're right. I've tested it with GitHub Pages, looks good. How do I test it with local jekyll installations? Or does this only work with GH Pages and we need to make it a gem in order to use locally?

[gpotter2]

If you're testing a github pages project locally, follow the instructions on https://github.com/github/pages-gem/blob/master/README.md (you'll need to create a Gemfile).

If your testing a project outside of github pages, you can manually use https://github.com/benbalter/jekyll-remote-theme

[skalee]

I confirm that at least in case of my site all works as of 56c4cea.

[daattali]

I didn't like that if I run jekyll new and then add this theme, the homepage doesn't work at all, because the default jekyll assumes a home layout in the index page.

So I created a home layout and use it now in the index page, so that if someone uses jekyll new and wants to use this theme, they'll be able to see something immediately. I'll continue testing and hopefully merge tomorrow.

@gpotter2 @skalee do you know what's required to make this into a proper Ruby gem?

[gpotter2]

You probably just need to update the Gemfile and add a gemspec. there are lots of great tutorials out there

Just another thought: remote theme has no versioning, so please note that once it goes live, you will be in charge to make sure any update you do is always backward compatible. If you want to make breaking changes without affecting users (e.g. change how the config values work, for instance you just changed how "author" was stored), now is the time.

[daattali]

Good point, thanks for making that clear. I'm glad I spent a lot of time this week overlooking the entire config file.

[gpotter2]

Great thanks for the merge. (I also noticed I forgot to add a credit 😄)

[daattali]

I added both of you to the changelog

…

--- Dean Attali President & CEO AttaliTech Ltd http://AttaliTech.com <http://attalitech.com>

On Sun, 26 Apr 2020 at 14:35, Gabriel ***@***.***> wrote: Great thanks for the merge. (I also noticed I forgot to add a credit 😄) — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#590 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHIQFHFJJTZQYJPBH4SJS3ROR5GXANCNFSM4LSVB2GA> .

[skalee]

Just another thought: remote theme has no versioning, so please note that once it goes live, you will be in charge to make sure any update you do is always backward compatible. If you want to make breaking changes without affecting users (e.g. change how the config values work, for instance you just changed how "author" was stored), now is the time.

It does support versioning, see https://github.com/benbalter/jekyll-remote-theme#declaring-your-theme. For instance, to freeze on what's currently on master, following could be specified: daattali/beautiful-jekyll@790bbf59a248596317a47353c45b4823d7fcf471 And with Git tags it can be much more user-friendly.

[gpotter2]

Nice catch.
Git tags are a good idea. For simplicity you probably want to start the versioning where the gems left off.

[daattali]

Thanks, I'll make a tag indeed.

[daattali]

@gpotter2 I just noticed that there are two instances where you didn't migrate the js/css into assets:

https://github.com/daattali/beautiful-jekyll/blob/master/_includes/staticman-comments.html#L77-L81

and

beautiful-jekyll/_includes/head.html

Line 130 in 6552c57

<link rel="stylesheet" href="{{ "/css/staticman.css" | relative_url }}" />

Is this correct, was this done on purpose for any reason?

[gpotter2]

No i missed those. It wasn't obvious in the testing

[daattali]

Everything has been addressed, and it works great. @skalee in the documentation I wrote that they should use beautiful-jekyll-theme@2.3.0 and that every so often they can check for new versions

[gpotter2]

Looking good, great job 🎉