Opened 14 years ago

Closed 8 years ago

#12694 closed Cleanup/optimization (fixed)

Adding an object-tool requires template code duplication

Reported by: Andy Baker Owned by: Simon Meers
Component: contrib.admin Version: dev
Severity: Normal Keywords: admin, blocks, template, object-tools, sprintdec2010
Cc: andy@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently adding an object tool to an admin change list template requires that you duplicate the following:

    {% block object-tools %}
      {% if has_add_permission %}
        <ul class="object-tools">
          <li>
            <a href="add/{% if is_popup %}?_popup=1{% endif %}" class="addlink">
              {% blocktrans with cl.opts.verbose_name as name %}Add {{ name }}{% endblocktrans %}
            </a>
          </li>
        </ul>
      {% endif %}
    {% endblock %}

It's much more likely that someone will want to merely add an object tool rather than replace the existing ones. Therefore an empty block before the first <li> would allow for cleaner template inheritance and reduce problems when upgrading Django installs. My install of Django is patched as follows:

    {% block object-tools %}
        <ul class="object-tools">
          {% block pre-object-tools %}{% endblock %}
            {% if has_add_permission %}
              <li>
                <a href="add/{% if is_popup %}?_popup=1{% endif %}" class="addlink">
                  {% blocktrans with cl.opts.verbose_name as name %}Add {{ name }}{% endblocktrans %}
                </a>
              </li>
          {% endif %}
        {% block post-object-tools %}{% endblock %}
        </ul>
    {% endblock %}

Attachments (2)

12694.diff (1.0 KB ) - added by Simon Meers 13 years ago.
12694.2.diff (2.0 KB ) - added by Simon Meers 13 years ago.
change_form.html *and* change_list.html

Download all attachments as: .zip

Change History (15)

comment:1 by Jannis Leidel, 14 years ago

Triage Stage: UnreviewedAccepted

by Simon Meers, 13 years ago

Attachment: 12694.diff added

comment:2 by Simon Meers, 13 years ago

Has patch: set
milestone: 1.3
Owner: changed from nobody to Simon Meers
Status: newassigned
Version: 1.1SVN

This has frustrated me for years also. I've regularly used jQuery to insert list items within the object tools list to stay DRY...

Very simple patch attached. I'd prefer to see this single inner-block rather than "pre-" and "post-" blocks because:

  • It is simpler
  • You can achieve the same by using {% block object-tools-items %} BEFORE {{ block.super }} AFTER {% endblock %}
  • pre-object-tools sounds like it comes before the object-tools block, despite being inside it...

by Simon Meers, 13 years ago

Attachment: 12694.2.diff added

change_form.html *and* change_list.html

comment:3 by Simon Meers, 13 years ago

Keywords: sprint_dec_2010 added

comment:4 by Simon Meers, 13 years ago

Keywords: sprintdec2010 added; sprint_dec_2010 removed

comment:5 by Julien Phalip, 13 years ago

Triage Stage: AcceptedReady for checkin

This has also frustrated me for years. DrMeers' approach seems much simpler and more flexible than the originally proposed patch.

comment:6 by Russell Keith-Magee, 13 years ago

Resolution: fixed
Status: assignedclosed

(In [14795]) Fixed #12694 -- Added template block to make it easier to add new tools to the change_list and change_form templates. Thanks to andybak for the suggestion, and Simon Meers for the final patch.

comment:7 by Adam Vandenberg, 13 years ago

On change_list, I'm not sure that this block should be within the "user has add permission" if block, since you may want to add tools/links that don't require the add permission.

in reply to:  7 comment:8 by Simon Meers, 13 years ago

Replying to adamv:

On change_list, I'm not sure that this block should be within the "user has add permission" if block, since you may want to add tools/links that don't require the add permission.

I agree, however we've been unable to come up with a solution to allow this that won't result in an (invalid) empty <ul> if the user does not have add permission and no additional tools are added. Suggestions most welcome.

comment:9 by Russell Keith-Magee, 13 years ago

One suggestion that just came up at the sprints: Change the UL to be a DIV. DIVs are allowed to have no child elements, so the empty case ceases to be a problem.

Of course, this means a bunch of changes to templates that could have all sorts of follow-on consequences.

comment:10 by Jacob, 12 years ago

milestone: 1.3

Milestone 1.3 deleted

comment:11 by Sassan Haradji, 8 years ago

Easy pickings: unset
Resolution: fixed
Severity: Normal
Status: closednew
Type: Uncategorized
UI/UX: unset

I think it's better to make it possible to add buttons to object-tool by python code (in inherited admin class) instead of overriding the html file, extending the default and adding the button. Something like actions field in admin class but for object-tool.

comment:12 by Sassan Haradji, 8 years ago

Sorry, I guess I set incorrect fields for this ticket. I'm not familiar with Django tickets system.

comment:13 by Tim Graham, 8 years ago

Resolution: fixed
Status: newclosed
Type: UncategorizedCleanup/optimization

Please open a new ticket rather than reopening a ticket that's already released. Your request seems to be a duplicate of #18914.

Note: See TracTickets for help on using tickets.
Back to Top