Added basic page range navigation. #2579
Conversation
|
Ah, cool. I'll try that. |
|
Works great! (Except that there are no more ellipses, which can be annoying if you have a large amount of pages in your index.) |
|
Why not? You could just have an ellipsis that links to |
…set surrounding size).
|
Now the ellipses are back, and the surrounding size can be modified easier (and is by default 5 instead of 3). |
| <a href="${page_links[i]}">${i+1}</a> | ||
| % endif | ||
| % elif i == current_page - surrounding - 1 or i == current_page + surrounding + 1: | ||
| <span class="ellipsis">⋯</span> |
| @@ -2374,11 +2376,16 @@ def generic_index_renderer(self, lang, posts, indexes_title, template_name, cont | |||
| lists.append(posts[:kw["index_display_post_count"]]) | |||
| posts = posts[kw["index_display_post_count"]:] | |||
| num_pages = len(lists) | |||
| displayed_page_numbers = [utils.get_displayed_page_number(i, num_pages, self) for i in range(max(num_pages, 1))] | |||
There was a problem hiding this comment.
For a simpler comparison: num_pages or 1
| @@ -2374,11 +2376,16 @@ def generic_index_renderer(self, lang, posts, indexes_title, template_name, cont | |||
| lists.append(posts[:kw["index_display_post_count"]]) | |||
| posts = posts[kw["index_display_post_count"]:] | |||
| num_pages = len(lists) | |||
| displayed_page_numbers = [utils.get_displayed_page_number(i, num_pages, self) for i in range(max(num_pages, 1))] | |||
| page_links = [page_link(i, displayed_page_numbers[i], num_pages, False) for i in range(max(num_pages, 1))] | |||
There was a problem hiding this comment.
page_links = [page_link(i, page_number, num_pages, False) for i, page_number in enumerate(displayed_page_numbers)]| displayed_page_numbers = [utils.get_displayed_page_number(i, num_pages, self) for i in range(max(num_pages, 1))] | ||
| page_links = [page_link(i, displayed_page_numbers[i], num_pages, False) for i in range(max(num_pages, 1))] | ||
| if kw['show_index_page_navigation']: | ||
| temp_map = {page_number - 1: link for page_number, link in zip(displayed_page_numbers, page_links)} |
There was a problem hiding this comment.
Why is this a dict? I’d expect the lists to be sorted already, so this could be a list. In any case, avoid iterating over range(num_pages) more than once — not only you might have missed the case when num_pages = 0, but also [i[1] for i in sorted(temp_map.values())] has more relation to the dict.
There was a problem hiding this comment.
The list is not sorted already: in case INDEXES_STATIC == True, displayed_page_numbers will be [num_pages, 1, 2, 3, ..., num_pages - 1]. In case INDEXES_STATIC == False, it will be [1, 2, ..., num_pages] (see utils.get_displayed_page_number). To avoid having to duplicate that kind of strange logic in here, I'm using the map.
In case num_pages == 0, page_links_context should have length 0 (and not length 1 as displayed_page_numbers and page_links). This isn't the case for [i[1] for i in sorted(temp_map.values())], which will have length 1.
Anyway, in case num_pages == 0, the whole thing is not working that well anyway. In case INDEXES_PRETTY_PAGE_URL is True, a redirect to a non-existing page will be created, and in any case, the main index page is not created. How about fixing that here as well?
There was a problem hiding this comment.
If you want to fix this, go ahead. As it stands, this code LGTM, although this comment should appear in the source code to explain the temp_map’s existence.
There was a problem hiding this comment.
I added the comment. I'll fix that maybe later in a different PR.
|
Thanks for merging :) |
Fixes #2299.
This PR provides a basic page range navigation as asked for in #2299 (I hope). Needs better styling (both CSS and template).