Draft: Updates to the previous releases page #100
Conversation
zerolab
changed the title
AWitcherONS
changed the title
|
|
||
| {% block header_area %} | ||
| <div class="common-header article-header {% if page.updates %}article-header--flush{% endif %}"> | ||
| <div class="ons-container"> | ||
| {% include "templates/components/navigation/breadcrumbs.html" %} |
There was a problem hiding this comment.
The breadcrumbs include needs to stay on the statistical article page
cms/settings/dev.py
Outdated
| @@ -99,3 +99,6 @@ | |||
| "0002_articleseriespage_listing_image_and_more", # Ignoring NOT NULL constraint on columns | |||
| ], | |||
| } | |||
|
|
|||
| # Override size of the pagination used on the previous_release page. | |||
| PREVIOUS_RELEASES_PER_PAGE = 3 | |||
There was a problem hiding this comment.
It is probably best to move this to local.py.example, and commented out too. This should be a developer choice
| Given a topic page exists with status published | ||
| And the topic page has a statistical article in a series with status published | ||
| When an external user navigates to statistical article in a series previous releases page | ||
| Then the external user should not see pagination |
There was a problem hiding this comment.
surely if you are on the previous releases page, the breadcrumb should end with the latest article in series, rather than not show at all. We are, after all, one level deeper
cms/settings/test.py
Outdated
| @@ -72,3 +72,6 @@ | |||
| SLACK_NOTIFICATIONS_WEBHOOK_URL = None | |||
|
|
|||
| ONS_API_BASE_URL = "dummy_url" | |||
|
|
|||
| # Override size of the pagination used on the previous_release page. | |||
| PREVIOUS_RELEASES_PER_PAGE = 3 | |||
There was a problem hiding this comment.
This is probably fine.
For future, Django's test suite has override_settings which helps with overriding any of its settings during tests, either at test class level or individual test function
| {% block ONS_pagination_v1 %} | ||
| {% from "components/pagination/_macro.njk" import onsPagination %} | ||
| {% if pages.paginator.num_pages > 1 %} | ||
| <div class="ons-pagination__position">Page {{ pages.number }} of {{pages.paginator.num_pages }}</div> | ||
|
|
||
| {{ | ||
| onsPagination({ | ||
| "hideRangeIndicator": true, | ||
| "currentPageNumber": pages.number , | ||
| "pages": ons_pagination_url_list | ||
| }) | ||
| }} | ||
| {% endif %} | ||
| {% endblock %} |
There was a problem hiding this comment.
| {% block ONS_pagination_v1 %} | |
| {% from "components/pagination/_macro.njk" import onsPagination %} | |
| {% if pages.paginator.num_pages > 1 %} | |
| <div class="ons-pagination__position">Page {{ pages.number }} of {{pages.paginator.num_pages }}</div> | |
| {{ | |
| onsPagination({ | |
| "hideRangeIndicator": true, | |
| "currentPageNumber": pages.number , | |
| "pages": ons_pagination_url_list | |
| }) | |
| }} | |
| {% endif %} | |
| {% endblock %} | |
| {% block pagination %} | |
| {% from "components/pagination/_macro.njk" import onsPagination %} | |
| {% if pages.paginator.num_pages > 1 %} | |
| <div class="ons-pagination__position">Page {{ pages.number }} of {{pages.paginator.num_pages }}</div> | |
| {# fmt:off #} | |
| {{ | |
| onsPagination({ | |
| "hideRangeIndicator": true, | |
| "currentPageNumber": pages.number , | |
| "pages": ons_pagination_url_list | |
| }) | |
| }} | |
| {# fmt:on #} | |
| {% endif %} | |
| {% endblock %} |
although I question the need to wrap this whole bit in {% block %} since we don't plan to derive the template
cms/articles/models.py
Outdated
|
|
||
| try: | ||
| pages = paginator.page(int(request.GET.get("page", 1))) | ||
| ons_pagination_url_list = [{"url": "?page=" + str(n + 1)} for n in range(paginator.num_pages)] |
There was a problem hiding this comment.
| ons_pagination_url_list = [{"url": "?page=" + str(n + 1)} for n in range(paginator.num_pages)] | |
| ons_pagination_url_list = [{"url": f"?page={n}"} for n in paginator.page_range] |
See https://docs.djangoproject.com/en/5.1/topics/pagination/
or, if you prefer using range, then
| ons_pagination_url_list = [{"url": "?page=" + str(n + 1)} for n in range(paginator.num_pages)] | |
| ons_pagination_url_list = [{"url": f"?page={n}"} for n in range(1, paginator.num_pages + 1)] |
as per https://docs.python.org/3.12/library/stdtypes.html#range
Kudos for the approach to satisfy the page urls list requirement for the pagination component
There was a problem hiding this comment.
I liked your first suggestion but it needed an "in"
There was a problem hiding this comment.
overzealous deletion of characters. edited
Pagination on Previous Release Page
Add the Pagination to the release Page
How to review
Content / functionality
Describe the steps required to test the changes (include screenshots if appropriate).
Follow-up Actions
Issues I have noticed
Previous Release page showing 11 entries expected 10
Previous Release Page Sorting correctly