Skip to content

Conversation

ekkus93
Copy link

@ekkus93 ekkus93 commented Jun 17, 2012

This is a patch and unit test for Ticket #13629 - Admin Changelist: add app-model_name class to tag.
https://code.djangoproject.com/ticket/13629#comment:10

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need to add a class here, as there already is 'dashboard'.

@jphalip
Copy link
Member

jphalip commented Jun 17, 2012

This generally looks good, thank you. Now that I've thought about it some more, I think the classes should in fact be app- and model-. This would allow to dissociate the app from the model, and have for example app-wide styles.

Some templates are also missing: app_index.html and object_history.html

Thanks!

@ekkus93
Copy link
Author

ekkus93 commented Jun 20, 2012

Julien, I'm in the process of making those changes but you can you clarify what you mean by "the classes should in fact be app- and model-."? Right now, an admin link such as '/test_admin/%s/admin_views/section/add/' will have '' for the body tag. I think what you're suggesting is to also have a class for the body tag for non-admin (regular) pages. If this is true, could you give me an example of what the class for the non-admin body tag should be called?

@jphalip
Copy link
Member

jphalip commented Jul 2, 2012

Hi Phil. Sorry for the late reply. What I mean is that, if we have an app called 'blog' and a model called 'Entry', then we should have two classes: app-blog and model-entry. Both classes would be used on the add/change/history/delete views, and only the app-blog class would be used on the app index view. Does it make sense?

@apollo13
Copy link
Member

Closing here since #575 seems to have way more tests and is cleaned up a bit.

@apollo13 apollo13 closed this Dec 30, 2012
nanuxbe pushed a commit to nanuxbe/django that referenced this pull request Jul 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants