Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Introduce core theme. #2810

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
Contributor

gwax commented Jun 2, 2017

Pull Request Checklist

  • I’ve read the guidelines for contributing.
  • I updated AUTHORS.txt and CHANGES.txt (if the change is non-trivial) and documentation (if applicable).
  • I tested my changes.

Description

Closes #2744

Introduces new core and core-jinja themes to serve as lower level starting points for theme developers. These themes remove nearly all style and js based functionality from base. Additionally, base and base-jinja are redirected to derive from core and core-jinja respectively.

+ // copy element attributes to the wrapper
+ while (index--) {
+ node = nodes[index];
+ node.specified && wrapper.setAttribute(node.nodeName, node.nodeValue);
+ // copy element attributes to the wrapper
+ while (index--) {
+ node = nodes[index];
+ node.specified && wrapper.setAttribute(node.nodeName, node.nodeValue);

@gwax gwax referenced this pull request Jun 2, 2017

Closed

base theme has too much stuff #2744

+ // copy element attributes to the wrapper
+ while (index--) {
+ node = nodes[index];
+ node.specified && wrapper.setAttribute(node.nodeName, node.nodeValue);
Owner

Kwpolska commented Jun 2, 2017

GitHub makes it a bit of a pain to review, so here are comments:

  • I’m not sure if nikola_ipython.css can be removed from core.
  • ipython.min.css should still be there
  • rst.css bootstrap-esque styles should still be there, at least in bootstrap3, maybe in base
  • fancydates.js should be included, or at least available in core/assets. Fancy dates are an advertised feature (in conf.py), so it shouldn’t be limited in non-base themes
  • How is aria-label="${messages('(active)', lang)}" handled by screen readers?
  • Why html5shiv-printshiv?

@Kwpolska Kwpolska referenced this pull request Jun 2, 2017

Closed

Ignoring assets #2812

Owner

Kwpolska commented Jun 2, 2017

I’ve had some time to think about this. You see, the existence of a core and base theme side by side can be confusing. If someone just looks at the theme list, they won’t be able to tell the difference between those two. So, perhaps it would be better to swap the roles here? I think that it would work better if base was still the root theme in our hierarchy, with all the “opinions” (baguettebox/flowr/whatever). And then there would be a base-micro theme, with no styling and such. Now, you may say that this would still copy the unwanted assets into sites — and I think this solvable by making it possible for themes to ignore assets from parent themes. I created #2812 for that (regardless of what happens with the core theme).

Contributor

gwax commented Jun 2, 2017

@Kwpolska Those are pretty solid points and, for me, they raise the question of what the purpose of the base theme is in the first place. It also calls to question why base and bootstrap3 live in nikola while all of the other themes live in nikola-themes.

Some of this may be better to discuss in a v8 context where breaking changes might be permitted in themes.

Owner

Kwpolska commented Jun 2, 2017

bootstrap3 is our default theme, and it’s a good-enough appearance. Nikola’s philosophy is “install and go” — you do not need to look for a theme and other stuff to create a site (and that is not going to change). We also need base to provide some stuff to all themes, including non-bootstrap ones.

Including other themes would increase bloat (and our packages are already quite large) and introduce a burden. You see, some of the themes in the index are not well supported, were contributed by other people, and we don’t have the resources to look after all of them. So we have a repository that everyone can contribute to, and where everyone can find a theme they like.

v8 should probably happen one day, but we’re a bit wary of just making a release to delete stuff… we probably should get it done soon-ish though. That said, I’m not sure I fully agree with your vision for base/core, whatever the names.

Create core theme below base with fewer opinions.
Duplicate base to core

modify metadata and remove duplicate messages

trim and update assets

strip things down in core and remove from base

update rss files

typo

feed consolidation

feed_helper fixes

do not pre-minify

carve down base a bit

fix translations

temporarily move things around to facilitate diffs

add some links to fix diffs

add some links to fix diffs

rst.css

diverge less

back to the new names

readd cz

fix bundle

unremove annotation_helper

remove unneeded assets

fix translation messages

modify jinjify to handle core and possible bootstrap deletion

use minified assets

jinjify and fix metadata for jinja themes

Update changelog

resynchronize base with core

ipython.min.css

html5shiv from bower

@gwax gwax referenced this pull request Jun 3, 2017

Merged

Update and strip base #2815

3 of 3 tasks complete
Owner

Kwpolska commented Jun 7, 2017

I‘m closing this. If you still want to make a base-micro theme (what would the difference be?), go ahead.

@Kwpolska Kwpolska closed this Jun 7, 2017

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