Jinja2 extension

[dperetti]

Any plan to support wagtailmenus' tags through a jinja2 extension ? That would be awesome !

[ababic]

@dperetti No, no plans I'm afraid. I'm not familiar with Jinja2 at all. I wouldn't know where to start.

[dperetti]

It's really worth it. When you start using it, you don't want to go back ! Very powerful and very clean... and wagtail supports it ! From what I've seen of wagtailmenus' code, it wouldn't require any deep changes at all. If you're willing to accept a pull request, I might feel like doing it :-)

[ababic]

@dperetti If you'd be happy to add tests and documentation to go along with it, a pull-request would be welcome, thanks.

[hongquan]

My temporary solution is to copy main_menu and sub_menu templatetags code to a custom Jinja2 environment, doing some modification to make it works with Jinja2 API. Some notes:

1. Original main_menu take a context as 1st agrument, so you have to wrap it like this

class MenuExtension(Extension):
    def __init__(self, environment):
        super().__init__(environment)
        environment.globals.update({
            'main_menu': jinja2.contextfunction(main_menu),
            'sub_menu': jinja2.contextfunction(sub_menu),
        })

2. Unlike Django engine, the context of Jinja2 engine is immutable, so when in original main_menu, you do:

c = copy(context)
c.update({
    'menu_items': {}
})

in Jinja2, you must do

c = context.parent
ctx = {'menu_items': {})
c.update(ctx)

[jorenham]

@hongquan I can't seem to get your solution to work; the context argument is missing for the main_menu and sub_menu functions.

[hongquan]

@jorenham Your source copy may be changed? The original source code has context argument: https://github.com/rkhleics/wagtailmenus/blob/master/wagtailmenus/templatetags/menu_tags.py#L27

[ababic]

Is anyone willing to work on a pull-request to add this functionality?

I've just merged some changes into master that I think will help with the 'immutable context objects' part of the problem (see https://github.com/rkhleics/wagtailmenus/commit/a8c81d465358c316880f6865afec8768944d6fe5) (though, I could be wrong).

So, i'm hoping it should just be a case of adding the Extension somewhere, and adding some docs/tests?

[hongquan]

I wanted to work on it, but currently I shift my focus away my web app which uses wagtailmenus. I have working code, but only cover some cases (no modification for flat_menu yet).

[ababic]

Thanks for replying @hongquan. I'd really like to avoid having an entirely separate set of tags to maintain if possible, as it will make further development difficult. I'm hoping we can get away with changing just enough in the original tags so that they work for both engines, then just define the Extension itself in a separate file. Is this realistic/possible?

I'm guessing we'll also need to add jinja-compatible versions of all included menu templates too? This I can live with, as template changes are incredibly rare.

What I'm most unsure about is how to extend the test suite to ensure Jinja compatibility remains intact once it's in place. But I suppose that can be tackled last of all.

Even if you didn't do the work yourself, any advice you (or anybody else) can share on the above topics would be much appreciated.

[hongquan]

I'd really like to avoid having an entirely separate set of tags to maintain if possible, as it will make further development difficult. I'm hoping we can get away with changing just enough in the original tags so that they work for both engines, then just define the Extension itself in a separate file. Is this realistic/possible?

I think it is possible, like the upstream wagtail does.

[hongquan]

I have pending PR #190 which is hoped to solve this.

With that PR being merged, you can setup like this to use Jinja2 template:

1. Create jinja.py file:

import jinja2
from jinja2.ext import Extension
from wagtailmenus.templatetags.menu_tags import main_menu, sub_menu

class MenuExtension(Extension):
    def __init__(self, environment):
        super().__init__(environment)
        environment.globals.update({
            'main_menu': jinja2.contextfunction(main_menu),
            'sub_menu': jinja2.contextfunction(sub_menu),
        })

2. Setup TEMPLATES in settings.py (not complete):

TEMPLATES = (
    {
        'NAME': 'django-jinja',
        'BACKEND': 'django_jinja.backend.Jinja2',
        'DIRS': [os.path.join(BASE_DIR, 'templates')],
        'APP_DIRS': True,
        'OPTIONS': {
            'environment': 'project.jinja.environment',
            'match_extension': '.jinja',
           'extensions': DEFAULT_EXTENSIONS + [
                'wagtail.wagtailcore.jinja2tags.core',
                'wagtail.wagtailadmin.jinja2tags.userbar',
                'wagtail.wagtailimages.jinja2tags.images',
                # My extension for wagtailmenu
                'project.jinja.MenuExtension',
            ],
        }
    },
)

3. Write your Jinja2 template for menu and change WAGTAILMENUS_DEFAULT_MAIN_MENU_TEMPLATE setting.

My convention is that Jinja2 template files are named with .jinja extension and saved in templates folder.

[ababic]

Hi @hongquan.

