Jinja version of bakerydemo #452
Conversation
thibaudcolas
force-pushed
the
jinjademo-wip
branch
from
ee3856f
to
2d58c37
Compare
|
|
||
| from wagtail.contrib.search_promotions.templatetags.wagtailsearchpromotions_tags import ( | ||
| get_search_promotions, | ||
| ) |
There was a problem hiding this comment.
This is the only bakerydemo feature for which we don’t have Jinja support directly in Wagtail.
bakerydemo/jinja2/base.html
Outdated
| {% endblock %} | ||
| </title> | ||
| <meta name="description" content="{% if page.search_description %}{{ page.search_description }}{% endif %}"> | ||
| <meta name="description" content="{{ page.search_description if page and page.search_description }}"> |
There was a problem hiding this comment.
This isn’t good in the base bakerydemo as views that aren’t page will have the meta tag with an empty value. Resolved in #453.
| @@ -19,7 +17,7 @@ <h1 class="index-header__title index-header__title--blog">{{ page.title }}</h1> | |||
| {% endif %} | |||
| {% if page.date_published %} | |||
| <div class="blog__published"> | |||
| {{ page.date_published }} | |||
| {{ page.date_published|date("DATE_FORMAT") }} | |||
There was a problem hiding this comment.
I’m surprised Jinja doesn’t do this by default, perhaps there’s a way with configuration.
bakerydemo/jinja2/tags/gallery.html
Outdated
| <div class="picture-card__contents"> | ||
| <p class="picture-card__title">{{ img.title }}</p> | ||
| </div> | ||
| </figure> | ||
| </div> | ||
| {{ image.title }} |
There was a problem hiding this comment.
This looks odd and doesn’t seem to do anything even in the base demo site. Resolved in #453 too.
| {% for menuitem in menuitems %} | ||
| <li class="presentation {{ menuitem.title|lower|cut:" " }}{% if menuitem.active %} active{% endif %}{% if menuitem.show_dropdown %} has-submenu{% endif %}"> | ||
| <li class="presentation {{ menuitem.title|lower|cut(" ") }}{{ "active" if menuitem.active }} {{ "has-submenu" if menuitem.show_dropdown }}"> |
There was a problem hiding this comment.
This "active" class does nothing currently so I’m not entirely sure why we have it.
| {% image self.image fill-600x338 loading="lazy" %} | ||
| <figcaption>{{ self.caption }} - {{ self.attribution }}</figcaption> | ||
| {{ image(value.image, "fill-600x338", loading="lazy") }} | ||
| <figcaption>{{ value.caption }} - {{ value.attribution }}</figcaption> |
There was a problem hiding this comment.
Those fields are optional, which looks a bit odd right now in the base site.
thibaudcolas
force-pushed
the
jinjademo
branch
from
1df09e4
to
fe949e3
Compare
thibaudcolas
force-pushed
the
jinjademo-wip
branch
from
2d58c37
to
7c4b351
Compare
thibaudcolas
force-pushed
the
jinjademo-wip
branch
from
7c4b351
to
b0fc20f
Compare
|
@laymonage this should actually be ready to review now (but this is very low-priority). |
Targets a separate Jinja branch, which will remain fully separate from
main. I thought I would open this as a pull request for feedback and also so there’s a good record of exactly what is needed to get a site likebakerydemoconverted to Jinja. @laymonage and I discussed keeping the separatejinjademobranch for future compatibility testing with Jinja, though we’d need further consensus from other maintainers on whether this is worthwhile or not.This does not include an initial commit moving all templates from
templates/tojinja2/.Implementation notes:
mainin the future.Because of those two choices, there are quite a few things in there that you’d probably not do this way in a Jinja project where the focus is reducing tech debt – but probably make sense with a focus on easy maintenance for us as a test site.