Opened 6 days ago
Last modified 17 hours ago
#36175 assigned Cleanup/optimization
Implement consistent pagination across admin pages.
Reported by: | Antoliny | Owned by: | Antoliny |
---|---|---|---|
Component: | contrib.admin | Version: | 5.1 |
Severity: | Normal | Keywords: | pagination |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Pull Requests: | 19172 build:failure | ||
Description (last modified by ) ¶
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.
According to the ticket's flags, the next step(s) to move this issue forward are:
- For anyone except the patch author to review the patch using the patch review checklist and either mark the ticket as "Ready for checkin" if everything looks good, or leave comments for improvement and mark the ticket as "Patch needs improvement".
Change History (6)
comment:1 by , 6 days ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:2 by , 6 days ago
Description: | modified (diff) |
---|
comment:3 by , 4 days ago
Summary: | Implement consistent paginator across admin pages. → Implement consistent pagination across admin pages. |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:4 by , 19 hours ago
Has patch: | set |
---|
comment:6 by , 17 hours ago
Needs tests: | unset |
---|---|
Patch needs improvement: | unset |
I will close #36174 as a duplicate of this ticket and accept this one, thank you