My apologies. I've had to revert the merge of your PR (#190) for now, because I'm not 100% sure about the reasoning for some of the changes (I should have flagged this before merging, but I wasn't really thinking clearly this past week, due to other pressures). There are quite a few backwards-incompatible changes in the pull that I need to explain in release notes etc, so I really need to understand things a little better before I merge again.

The main thing i'm unsure about is why the switch to Django's get_template and select_template is necessary. Looking at the Django docs for these methods, there's a bit at the end that explains:

If you’re loading a template while you’re rendering another template with the Django template language and you have access to the current context, for instance in the render() method of a template tag, you can use the current Engine directly. Instead of using get_template (example given), you can write:

template = context.template.engine.get_template('included.html')

This will load the template with the current engine without triggering the multiple template engines machinery, which is usually the desired behavior. Unlike previous solutions, this returns a django.template.Template, like get_template() used to in Django 1.7 and earlier, avoiding all backwards-compatibility problems.

I took this to mean that if the Jinja template engine (or any other engine) is being used to render a context, then context.template.engine would give us the same engine as that already being used, which would be more efficient than triggering the 'multiple template engine machinery'. Is this not the case?

Or, is the problem not that the right engine is used to find the template, but that the django.template.Template object returned can't be used for rendering?

If anyone can help explain, that would be much appreciated :) Thanks.

[hongquan]

No, the context.template.engine is only available if the current-used engine is Django Template engine. The context.template in this case is an instance of django.template.base.Template, which has engine attribute.

But if you are using Jinja2 template, the context is of jinja2.runtime.Context class, which has no template attribute and hence, no engine info.

That is why I have to use get_template, select_template.

[ababic]

Okay, thanks for confirming @hongquan. In this case then, I think I need to do a little more work to make the changes backwards compatible. I'll remerge your work again as it is, and work on the backwards compatibility changes as a new PR.

[jorenham]

When I use main_menu with custom menu templates

{{ main_menu(template='pages/menus/main_menu.html', sub_menu_template='pages/menus/sub_menu.html') }}

with the jinja2 extension @hongquan posted, I get the error

  File "project/pages/jinja2/pages/base.html", line 62, in block "header"
    {{ main_menu(template='') }}
  File "project/lib/python3.5/site-packages/wagtailmenus/templatetags/menu_tags.py", line 39, in main_menu
    **kwargs
  File "project/lib/python3.5/site-packages/wagtailmenus/models/menus.py", line 108, in render_from_tag
    return instance.render_to_template()
  File "project/lib/python3.5/site-packages/wagtailmenus/models/menus.py", line 203, in render_to_template
    context_data = self.get_context_data()
  File "project/lib/python3.5/site-packages/wagtailmenus/models/mixins.py", line 63, in get_context_data
    t = self.sub_menu_template
  File "project/lib/python3.5/site-packages/django/utils/functional.py", line 35, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
  File "project/lib/python3.5/site-packages/wagtailmenus/models/mixins.py", line 29, in sub_menu_template
    return self.get_sub_menu_template()
  File "project/lib/python3.5/site-packages/wagtailmenus/models/mixins.py", line 18, in get_sub_menu_template
    engine = self.get_template_engine()
  File "project/lib/python3.5/site-packages/wagtailmenus/models/menus.py", line 553, in get_template_engine
    return self._contextual_vals.parent_context.template.engine
AttributeError: 'Context' object has no attribute 'template'

I see that this is because of function call that is deprecated and will be removed in 2.8, but how can I work around this for now?

wagtailmenus 2.6.0 wagtail 1.2.3 django 1.11 jinja 2.10

[ababic]

Hi @jorenham,

