Opened 15 years ago

Closed 11 years ago

#13629 closed New feature (fixed)

Admin Changelist: add app-model_name class to <body> tag

Reported by: Łukasz Korzybski Owned by: tcsorrel
Component: contrib.admin Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: yes

Description (last modified by Russell Keith-Magee)

Change form pages have "app-model_name" classes in body tag, for eg:
<body class="main-client change-form">. Change list pages don't, which
makes them unrecognizable in JavaScript code.

Example use case:
I'm currently implementing jQuery tooltips for searchbars to give user
information about which model's fields are searchable in each change list.

Currently to achieve this I had to override ModelAdmin.changelist_view (to
add model._meta to context) and change_list.html template. This is ok but I
think it would be nice to have it already done in django?

Attachments (2)

0001-Closes-13629-add-app-model_name-class-to-body-tag.patch​ (1.5 KB ) - added by Kenneth Belitzky 14 years ago.
more-classes-in-admin.patch​ (3.3 KB ) - added by patrick 13 years ago.

Download all attachments as: .zip

Change History (25)

comment:1 by Russell Keith-Magee, 14 years ago

Description: modified (diff)
Triage Stage: Unreviewed → Accepted

by Kenneth Belitzky, 14 years ago

Attachment: 0001-Closes-13629-add-app-model_name-class-to-body-tag.patch​ added

comment:2 by Kenneth Belitzky, 14 years ago

Has patch: set
Needs tests: set

comment:3 by Julien Phalip, 14 years ago

Severity: → Normal
Type: → New feature

comment:4 by Julien Phalip, 14 years ago

Easy pickings: set
Patch needs improvement: set

This is a good idea but while we're at it let's do it in every admin template, not just the change list. Also, using opts.module_name is preferable over opts.object_name.lower.

comment:5 by Julien Phalip, 14 years ago

Scrap my comment about opts.object_name.lower, which is fine to use here. I'd still like to see this new feature applied to all relevant admin templates though.

comment:6 by patrick, 13 years ago

Owner: changed from nobody to patrick
UI/UX: unset

comment:7 by patrick, 13 years ago

Patch needs improvement: unset
Resolution: → fixed
Status: new → closed
Version: 1.2 → 1.3

I've updated the code to work with django 1.3. Also I've added more classes.

Class added:

  • In home: dashboard home
  • In application model list: dashboard app_label
  • In change form: app_label-modelname change-form
  • In change list: app_label-modelname change-list
  • In delete confirmation: app_label-modelname delete-confirmation
  • In delete selected confirmation: app_label-modelname delete-confirmation delete-selected-confirmation

by patrick, 13 years ago

Attachment: more-classes-in-admin.patch​ added

comment:8 by Julien Phalip, 13 years ago

Needs documentation: set
Resolution: fixed
Status: closed → reopened
UI/UX: set

This isn't fixed until it has been checked in ;)

comment:9 by patrick, 13 years ago

Owner: patrick removed
Status: reopened → new

comment:10 by phillip_c_chin@…, 12 years ago

Needs tests: unset

I applied the more-classes-in-admin.patch patch and added a unit test to regressiontests/admin_views/test.py called testAppModelNameInClassForBodyTag(). I submitted a pull request for this.

comment:11 by anonymous, 12 years ago

The pull request is here: ​https://github.com/django/django/pull/158

comment:12 by Julien Phalip, 12 years ago

Patch needs improvement: set

Thanks for your work. I've left some specific comments on the pull request. Could you take a look and update it? Thanks!

comment:13 by tcsorrel, 12 years ago

Owner: set to tcsorrel

comment:14 by tcsorrel, 12 years ago

Status: new → assigned

New pull request can be found here : ​https://github.com/django/django/pull/575

comment:15 by tcsorrel, 11 years ago

Patch needs improvement: unset

Patch is updated with last test done in the same pull request

comment:16 by tcsorrel, 11 years ago

Needs documentation: unset

I don't know how and where this ticket should be documented

comment:17 by Julien Phalip, 11 years ago

Patch needs improvement: set

Thanks a lot for the updated pull request!

I think the patch looks pretty good. I just have one main comment: I'm not sure Selenium is really needed for the last test. Selenium tests are usually quite slow to run, so if there's a way to use the dummy client instead, then that would be preferable. See maybe if you can find similar examples in admin_views.tests.AdminActionsTest.

Thanks again!

comment:18 by tcsorrel, 11 years ago

Thanks for the advice on sending post request more simply.
I changed the test client. And took also the opportunity to

  • remove prefix redundant with assertContains message
  • limit the test to check the body class app and model tag

comment:19 by tcsorrel, 11 years ago

Patch needs improvement: unset

comment:20 by Julien Phalip, 11 years ago

Thanks a lot Thomas! Could you please update the pull request? I'll review your changes as soon as I can. Thanks!

comment:21 by Julien Phalip, 11 years ago

Thank you for updating the pull request. I think everything looks great. I just have 2 minor comments before merging your work. Could you please move the tests to the CSSTest class inside admin_views/tests.py, and could you make sure there are no trailing white spaces in the patch? Thank you!

comment:22 by tcsorrel, 11 years ago

Version: 1.3 → master

Tests are moved CSSTest.
Trailing spaces are removed.
The ticket changes is rebased on master inside one commit.
Tests are passed.

comment:23 by Julien Phalip <jphalip@…>, 11 years ago

Resolution: → fixed
Status: assigned → closed

In bb145e2c47d71b7f68280c00ced727442d2effa2:

Fixed #13629 -- Added CSS classes to the <body> tag of some admin templates to allow style customizations per app or per model.

Note: See TracTickets for help on using tickets.
Back to Top