New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Removed fictional class 'extrapretty' from admin docs. #18075
base: main
Are you sure you want to change the base?
Conversation
@@ -417,7 +417,7 @@ subclass:: | |||
Example:: | |||
|
|||
{ | |||
"classes": ["wide", "extrapretty"], | |||
"classes": ["wide"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I saw this in the docs recently I had the opposite feeling, that we should replace wide
with a very obvious fictional name, such as your-custom-css-class-here
:
"classes": ["wide"], | |
"classes": ["custom-class-1", "custom-class-2"], |
My rationale is that the paragraph immediately following already discusses the two "useful classes defined by the default admin site stylesheet". However, since this specific example focuses on explaining how to utilize the classes
attribute to apply any class defined in your CSS stylesheet, I believe it's best to keep these two ideas separate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would guess wide
and collapse
cover 90%+ usage of this feature. It’s non-trivial to write extra CSS that fits in with all the Admin’s styles, see the provided classes:
https://github.com/django/django/blob/main/django/contrib/admin/static/admin/css/forms.css#L191-L211
So I’d prefer to present a real-world, copy-pastable example than a hypothetical one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My concern is that what wide
does and what collapse
does is explained after the example, in a separated paragraph. So when reading top-bottom, the user encounters:
- explanation of the
fields
options dict, which shows fictional fields - explanation of the
classes
options dict, with an initial sentence reading A list or tuple containing extra CSS classes to apply to the fieldset. Example:
So at this point I think a fictional example is expected (or assumed) as well. Otherwise,if we use real admin CSS classes, the user:
- Will not know at the time of the (first time) reading that those are real classes
- May get the impression that they can only choose from those two classes
If you feel strongly about using the builtin admin CSS classes in the example, what about rewording the initial paragraph? Initial proposal:
diff --git a/docs/ref/contrib/admin/index.txt b/docs/ref/contrib/admin/index.txt
index f75b950262..c8ce4b0a57 100644
--- a/docs/ref/contrib/admin/index.txt
+++ b/docs/ref/contrib/admin/index.txt
@@ -413,18 +413,20 @@ subclass::
* ``classes``
A list or tuple containing extra CSS classes to apply to the fieldset.
+ These classes can include any custom CSS class defined in the project,
+ as well as any of the CSS classes provided by Django. Within the
+ default admin site CSS stylesheet, two particularly useful classes are
+ defined: `collapse` and `wide`.
Example::
{
- "classes": ["wide", "extrapretty"],
+ "classes": ["wide", "collapse"],
}
- Two useful classes defined by the default admin site stylesheet are
- ``collapse`` and ``wide``. Fieldsets with the ``collapse`` style
- will be initially collapsed in the admin and replaced with a small
- "click to expand" link. Fieldsets with the ``wide`` style will be
- given extra horizontal space.
+ Fieldsets with the ``collapse`` style will be initially collapsed in
+ the admin and replaced with a small "click to expand" link. Fieldsets
+ with the ``wide`` style will be given extra horizontal space.
* ``description``
A string of optional extra text to be displayed at the top of each
Trac ticket number
N/A
Branch description
The class
extrapretty
is a fictional example, whilst the classwide
is not. Its existence in the documentation dates back to when classes were specified in a string, from a19ed8a, and it has been untouched since.I just found a project where the example
classes
had been copy-pasted around the place, applying this fictional class in many admins. To avoid this kind of confusion recurring, I think we should remove it from the docs, covering only the concrete classes defined by Django.Checklist
main
branch.