Updated new pagination feature. #523
Conversation
djangosnippets/templates/base.html
Outdated
| <li><a hx-get="{% url 'cab_top_authors' %}" hx-target="#base-container" hx-swap="innerHTML" hx-trigger="click">By author</a></li> | ||
| <li><a hx-get="{% url 'cab_language_list' %}" hx-target="#base-container" hx-swap="innerHTML" hx-trigger="click" >By language</a></li> | ||
| <li><a hx-get="{% url 'cab_top_tags' %}" hx-target="#base-container" hx-swap="innerHTML" hx-trigger="click">By tag</a></li> | ||
| <li><a hx-get="{% url 'cab_top_rated' %}" hx-target="#base-container" hx-swap="innerHTML" hx-trigger="click">Highest rated</a></li> | ||
| <li><a hx-get="{% url 'cab_top_bookmarked' %}" hx-target="#base-container" hx-swap="innerHTML" hx-trigger="click">Most bookmarked</a></li> | ||
| <li><a href="{% url 'cab_top_authors' %}">By author</a></li> | ||
| <li><a href="{% url 'cab_language_list' %}">By language</a></li> | ||
| <li><a href="{% url 'cab_top_tags' %}">By tag</a></li> | ||
| <li><a href="{% url 'cab_top_rated' %}">Highest rated</a></li> | ||
| <li><a href="{% url 'cab_top_bookmarked' %}">Most bookmarked</a></li> |
There was a problem hiding this comment.
I removed the use of htmx from the homepage.
I'm wondering if there was a specific reason for using htmx in this part.
Personally, I felt that this section didn’t need to be served via htmx or a partial template. 🧐
There was a problem hiding this comment.
If I recall it was going for avoiding a full page refresh when clicking on a header
cab/pagination.py
Outdated
| class Pagination: | ||
| def __init__( |
There was a problem hiding this comment.
I added pagination related features to the cab app.
That said, I believe introducing a base app for shared components like pagination would be a cleaner approach.
Please let me know if you're open to this direction — I’d be happy to move the related code (e.g. templatetags, templates ...) into the base app.
cab/pagination.py
Outdated
| def get_query_string(self, new_params=None): | ||
| if new_params is None: | ||
| new_params = {} | ||
| params = self.params.copy() | ||
| for param, value in new_params.items(): | ||
| if value is None: | ||
| if param in params: | ||
| del params[param] | ||
| else: | ||
| params[param] = value | ||
| return "?%s" % urlencode(sorted(params.items()), doseq=True) |
There was a problem hiding this comment.
This method is almost identical to the get_query_string method of Django’s ChangeList class.
I think it would be better to replace this with the querystring template tag that will be added in Django 6.0, especially considering that the table page is likely to use many query parameters in the future.
The current Django version is 3.2 — would it be acceptable to copy and use the querystring template tag from 6.0 as is for now?
|
I have a question that’s not directly related to this PR. |
Antoliny0919
force-pushed
the
new_pagination_feature
branch
2 times, most recently
from
a69d916 to
347a55f
Compare
Yes it is confusing and I also wondered what the cab directory was for. I think it had something to do with testing but a really old approach. If memory serves I removed code that running it , so we can definitely look to remove it Also cab is one of Django Reinharts songs :) |
|
The build is failing due to us using an older python version. Give me a day or so to get the CI/CD working well. Hasnt been loved for a while |
Oh, I knew where the name Django came from, but I had no idea about this part. |
Thank you @chriswedgwood . |
Antoliny0919
force-pushed
the
new_pagination_feature
branch
3 times, most recently
from
17f37ab to
0c79cc4
Compare
|
With the changes made in commit 0c79cc4, the search functionality is now working again. However, all tests passed previously, which suggests that there are no tests related to the search feature 😢 |
Antoliny0919
force-pushed
the
new_pagination_feature
branch
from
0c79cc4 to
b0e868a
Compare
| "base.tests", | ||
| "comments_spamfighter", | ||
| "cab", | ||
| "ratings", | ||
| "taggit", | ||
| "ratings.tests", |
There was a problem hiding this comment.
Since multiple tests apps cannot coexist, I made base.tests the main test app and moved the test models to base.tests.models (along with the fixture).
Antoliny0919
force-pushed
the
new_pagination_feature
branch
2 times, most recently
from
cdfecd0 to
9a2007c
Compare
Antoliny0919
force-pushed
the
new_pagination_feature
branch
from
9a2007c to
4e1a63e
Compare
|
@chriswedgwood It seems like the SCSS changes are not being reflected on the actual site. What could be the reason? 🧐 |
I added a new pagination system. The previous one only provided "Previous" and "Next" buttons, but now users can navigate more freely using numbered page buttons.