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

Improve theme customization #607

Closed
waylan opened this issue Jun 7, 2015 · 8 comments
Closed

Improve theme customization #607

waylan opened this issue Jun 7, 2015 · 8 comments
Milestone

Comments

@waylan
Copy link
Member

waylan commented Jun 7, 2015

In both #597 and #467 possible solutions include making changes to the existing themes so that users can override part of the template without having to re-implement the entire template. While those issues address specific requests, this is intended as an attempt to define a general model/policy for how this should work. Once the decisions are made, someone(s) can go through the templates and start to implement it.

I see three potential approaches:

  1. Includes:

Break out each overridable piece of the template into a separate file and use an include in the parent template to pull it in. The parent template would look like this:

<div>
    <!-- some content -->
</div>

{% include 'somefeature.html' %}

<div>
    <!-- some more content -->
</div>

Then the file somefeature.html would simply contain the template code to implement "somefeature".

If a user wants to override "somefeature," she can create a directory and assign it to the theme_dir config setting. Within that dir, she then creates a file somefeature.html which contains the replacement template for that feature. When the build process runs, Jinja should check the theme_dir first for each "included" template and fall back to the dir of the theme assigned to the theme config setting. That way, the user defined template gets precedence and a user can override a given feature without messing with the entire template.

For example, to disable analytics, a user would simply create a blank file with the appropriate name (analytics.html) and save it to the theme_dir. Or if a user wanted to use a service other than Google's, she could create the appropriate file (analytics.html) which contains the HTML required by her service of choice.
2. Blocks (template inheritance):

Wrap each overridable piece in a separate block, like this:

<div>
    <!-- some content -->
</div>

{% block somefeature %}
    <!-- implementation here -->
{% endblock %}

<div>
    <!-- some more content -->
</div>

The default implementation would then be defined right there in the built-in template (as it is now).

To override "somefeature," the user would create a directory and assign it to the theme_dir config setting. Within that dir, she then creates a template file which "extends" the theme's base template:

{% extends "base.html" %}

{% block somefeature %}
    <!-- custom implementation here -->
{% endblock %}

Jinja would need to be configured so that if a base template exists in the users theme_dir is loads that template as the base template, but it also will look in the theme's dir to load templates which the user's base would extend.
3. Blocks and Includes:

Implement both solutions together so that the include statement is wrapped in a block, like this:

<div>
    <!-- some content -->
</div>

{% block somefeature %}
    {% include 'somefeature.html' %}
{% endblock %}

<div>
    <!-- some more content -->
</div>

Then the user can choose either method to override the default behavior.

Note that in all three scenarios the user does not need to change any settings to alter the behavior except for the theme_dir setting. If a user desires to add their own items to the context (to make them available to their custom template), they can use the extra setting.

A benefit of option 1 is that each feature is broken out into a separate file, which keeps everything modular. Of course, as more features are broken out, the number of files grows. And as a user overrides more features, that users number of files also grows. But users don't need to know anything about template inheritance.

A benefit of option 2 is that there is no additional files (everything can be contained in one file). And the user only needs to add one file. However, that one file could become unwieldy, expressly for beginners. Additionally, users need to understand how inheritance works to at least some extent, which could be overwhelming for beginners.

Option 3 provides the benefits of both and allows the users to eliminate the negatives according to their individual priorities.

Personally, I like option 3. My suggestion would be to use includes (wrapped in blocks) for a few of the more popular features and then wrap most (all) of the various parts of the template in a series of blocks. Fully document the includes (with example code, etc.) for beginners and simply mention that the blocks exist for advanced users.

