#4805 closed (fixed)
Pass paginator object in generic view's context instead of several variables related to the paginator
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Keywords: | ||
Cc: | Triage Stage: | Design decision needed | |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | yes | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
Currently, there is only one generic view with pagination capabilities (object_list
), but it is being asked for in the date-based generic views (#2367). I propose that instead of passing 11 different context variables related to pagination, that we pass just the paginator object and let the templates use what they need.
This would provide the following benefits:
- Paginator methods would only be called when needed instead of called to fill the 11 different context variables that might not get used in the template anyway.
- We are not repeating ourselves in the generic views. Instead of 11 context variables for each of the generic views and documentation of each, we have just one variable and have a Paginator documentation page that the generic view documentation sections could link to.
- Future enhancements would not require context variable and documentation changes for each generic view. They keep passing the paginator object as a whole.
Attachments (3)
Change History (12)
comment:1 by , 17 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
comment:2 by , 17 years ago
comment:3 by , 17 years ago
#2576 which suggested introducing this was closed as wontfix but maybe I wasn't very clear about why it would be useful and I got a bit side-tracked with my base-0 or base-1 options.
comment:4 by , 17 years ago
Yes, I actually saw the related django-dev thread and your ticket (#2576). The passing of all these variables just bothers me to no end, it must be fixed!
In that thread, Adrian was a fan of the separate class idea, which is what #2576 was. I think that a separate class might be a little overkill, and favor just a single, page-aware paginator class. I was going to open a separate "paginator overhaul" ticket, but then your comment made me realize that this ticket would not be possible without some page-aware class since, for the templates, we need methods that don't have to take parameters. So I guess will just attach the code to this ticket.
We should really start a new django-dev thread to discuss all this.
by , 17 years ago
comment:5 by , 17 years ago
I'm not sure if we would want to create a new Paginator class or change the existing ObjectPaginator class. The patch above I think is only backwards incompatible with a few of the functions that I changed to be 1-based. I think 1-based page numbers are more natural, and is the way it is currently coming from the URL. So we need some design decisions here.
The patch allows page to be set in the constructor, but it can also be set after the instance is created. Things like num_per_page
and the object sequence can also be changed on the fly. Once a page is set you can call methods such as .next_page()
or .first_on_page()
without passing a page number, perfect for use in templates.
comment:6 by , 17 years ago
Has patch: | set |
---|---|
Patch needs improvement: | set |
comment:7 by , 17 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
comment:8 by , 17 years ago
by , 17 years ago
Attachment: | 4805-alt.2.diff added |
---|
Alternate implementation based on #2576... that isn't completely broken like the last one I added
comment:9 by , 17 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I've thought about this before too, and I'm +1.
It would also make it easier to add pagination to other generic views (#2367), and custom pagination template tags wouldn't depend on those 11 context variables. It would be a lot cleaner to just add the paginator object to the template context in a custom view, than to have to add the 11 variables.