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

Add support for extra_css and extra_javascript in all the themes #90

Merged
merged 1 commit into from Oct 1, 2014

Conversation

@jimporter
Copy link
Contributor

@jimporter jimporter commented May 24, 2014

I noticed that most of the themes don't actually support extra_css. I think I saw you mention that these themes would eventually be removed from mkdocs core and be turned into add-ons or something, but in the meantime, I think it makes sense to maintain feature parity across the themes.

I'm not sure if you'd like tests for this, but if you do, just let me know and I'll try to figure something out.

@d0ugal
Copy link
Member

@d0ugal d0ugal commented Aug 8, 2014

This change looks good to me, I think a test would be useful to verify it is working with one of the themes. If nothing else it'll stop us getting into this situation again.

@d0ugal d0ugal added the bug label Aug 8, 2014
@jimporter
Copy link
Contributor Author

@jimporter jimporter commented Aug 13, 2014

I like tests! What's the best way to write a test for this?

@dnrce
Copy link
Contributor

@dnrce dnrce commented Sep 2, 2014

@jimporter I just saw this after opening #135. You may wish to consider the same change from

{% for path in extra_css %}<link href="{{ base_url }}/{{ path }}" rel="stylesheet">{% endfor %}

to

{%- for path in extra_css %}
<link href="{{ base_url }}/{{ path }}" rel="stylesheet">
{%- endfor %}

(with appropriate leading whitespace), which generates prettier HTML.

@jimporter
Copy link
Contributor Author

@jimporter jimporter commented Sep 2, 2014

@zoombody Noted. If your PR lands, I'll be sure to update this one.

Still not sure where tests for this should go, though...

@dnrce
Copy link
Contributor

@dnrce dnrce commented Sep 2, 2014

Also, since dc84d21 the use of full URLs is allowed in extra_* so the {{ base_url }}/ piece should be dropped.

@jimporter
Copy link
Contributor Author

@jimporter jimporter commented Sep 2, 2014

I updated and noticed that 7167406 totally broke the custom themes, since config isn't a variable for the templates anymore...

I'll file a new PR for that.

@Keats
Copy link

@Keats Keats commented Sep 5, 2014

Looks good, could you add the extra_javascript that are missing as well ? (also +1 for @zoombody use of {%-)

@d0ugal
Copy link
Member

@d0ugal d0ugal commented Sep 5, 2014

I think practicality wins over purity here, we don't need a test. I'll give this a manual test next week.

@jimporter jimporter force-pushed the jimporter:extra-css branch from 06afac6 to 0464f19 Sep 6, 2014
@jimporter jimporter changed the title Add support for extra_css in all the themes Add support for extra_css and extra_javascript in all the themes Sep 6, 2014
@jimporter
Copy link
Contributor Author

@jimporter jimporter commented Sep 6, 2014

Ok, I updated this to include extra_javascript and to do the multi-line thing.

@jimporter jimporter force-pushed the jimporter:extra-css branch from 0464f19 to 6c33fe8 Sep 12, 2014
This was referenced Sep 14, 2014
@d0ugal
Copy link
Member

@d0ugal d0ugal commented Oct 1, 2014

Sorry for the tardy followup.

d0ugal added a commit that referenced this pull request Oct 1, 2014
Add support for extra_css and extra_javascript in all the themes
@d0ugal d0ugal merged commit 745b7d0 into mkdocs:master Oct 1, 2014
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
@jimporter jimporter deleted the jimporter:extra-css branch Feb 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants