django-polymorphic / django-polymorphic-tree

Polymorphic MPTT tree support for models
Other
171 stars 43 forks source link

django-grappelli support #4

Open papalagichen opened 12 years ago

papalagichen commented 12 years ago

django-grappelli is a very popular theme for django admin. https://github.com/sehmaschine/django-grappelli

When django-polymorphic-tree works with django-grappelli, the templates, breadcrumbs and actions for models seem to be broken. Is it possible for django-polymorphic-tree to support django-grappelli in the future release?

vdboor commented 12 years ago

I'll see what I can do, in the next week. Since we override the breadcrumbs, it may be a matter of just adding some CSS classes to the templates.

If you're able to do this already in the meantime, I'd be happy to accept the pull request of course.

vdboor commented 12 years ago

Hi, Ive addressed the list appearance in a5c76f639fcb82cefcb5164ab3bac730eb8ff4a6 it should look proper now.

The breadcrumbs are still off, as django-grapelli uses different HTML formatting to format them. I'm not sure yet how I'll implement that. Either it could be a separate template, or a {% if has_grapelli %}` statement in the template. Will be continued :)

DylanLukes commented 9 years ago

Seems like this issue has languished for a while. I'd like to re-open it and get this problem solved for the breadcrumbs. For comparison, let's look at the Change Form templates. Here is the current template in django-polymorphic-tree:

{% extends "admin/change_form.html" %}
{% load i18n admin_modify polymorphic_admin_tags polymorphic_tree_admin_tags %}

{# Add tree levels to polymorphic breadcrumb #}
{% block breadcrumbs %}{% if not is_popup %}{% breadcrumb_scope base_opts %}
<div class="breadcrumbs">
     <a href="../../../">{% trans "Home" %}</a> &rsaquo;
     <a href="../../">{{ app_label|capfirst|escape }}</a> &rsaquo;
     {% if has_change_permission %}<a href="../">{{ opts.verbose_name_plural|capfirst }}</a>{% else %}{{ opts.verbose_name_plural|capfirst }}{% endif %} &rsaquo;
     {% for p in original|mptt_breadcrumb %}
       <a href="../{{ p.id }}/">{{ p }}</a> &rsaquo;
     {% endfor %}
     {% if add %}{% trans "Add" %} {{ opts.verbose_name }}{% else %}{{ original|truncatewords:"18" }}{% endif %}
</div>
{% endbreadcrumb_scope %}{% endif %}{% endblock %}

Here is the relevant block from Grappelli's admin/base.html and admin/change_form.html:

{% block context-navigation %}
    <!-- CONTEXT NAVIGATION -->
    <div id="grp-context-navigation">
        <nav id="grp-breadcrumbs" class="{% block breadcrumbs-class %}{% endblock %}">
            <header style="display:none"><h1>Breadcrumbs</h1></header>
            {% block breadcrumbs %}
                <ul>
                    <li><a href="{% url 'admin:index' %}">{% trans 'Home' %}</a></li>
                    {% if title %}
                        <li> &rsaquo; {{ title }}</li>
                    {% endif %}
                </ul>
            {% endblock %}
        </nav>
        <nav id="grp-page-tools">
            <header style="display:none"><h1>Page Tools</h1></header>
            {% block page-tools %}{% endblock %}
        </nav>
    </div>
{% endblock %}
<!-- BREADCRUMBS -->
{% block breadcrumbs %}
    {% if not is_popup %}
        <ul>
            <li><a href="{% url 'admin:index' %}">{% trans "Home" %}</a></li>
            <li><a href="{% url 'admin:app_list' app_label=opts.app_label %}">{{ opts.app_config.verbose_name }}</a></li>
            <li>{% if has_change_permission %}<a href="{% url opts|admin_urlname:'changelist' %}">{{ opts.verbose_name_plural|capfirst }}</a>{% else %}{{ opts.verbose_name_plural|capfirst }}{% endif %}</li>
            <li>{% if add %}{% trans "Add" %} {{ opts.verbose_name }}{% else %}{{ original|truncatewords:"18" }}{% endif %}</li>
        </ul>
    {% endif %}
{% endblock %}

And finally, the standard Django admin's breadcrumbs from admin/base.html and admin/change_form.html respectively:

{% block breadcrumbs %}
<div class="breadcrumbs">
  <a href="{% url 'admin:index' %}">{% trans 'Home' %}</a>
  {% if title %} &rsaquo; {{ title }}{% endif %}
</div>
{% endblock %}
{% block breadcrumbs %}
<div class="breadcrumbs">
<a href="{% url 'admin:index' %}">{% trans 'Home' %}</a>
&rsaquo; <a href="{% url 'admin:app_list' app_label=opts.app_label %}">{{ opts.app_config.verbose_name }}</a>
&rsaquo; {% if has_change_permission %}<a href="{% url opts|admin_urlname:'changelist' %}">{{ opts.verbose_name_plural|capfirst }}</a>{% else %}{{ opts.verbose_name_plural|capfirst }}{% endif %}
&rsaquo; {% if add %}{% trans 'Add' %} {{ opts.verbose_name }}{% else %}{{ original|truncatewords:"18" }}{% endif %}
</div>
{% endblock %}

So, the changes in the HTML are relatively minor and can be implemented with an assignment tag that detects whether Grappelli is being used. Note there is a distinction between Grappelli appearing in INSTALLED_APPS and actually being used. The problem is Grappelli is actually very non-intrusive. It pretty much only overrides templates.

There is thus no way to detect Grappelli at runtime. Honestly, I don't think that's the right idea though.

Rather, the best option is to have a POLYMORPHIC_TREE_GRAPPELLI_COMPAT = True setting, which defaults to True when Grappelli is in INSTALLED_APPS.

Thoughts?

See: https://docs.djangoproject.com/en/1.7/howto/custom-template-tags/#assignment-tags

robslotboom commented 8 years ago

Have a look at django-mptt. It utilizes

IS_GRAPPELLI_INSTALLED = 'grappelli' in settings.INSTALLED_APPS

To load different templates.

vdboor commented 8 years ago

Agreed @robslotboom, I'd accept a pull request when it cleanly provides support for both! We already filter the CSS for the specific theme. (@DylanLukes, would you be able to provide this?)