Opened 17 years ago
Last modified 8 months ago
#6396 new Cleanup/optimization
Remove customization-unfriendly admin template tags
Reported by: | korpios | Owned by: | nobody |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | nfa-someday |
Cc: | kmike84@…, cmawebsite@…, Ülgen Sarıkavak | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
The admin interface makes use of various template tags that are opaque, and make customization difficult.
An example from admin/change_list.html
:
{% block result_list %}{% result_list cl %}{% endblock %}
This calls an inclusion_tag
called result_list
that is used exactly once in the admin (i.e., here); that inclusion_tag
drops in some extra context variables before rendering admin/change_list_results.html
. This doesn't work with the common [app_label]/[object_name]/template.html
magic, since inclusion_tag
sets the template names upon registration. If there's a good reason for using a template tag here, I'm not seeing it. :)
The attached patch pulls the template code from change_list_results.html
into change_list.html
, and updates the latter's context with the extra context variables from the former. I intend to start yanking out similar uses of one-off template tags as I come across them in the admin, so I'll be dropping updated patches on this ticket as I go along.
The rearranging of the functions result_headers
, etc., is to avoid some circular import nastiness; I dropped them after ChangeList
, which seems to make the most sense.
Attachments (3)
Change History (32)
by , 17 years ago
Attachment: | ticket_6396.patch added |
---|
comment:1 by , 17 years ago
comment:2 by , 17 years ago
Cc: | added |
---|
comment:3 by , 17 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
comment:4 by , 17 years ago
the patch seems to be outdated? django/contrib/admin/options.py doesn't seem to exist (revision 7135).
comment:6 by , 17 years ago
i have a model with DateField and TimeField, if i use the provided patch the 'Today | <little calendar pic>' and 'Now | <little clock pic>' are not present, while they seem to be present with not patched newforms-admin. am i overlooking something obvious?
follow-up: 8 comment:7 by , 17 years ago
i have another issue, when i try to submit an edited model (either using 'save' or 'save and add another' buttons) from admin interface i get an error, while if i use 'save and continue editing' it edits the model correctly without any errors.
the error is:
Page not found (404)
Request Method: GET
Request URL: <mysite>:<my port>/admin/todo/
The requested admin page does not exist.
You're seeing this error because you have DEBUG = True in your Django settings file. Change that to False, and Django will display a standard 404 page.
comment:8 by , 17 years ago
Replying to almir@kiberpipa.org:
i have another issue, when i try to submit an edited model (either using 'save' or 'save and add another' buttons) from admin interface i get an error, while if i use 'save and continue editing' it edits the model correctly without any errors.
the error is:
Page not found (404)
Request Method: GET
Request URL: <mysite>:<my port>/admin/todo/
The requested admin page does not exist.
You're seeing this error because you have DEBUG = True in your Django settings file. Change that to False, and Django will display a standard 404 page.
this error is due to django not putting 'task' (which happens to be my model) after the 'todo' (which happens to be app in which task model is saved)
comment:9 by , 17 years ago
i found that the 404 error is not specific to this patch, i encounter it everytime i 'by hand' enter the url in in my browser, for example if i access http://my_django_project/admin/todo/task the urls will be missing 'task/' and cause a 404 when clicking on any of my object instances.
comment:10 by , 16 years ago
Keywords: | nfa-someday added |
---|
comment:11 by , 16 years ago
Version: | newforms-admin → SVN |
---|
I recently ran into this problem, when I needed to customize the rows in a change-list.
There is a much more elegant solution that will require very few changes to the (now trunk) newforms-admin code: Define your own tag, which inserts your own version of change_list_results.html.
Attached is a patch which changes the admin.templatetags.admin_list.items_for_result function to return a dictionary, rather than a complete row with embedded html. With it is a patched version of change_list_results.html which uses the dictionary fields to move all tag rendering into the template. In all, only about 5 lines of code are changed.
The result is that it is now possible to create your own tag, and override change_list.html to use it.
Patch will follow.
by , 16 years ago
Attachment: | better_patch-6396.diff added |
---|
Much less intrusive patch to address this problem.
comment:12 by , 16 years ago
Keywords: | someday added; nfa-someday removed |
---|
comment:13 by , 16 years ago
Component: | Template system → Admin interface |
---|
comment:14 by , 16 years ago
Keywords: | nfa-someday added; someday removed |
---|
At this point don't change the keywords from nfa-someday to someday. That just adds additional confusion at this point and may make some tickets lost. :)
comment:15 by , 16 years ago
Noted. Thanks for the pointer.
In the course of using this, I found that it is almost a requirement to include a reference to the result object in the dictionary which the custom tag is using to render the rows. Otherwise, there's no real way to access the data upon which one is working in a custom tag, if, for instance, you need to make a decision based upon the value of a field in a row.
Sorry for the rapid update, but I have included that patch, replacing my previous one.
by , 16 years ago
Attachment: | admin_changelist-6396.diff added |
---|
Update of previous patch - adding result object to dictionary
comment:16 by , 16 years ago
Cc: | removed |
---|
comment:17 by , 16 years ago
FWIW, this result_list
templatetag is the biggest pain in batchadmin's side. These really do make customization more difficult.
comment:18 by , 16 years ago
I would argue that the template tag is not the problem. The real problem is that the row results themselves are not being rendered in a template at all - meaning that there is no way to tweak the result list row items. Moving the code out of the template tag, as the first patch does, does not solve the problem. It only moves it to a new location.
It is easy to see why a custom tag was chosen here: Because the change_list template becomes especially cumbersome without it. Using the tag has encapsulated a chunk of code.
By doing nothing other than moving the row rendering into the admin_list template tag itself, the problem is solved. It is trivial to create your own version of the admin_list template tag, and use it in your own overridden change_list template.
This is still the one and only patch I apply to my Django installations. It is also backward-compatible. I hope it gets some attention soon.
follow-up: 20 comment:19 by , 16 years ago
I absolutely support the patch by Martin Diers and would like to see it included. Still I also agree with korpios, that a [app_label]/[object_name]/template.html behaviore of all admin templates, thus eliminating the need for an custom template tag here, would be very desirable. Just wanted to bring up the topic again.
follow-up: 21 comment:20 by , 15 years ago
Replying to dvine:
I absolutely support the patch by Martin Diers and would like to see it included. Still I also agree with korpios, that a [app_label]/[object_name]/template.html behaviore of all admin templates, thus eliminating the need for an custom template tag here, would be very desirable. Just wanted to bring up the topic again.
Seconded. I too would like to see this added as it would make adding items (i.e. searchbar) to custom admin views much easier.
comment:21 by , 15 years ago
Replying to athena_rd:
Replying to dvine:
I absolutely support the patch by Martin Diers and would like to see it included. Still I also agree with korpios, that a [app_label]/[object_name]/template.html behaviore of all admin templates, thus eliminating the need for an custom template tag here, would be very desirable. Just wanted to bring up the topic again.
Seconded. I too would like to see this added as it would make adding items (i.e. searchbar) to custom admin views much easier.
Yea, it definitely would. It would be great to have such a feature implemented. And i think it would make more sense, at least from my point of view.
comment:24 by , 14 years ago
Type: | → Cleanup/optimization |
---|
comment:25 by , 13 years ago
Severity: | → Normal |
---|
comment:26 by , 13 years ago
Easy pickings: | unset |
---|---|
Triage Stage: | Design decision needed → Accepted |
UI/UX: | unset |
I think the situation has improved recently, much code has been split off into utility functions, as a meta-point cleaning up code like this is always of value.
comment:27 by , 13 years ago
Cc: | added |
---|
comment:30 by , 10 years ago
Summary: | [newforms-admin] Customization-unfriendly admin template tags should die → Remove customization-unfriendly admin template tags |
---|
(Aggressive titles don't help.)
comment:31 by , 10 years ago
Cc: | added |
---|
comment:32 by , 8 months ago
Cc: | added |
---|
Updated the patch to fix a circular import.