Opened 6 years ago

Closed 5 years ago

Last modified 4 years ago

#12694 closed (fixed)

Adding an object-tool requires template code duplication

Reported by: andybak Owned by: DrMeers
Component: contrib.admin Version: master
Severity: 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: UI/UX:

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 DrMeers 5 years ago.
12694.2.diff (2.0 KB) - added by DrMeers 5 years ago.
change_form.html *and* change_list.html

Download all attachments as: .zip

Change History (12)

comment:1 Changed 5 years ago by jezdez

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Changed 5 years ago by DrMeers

comment:2 Changed 5 years ago by DrMeers

  • Has patch set
  • milestone set to 1.3
  • Owner changed from nobody to DrMeers
  • Status changed from new to assigned
  • Version changed from 1.1 to SVN

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

change_form.html *and* change_list.html

comment:3 Changed 5 years ago by DrMeers

  • Keywords sprint_dec_2010 added

comment:4 Changed 5 years ago by DrMeers

  • Keywords sprintdec2010 added; sprint_dec_2010 removed

comment:5 Changed 5 years ago by julien

  • Triage Stage changed from Accepted to Ready 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 5 years ago by russellm

  • Resolution set to fixed
  • Status changed from assigned to closed

(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 follow-up: Changed 5 years ago by 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.

comment:8 in reply to: ↑ 7 Changed 5 years ago by DrMeers

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

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 4 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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