The release notes should tell you everything you need to know (http://wagtailmenus.readthedocs.io/en/stable/releases/2.6.0.html). It sounds like you just need to add WAGTAILMENUS_USE_BACKEND_SPECIFIC_TEMPLATES = True to your project settings.

[jorenham]

@ababic Yup, that was it. Thanx!

[jorenham]

The problem of the solution of @hongquan is that jinja2.contextfunction(sub_menu) gets cached, i.e. the first time jinja2.contextfunction(sub_menu) returns the function, the second time the result of the last function call.

#main_menu.html
{% for item in menu_items %}
    <a href="{{ item.href }}">{{ item.text }}</a><br>
    {% if item.has_children_in_menu %}
        {# evaluates to `true` the first time only #}
        {%  if sub_menu.__class__.__name__ == 'function' %}
            {{ sub_menu(item, template='pages/menus/sub_menu.html') | safe }}
        {% else %}
            :(<br>{{ sub_menu.render_to_template()|safe }}
        {% endif %}
    {% endif %}
{% endfor %}

:confused:

[hongquan]

Thank for reporting. I will try to fix.

[jorenham]

Any progress?

[hongquan]

Sorry, these days I am occupied by some other projects, not coming back to work on Wagtail yet.

[ababic]

@jorenham / @hongquan I'm hoping the changes I'm currently working on in #279 will allow this problem to be circumvented, by removing the need to use the {% sub_menu %} tag all together.

[ababic]

If anyone is still struggling to use the {% sub_menu %} tag with Jinja2, the functionality referenced above has now been released. You can find more details in the release notes at the following URL: https://wagtailmenus.readthedocs.io/en/stable/releases/2.12.html#new-add-sub-menus-inline-option-for-menu-tags

[xdml]

Anyone working on this?

For me it looks, like multiple {{ flat_menu(...) }} is not about how Jinja2 caches results, but how wagtailmenus uses context.

In Django these template tags are really tags, but in Jinja2 they are functions(), so they lives in context. But If I look at wagtailmenus/models/menus.py I can see code like:

class SectionMenu(DefinesSubMenuTemplatesMixin, MenuFromPage):
    menu_short_name = 'section'  # used to find templates
    menu_instance_context_name = 'section_menu'
    related_templatetag_name = 'section_menu'
    ...

class ChildrenMenu(DefinesSubMenuTemplatesMixin, MenuFromPage):
    menu_short_name = 'children'  # used to find templates
    menu_instance_context_name = 'children_menu'
    related_templatetag_name = 'children_menu'
    ...

class SubMenu(MenuFromPage):
    menu_short_name = 'sub'  # used to find templates
    menu_instance_context_name = 'sub_menu'
    related_templatetag_name = 'sub_menu'
    ...

class AbstractMainMenu(DefinesSubMenuTemplatesMixin, MenuWithMenuItems):
    menu_short_name = 'main'  # used to find templates
    menu_instance_context_name = 'main_menu'
    related_templatetag_name = 'main_menu'
    content_panels = panels.main_menu_content_panels
    menu_items_relation_setting_name = 'MAIN_MENU_ITEMS_RELATED_NAME'
    ...

class AbstractFlatMenu(DefinesSubMenuTemplatesMixin, MenuWithMenuItems):
    menu_short_name = 'flat'  # used to find templates
    menu_instance_context_name = 'flat_menu'
    related_templatetag_name = 'flat_menu'
    base_form_class = forms.FlatMenuAdminForm
    content_panels = panels.flat_menu_content_panels
    menu_items_relation_setting_name = 'FLAT_MENU_ITEMS_RELATED_NAME'
    ...

In my opinion, in Django it is fine, tag {% main_menu %} is different to instance {{ main_menu }}, but in Jinja2 function {{ main_menu() }} is redefined to instance {{ main_menu }} and later call in the same context results into error.

Please can anyone else double check my idea? Thank you.

So I see two possible solutions:

1. Implement real Jinja2 tags {% main_menu %}, etc.
2. or (my preference), rename instances to something else (e.g. main_menu_instance) and use function {{ main_menu() }}, etc.

As a workaround I use this code:

import jinja2
from jinja2.ext import Extension

from wagtailmenus.templatetags.menu_tags import (
        main_menu,
        flat_menu,
        section_menu,
        children_menu,
        sub_menu,
    )

class WagtailmenusExtension(Extension):
    def __init__(self, environment):
        super().__init__(environment)

        self.environment.globals.update({
                'main_menu_jinja2': jinja2.pass_context(main_menu),
                'flat_menu_jinja2': jinja2.pass_context(flat_menu),
                'section_menu_jinja2': jinja2.pass_context(section_menu),
                'children_menu_jinja2': jinja2.pass_context(children_menu),
                'sub_menu_jinja2': jinja2.pass_context(sub_menu),
            })

# Nicer import names
menu_tags = WagtailmenusExtension

and in my templates I use {{ main_menu_jinja2() }} instead of {{ main_menu() }}.

[xdml]

Looks like Jinja2 uses context differently. I realized, that in Jinja2 sub_menu some time works, some time not. After small diving in, I realized, that in wagtailmenus/models/menus.py _create_contextualvals_obj_from_context(cls, context) there is context.get('current_level', 0) + 1.

But this in Jinja2 never get reset to 1, so if I have on single page more {{ flat_menu() }} and {{ main_menu() }} with sub_menus, rendering of sub_menus depends on the order, as current_level starts from 1 only for first menu, but for any other it continues increasing the previous value.

Hope this report make sense.

[vikingmoose]

I feel like this should be explained in detail in the docs. This issue is pretty much the only place jinja2 is mentioned - apart from the release notes that says it's supported.

[vikingmoose]

Got wagtailmenus 3.1.9 working with jinja2 by doing as described here. https://github.com/jazzband/wagtailmenus/issues/89#issuecomment-343105489

Everything works nicely, however the active_class is always empty.

<!-- settings.py -->
WAGTAILMENUS_USE_BACKEND_SPECIFIC_TEMPLATES = True
WAGTAILMENUS_ACTIVE_CLASS = "test"
WAGTAILMENUS_DEFAULT_MAIN_MENU_TEMPLATE = "nav/main_menu.html"

<!--nav/main_menu.html-->
{% for item in menu_items %}
<a href="{{ item.href }}" class="......{{ item.active_class }}">{{ item.text }}</a>
{% endfor %}

<!-- _base.html -->
{{ main_menu() | safe }}

Anything I may have missed?