[Django] #24316: Admin changelist column CSS classes do not use short description, instead using str(list_display_function)
Django
using str(list_display_function)
-------------------------------+--------------------
Reporter: JordanBright | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
The column names in a changelist respect the short description but the CSS
classes do not. This is particularly bad if you use a lambda as the class
name becomes `column-< ;function <lambda> at ...` in the `th` and
`field-<function <lambda> at ...` in the `td`.
I think this is caused by two functions in
`django/contrib/admin/templatetags/admin_list.py`
`result_headers` uses `field_name` from `for i, field_name in
enumerate(cl.list_display)` when it should be using `text` (from `text,
attr = label_for_field(...)`) and doing something to make it a safe css
class name, I think
`items_for_result` also uses `field_name` from `for field_name in
cl.list_display` and it should probably just use whatever `result_headers`
should be using.
To reproduce simply create a lambda function and put it in a list display
then inspect the header and field classes in your browser. I'll attach a
test case that reproduces it shortly.
--
Ticket URL: <https://code.djangoproject.com/ticket/24316>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
using str(list_display_function)
Reporter: JordanBright | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by JordanBright):
* needs_docs: => 0
* component: Uncategorized => contrib.admin
* needs_tests: => 0
* needs_better_patch: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24316#comment:1>
Django
using str(list_display_function)
Reporter: JordanBright | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24316#comment:2>
Django
using str(list_display_function)
Reporter: JordanBright | Owner: lorenzo-pasa
Type: Bug | Status: assigned
Component: contrib.admin | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by lorenzo-pasa):
* status: new => assigned
* owner: nobody => lorenzo-pasa
--
Ticket URL: <https://code.djangoproject.com/ticket/24316#comment:3>
Django
using str(list_display_function)
Reporter: JordanBright | Owner:
Type: Bug | Status: new
Component: contrib.admin | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by lorenzo-pasa):
* status: assigned => new
* owner: lorenzo-pasa =>
--
Ticket URL: <https://code.djangoproject.com/ticket/24316#comment:4>
Django
using str(list_display_function)
Reporter: JordanBright | Owner:
Type: Bug | Status: new
Component: contrib.admin | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by argaen):
* Attachment "24316_test_case_1.diff" added.
New TestCase since previous one was passing 2 asserts that it shouldn't
Django
using str(list_display_function)
Reporter: JordanBright | Owner:
Type: Bug | Status: new
Component: contrib.admin | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
Comment:
I didn't check if this patch fully resolves the ticket, but here's an
initial [https://github.com/django/django/pull/5885 pull request].
I came across this while debugging #23285. A test would occasionally fail
because it was checking the ordering of strings like "2000" and "2008" in
the response and sometimes they would appear elsewhere in the response
such as `<td class="field-<function callable_year at 0x7fe02000f230>">`.
--
Ticket URL: <https://code.djangoproject.com/ticket/24316#comment:5>
Django
using str(list_display_function)
Reporter: JordanBright | Owner:
Type: Bug | Status: new
Component: contrib.admin | Version: 1.7
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I improved the patch to address both cases mention in the ticket
description. I chose to use `callable.__name__` instead of
`short_description` as I didn't see why the `short_description` should be
considered for a CSS class. In particular, I expect it would be more
common to change the `short_description` than the callable's name, and I
think less the CSS class changes, the better. Let me know if I missed
something.
--
Ticket URL: <https://code.djangoproject.com/ticket/24316#comment:6>
Django
using str(list_display_function)
Reporter: JordanBright | Owner:
Type: Bug | Status: new
Component: contrib.admin | Version: 1.7
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by berkerpeksag):
* stage: Accepted => Ready for checkin
Comment:
PR https://github.com/django/django/pull/5885 looks good to me.
--
Ticket URL: <https://code.djangoproject.com/ticket/24316#comment:7>
Django
using str(list_display_function)
Reporter: JordanBright | Owner: Tim
| Graham <timograham@…>
Type: Bug | Status: closed
Component: contrib.admin | Version: 1.7
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* owner: => Tim Graham <timograham@…>
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"37f7ef41fbaebd5f545b99a11ab6036d92f9232b" 37f7ef41]:
{{{
#!CommitTicketReference repository=""
revision="37f7ef41fbaebd5f545b99a11ab6036d92f9232b"
Fixed #24316 -- Made ModelAdmin.list_display callables use an appropriate
CSS class name.
Thanks Berker Peksag for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24316#comment:8>