Opened 4 hours ago

Last modified 3 hours ago

#36175 assigned Cleanup/optimization

Implement consistent paginator across admin pages.

Reported by: Antoliny Owned by: Antoliny
Component: contrib.admin Version: 5.1
Severity: Normal Keywords: pagination
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Antoliny)

Current pagination implemented in the admin pages performs the same functionality but is classified differently.
(Due to this difference, "Show all" is provided in list_display but not in history_view.)
When applying pagination in the change_list page, it was implemented through template tags.

ChangeList

# change_list.html
...
{% block pagination %}{% pagination cl %}{% endblock %}
...

# templatetags/admin_list.py

def pagination(cl):
    """
    Generate the series of links to the pages in a paginated list.
    """
    pagination_required = (not cl.show_all or not cl.can_show_all) and cl.multi_page
    page_range = cl.paginator.get_elided_page_range(cl.page_num) if pagination_required else []
    need_show_all_link = cl.can_show_all and not cl.show_all and cl.multi_page
    return {
        'cl': cl,
        'pagination_required': pagination_required,
        'show_all_url': need_show_all_link and cl.get_query_string({ALL_VAR: ''}),
        'page_range': page_range,
        'ALL_VAR': ALL_VAR,
        '1': 1,
    }

HistoryView

# object_history.html 
...
    <p class="paginator">
      {% if pagination_required %}
        {% for i in page_range %}
          {% if i == action_list.paginator.ELLIPSIS %}
            {{ action_list.paginator.ELLIPSIS }}
          {% elif i == action_list.number %}
            <span class="this-page">{{ i }}</span>
          {% else %}
            <a href="?{{ page_var }}={{ i }}" {% if i == action_list.paginator.num_pages %} class="end" {% endif %}>{{ i }}</a>
          {% endif %}
        {% endfor %}
      {% endif %}
      {{ action_list.paginator.count }} {% if action_list.paginator.count == 1 %}{% translate "entry" %}{% else %}{% translate "entries" %}{% endif %}
    </p>
...

Of course, I believe implementing this through a template tag would be a better approach. However, the reason history view had to be implemented this way is that the pagination related template tags only work with ChangeList.

Looking towards the future, this structure will require rewriting duplicate logic like object_history.html again.
(+ I think it could be quite painful if we need to extend or customize it.)

I'm confident that it would be better to manage these common parts through a template tag in one place.

Change History (2)

comment:1 by Antoliny, 4 hours ago

Owner: set to Antoliny
Status: newassigned

comment:2 by Antoliny, 3 hours ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.
Back to Top