This proposal was based in part on observing how various Sphinx themes have addressed this issue. A few have implemented option 3 and those themes now have a bunch of alternate themes available (some thirds party, some built-in) which inherit from them using blocks. At the same time, users can easily override a few basic features using includes. When the alternate theme also makes use of the includes (often including the parent's implementation), the user can even use the same override mechanisms when using the alternate theme. To be clear, I'm not suggesting MkDocs should strive to build up such a large collection of (only slightly different) built-in themes, or even support multiple levels of inheritance. I just mention it as an example of the flexibility option 3 could potentially provide users.

Any and all feedback is welcome.

@d0ugal
Copy link
Member

d0ugal commented Jun 8, 2015

Thanks for writing this up, I am trying to find the time to fully pay attention to it and give it some thought before replying.

@waylan
Copy link
Member Author

waylan commented Sep 23, 2015

I have done some more research on how to implement this and I found a potential issue with the use of blocks. As I explained previously:

Jinja would need to be configured so that if a base template exists in the users theme_dir is loads that template as the base template, but it also will look in the theme's dir to load templates which the user's base would extend.

The user would then create a base.html template which defined the overriding blocks. However, what template would this template extend? If the user's base.html (defined in the theme_dir) is loaded, then when it {% extends base.html %} it would not extend the base.html defined in the theme, but it would extend itself, which is not what we want.

Edit: See this comment for a simpler solution.

A workaround it to use a PrefixLoader. But then what prefix would we use? If the prefix is the name of the theme, then one might do {% extends readthedocs/base.html %}, but what if the user does not have the theme config set to readthedocs? Perhaps the prefix should be a generic fixed name (the name would not change regardless of the theme config setting) and it will always point to whichever theme is defined in theme. Something like {% extends base/base.html %}. Not sure if "base" is the best name though.

The problem is that MkdDocs code needs to be aware of theme prefixes. Every attempt to load a template from the Environment will need to include the prefix. So a call to load base.html would need to first check if theme_dir is set, and if so try to load theme_dir/base.html and if that fails, load base/base.html. The prefix loader won't do that fallback for us. Now, MkDocs could define its own custom loader which handles all this, but that is still more code to maintain in MkDocs.

I liked the idea of blocks because it leaves the existing templates pretty much as-is (only a few additional lines of template code are added). However, there is a lot more work in the Python code to make it work properly. However, while the "includes" method would require a major refactor of the templates (breaking one file up into a bunch of "included" files) there would be no changes needed on the Python code side of things.

@waylan
Copy link
Member Author

waylan commented Sep 23, 2015

I should note that there is an alternative (simpler) solution to my previous comment to make extending blocks work. In the included themes simply rename base.html to something else (we'll call it main.html). Then create a new base.html which only includes the line {% extends main.html %} and nothing else. Then the user would create a base.html file in their theme_dirwhich also extends main.html. No Python code changes necessary. However, it does require an extra, nearly blank template.

If this is the route taken, I would think that the use of "base" and "main" should probably be reversed. But then MkDocs would need to be altered to load main.html rather than base.html as it does now. And that means that any preexisting third party templates would all need to rename their base templates. Maybe if someone comes up with a better name that "main" we can leave the use of the base.html name as-is.

@waylan
Copy link
Member Author

waylan commented Feb 5, 2016

So far this discussion has been about the templates. Just adding a note regarding CSS and JavaScript for themes. Regardless of whether we use the options I mentioned in #756 or not (less or sass), it might make sense to compress/minify CSS and JS files to reduce download size and/or number of files. Sure, a user can use extra_javascript and extra_css to add all the extra stuff they want, but each file is another file to download and things could get heavy fast. Combining and compressing the files would be a beneficial optimization. A few interesting Python libs that do this are fantastic and django-compressor. While each provides a specific solution that wouldn't work directly (wsgi app and django app respectively), they offer some nice patterns and each rely on third party libs to do the actual work. They could provide some nice inspiration for how to design the public facing API in MkDocs.

And then there was this comment:

Maybe we create a class that represents a media directory, then a subclass of it could represent a theme - initially that would just exclude Python files, but later it might do more.

I'm thinking a Theme class could be helpful to address all of this stuff.

@waylan
Copy link
Member Author

waylan commented Feb 16, 2016

#826 highlights another problem with theme customization: how to disable a JS/CSS feature. In the case of #826, the feature is search, but the same applies to client side syntax highlighting. In both cases, there are JS and CSS that is served with every page that is not needed. With a fully customized theme, this is not a problem, but when overriding/customizing a provided theme, the only option now is to create a blank search.js file to replace the default. That still causes an extra (now completely unnecessary) http request. Adding an exclude_javascript setting would work, but something that uses less settings would be preferred. Compressing/minifying all JS to one file would be a solution, but the 'create a blank file' approach feels like a hack, not something that should be the official API.

@d0ugal
Copy link
Member

d0ugal commented Feb 16, 2016

but the 'create a blank file' approach feels like a hack, not something that should be the official API.

Yup, so hacky :)

waylan added a commit to waylan/mkdocs that referenced this issue May 24, 2016
waylan added a commit to waylan/mkdocs that referenced this issue May 24, 2016
waylan added a commit to waylan/mkdocs that referenced this issue May 24, 2016
waylan added a commit to waylan/mkdocs that referenced this issue May 25, 2016
waylan added a commit to waylan/mkdocs that referenced this issue May 25, 2016
waylan added a commit to waylan/mkdocs that referenced this issue May 25, 2016
waylan added a commit to waylan/mkdocs that referenced this issue May 25, 2016
waylan added a commit to waylan/mkdocs that referenced this issue May 25, 2016
As ReadTheDocs already had a few blocks defined with slightly differant
names, those names were adopted globally.

Partially addresses mkdocs#607.
waylan added a commit to waylan/mkdocs that referenced this issue May 25, 2016
As ReadTheDocs already had a few blocks defined with slightly differant
names, those names were adopted globally.

Partially addresses mkdocs#607.
waylan added a commit to waylan/mkdocs that referenced this issue May 26, 2016
waylan added a commit to waylan/mkdocs that referenced this issue May 26, 2016
As ReadTheDocs already had a few blocks defined with slightly differant
names, those names were adopted globally.

Partially addresses mkdocs#607.
waylan added a commit to waylan/mkdocs that referenced this issue May 27, 2016
This is a backward incompatable change as it switches the primary
template from base.html to main.html.  See the release notes for
details.

Partially addresses mkdocs#607.
pjbull pushed a commit to pjbull/mkdocs that referenced this issue Jun 25, 2016
This is a backward incompatable change as it switches the primary
template from base.html to main.html.  See the release notes for
details.

Partially addresses mkdocs#607.
@waylan
Copy link
Member Author

waylan commented Nov 2, 2016

Just noticed this was still open. This has been addressed across multiple PRs, many of which are linked above. While some of the suggestions above have not been implemented, those were questionable anyway and the major thrust of this issue has been implemented. I see no reason to leave this open.

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

No branches or pull requests

3 participants