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

Introduce a more-minimal base theme. #120

Closed
wants to merge 4 commits into from

Conversation

@gwax
Copy link
Contributor

gwax commented May 22, 2017

This is a somewhat more austere base theme, meant to provide details for the discussion in getnikola/nikola#2744

</nav>
</%def>

<%def name="html_translation_header()">

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska May 22, 2017

Member

What’s wrong with keeping it in base_helper.tmpl under the old name?

@@ -0,0 +1,10 @@
## -*- coding: utf-8 -*-

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska May 22, 2017

Member

What did you change here? Avoid duplicating templates if you only delete the namespace (which I agree is unnecessary, perhaps we should to that upstream)

else:
atom_ref = kind + '_atom'
rss_ref = kind + '_rss'
label = ' for ' + kind + ' ' + name

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska May 22, 2017

Member

Hm. I’d really like to see something like this upstream, but with i18n support, optional JSONFeed support, and without a raw Python block (which are not allowed in Jinja2) — changes to the core codebase to allow this everywhere would be more welcome.

@@ -0,0 +1,19 @@
## -*- coding: utf-8 -*-

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska May 22, 2017

Member

I would keep that mathjax_script helper and not change this template. Perhaps some people want to switch from base to base-austere, and they will get unusual errors about missing things.

@@ -0,0 +1 @@
<%inherit file="story.tmpl"/>

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska May 22, 2017

Member

Delete this file due to no changes.

@@ -0,0 +1,330 @@
/*

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska May 22, 2017

Member

No need to duplicate this file if you inherit from base.

${comments.comment_form(post.permalink(absolute=True), post.title(), post._base_path)}
</section>
% endif
${math.math_scripts_ifpost(post)}

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska May 22, 2017

Member

I had to make a diff and I found out we had a bug in base (scripts vs styles). I fixed that in getnikola/nikola@d9ee04f, but it’d be great if you would also make changes to the built-in base theme.

(That file is unnecessary after that fix.)

@Kwpolska

This comment has been minimized.

Copy link
Member

Kwpolska commented May 22, 2017

In rst.css:

/* reset inner alignment in figures */
-.figure.align-right {
+div.align-right {
   text-align: inherit }

Why?

<meta name="twitter:creator" content="${twitter_card['creator']}">
%endif
%endif
</%def>

This comment has been minimized.

Copy link
@Kwpolska

Kwpolska May 22, 2017

Member

Restore mathjax_script as well.

@gwax

This comment has been minimized.

Copy link
Contributor Author

gwax commented May 26, 2017

Maybe I'll try to cut a PR against upstream for the structural changes and then see what's left.

@Kwpolska

This comment has been minimized.

Copy link
Member

Kwpolska commented Aug 21, 2017

What’s the status of this? Do we really need this?

@gwax

This comment has been minimized.

Copy link
Contributor Author

gwax commented Aug 21, 2017

I think enough stuff got pared down in my rework of the base theme that this can be closed out.

@gwax gwax closed this Aug 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.