Opened 7 years ago

Closed 8 weeks 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: master
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 6 years ago.
12694.2.diff (2.0 KB) - added by Simon Meers 6 years ago.
change_form.html *and* change_list.html

Download all attachments as: .zip

Change History (15)

comment:1 Changed 7 years ago by Jannis Leidel

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

Changed 6 years ago by Simon Meers

Attachment: 12694.diff added

comment:2 Changed 6 years ago by Simon Meers

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...

Changed 6 years ago by Simon Meers

Attachment: 12694.2.diff added

change_form.html *and* change_list.html

comment:3 Changed 6 years ago by Simon Meers

Keywords: sprint_dec_2010 added

comment:4 Changed 6 years ago by Simon Meers

Keywords: sprintdec2010 added; sprint_dec_2010 removed

comment:5 Changed 6 years ago by Julien Phalip

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 Changed 6 years ago by Russell Keith-Magee

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 Changed 6 years ago by Adam Vandenberg

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.

comment:8 in reply to:  7 Changed 6 years ago by Simon Meers

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 Changed 6 years ago by Russell Keith-Magee

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 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

comment:11 Changed 8 weeks ago by Sassan Haradji

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 Changed 8 weeks ago by Sassan Haradji

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

comment:13 Changed 8 weeks ago by Tim Graham

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