Opened 6 years ago

Closed 3 years ago

#13629 closed New feature (fixed)

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

Reported by: naos Owned by: tcsorrel
Component: contrib.admin Version: master
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 russellm)

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 httpdss 6 years ago.
more-classes-in-admin.patch (3.3 KB) - added by patrick91 5 years ago.

Download all attachments as: .zip

Change History (25)

comment:1 Changed 6 years ago by russellm

  • Description modified (diff)
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 years ago by httpdss

  • Has patch set
  • Needs tests set

comment:3 Changed 5 years ago by julien

  • Severity set to Normal
  • Type set to New feature

comment:4 Changed 5 years ago by julien

  • 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 Changed 5 years ago by julien

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 Changed 5 years ago by patrick91

  • Owner changed from nobody to patrick91
  • UI/UX unset

comment:7 Changed 5 years ago by patrick91

  • Patch needs improvement unset
  • Resolution set to fixed
  • Status changed from new to closed
  • Version changed from 1.2 to 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

Changed 5 years ago by patrick91

comment:8 Changed 5 years ago by julien

  • Needs documentation set
  • Resolution fixed deleted
  • Status changed from closed to reopened
  • UI/UX set

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

comment:9 Changed 5 years ago by patrick91

  • Owner patrick91 deleted
  • Status changed from reopened to new

comment:10 Changed 4 years ago by phillip_c_chin@…

  • 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 Changed 4 years ago by anonymous

comment:12 Changed 4 years ago by julien

  • 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 Changed 3 years ago by tcsorrel

  • Owner set to tcsorrel

comment:14 Changed 3 years ago by tcsorrel

  • Status changed from new to assigned

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

comment:15 Changed 3 years ago by tcsorrel

  • Patch needs improvement unset

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

comment:16 Changed 3 years ago by tcsorrel

  • Needs documentation unset

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

comment:17 Changed 3 years ago by julien

  • 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 Changed 3 years ago by tcsorrel

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 Changed 3 years ago by tcsorrel

  • Patch needs improvement unset

comment:20 Changed 3 years ago by julien

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

comment:21 Changed 3 years ago by julien

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 Changed 3 years ago by tcsorrel

  • Version changed from 1.3 to master

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

comment:23 Changed 3 years ago by Julien Phalip <jphalip@…>

  • Resolution set to fixed
  • Status changed from assigned to 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