Opened 11 years ago
Last modified 18 months ago
#22298 new Cleanup/optimization
Rename Form Media to Static (or get rid of Form Media completely?)
Reported by: | Aymeric Augustin | Owned by: | |
---|---|---|---|
Component: | Forms | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | loic@…, mk@…, Sergey Fedoseev, Tobias Kunze, Mariusz Felisiak | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
The confusion between media and static files is almost cleared, except in the Widget API which still sports a Media class.
Change History (19)
comment:1 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|---|
Type: | Bug → Cleanup/optimization |
comment:2 by , 11 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
follow-up: 4 comment:3 by , 11 years ago
comment:4 by , 11 years ago
Replying to russellm:
Just throwing this out there on the stoop to see if the cat licks it up: is there any evidence that Media objects are actually being used?
I added this feature years ago, but it's not one of my proudest API moments - it didn't really solve the problem I was trying to fix.
Rather than go through a complicated rename process, should we just be deprecating it altogether?
I think it's pretty common to define a widget with a Media
class to reference static files (a JavaScript file enhancing the rendered input for instance).
It's also quite convenient way for third party apps to provide a fully bootstrapped object.
comment:5 by , 11 years ago
Ok - if you're convinced people are actually using it, consider my request withdrawn.
comment:6 by , 11 years ago
When I spotted this ticket, I thought the same thing as @russellm, why not just deprecate it, although I didn't dare mentioning it.
My biggest issue with it is that it's a very naive approach to a complicated problem. It doesn't really fit with today's development practices where a project can easily grow to hundreds of static assets, where javascript offers concepts like AMD, where CSS gained preprocessors like LESS or SASS and where files are concatenated for performance.
Having each input widget pull it's own assets can also be a performance footgun, and IMO it's hardy something we should recommend.
comment:7 by , 11 years ago
Well we can put this on hold until we have a superior replacement. Or maybe just drop the feature and tell people to include their JS globally.
comment:8 by , 11 years ago
Cc: | added |
---|
Telling people to include JS globally or to use a third party app akin to django-compressor and django-pipeline is IMO a viable option. It fixes the issue that random assets are loaded into the DOM by forcing users to acknowledge static dependencies.
A superior replacement would be to add one of the apps above to the contrib apps.
comment:9 by , 11 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:10 by , 11 years ago
I'd be in favor of deprecating it since I've never used it myself and it doesn't seem to attract much interest from core developers as far as I can tell. An alternative suggested (now nearly two years ago) in #18455 of starting "fresh" and creating a Static
class that integrates with contrib.staticfiles
may be interesting, although I tend to agree with Loic that this strategy may not really be consistent with modern practices and simply including JS globally may be easiest.
There are just two uses of this in Django itself, both in GeoDjango: OpenLayersWidget
and OSMWidget
.
Here are some related tickets:
#21987 - Allow Media objects to have their own MEDIA_TYPES
#18455 - Added hooks to Media for staticfiles app
#13978 - Allow inline js/css in forms.Media
#12265 - Media (js/css) collection strategy in Forms has no order dependence concept
#12264 - calendar.js depends on jsi18n but date widgets using it do not specify as required media
#9357 - Unable to subclass form Media class
I created a django-dev thread to get feedback from the community.
comment:11 by , 11 years ago
It'd be good to give the user the final choice. First of all, having a Static class instead of a Media class would be a good deprecation (and not a replacement until later versions).
Second: allow to have the currently supported behavior AND the alternative to load them as global -compressed- content.
Currently I developed a widget (a fully autocomplete-based app which contrasts with database entries | https://github.com/luismasuelli/jackfrost) which uses jquery ui by including static files via Media, and think that media objects, since they're application or widget-dependent, should not be deleted.
follow-up: 13 comment:12 by , 10 years ago
As a data point, I use it a bunch. As Mark said on django-developers, it's especially helpful for the admin. I'd be fine if we moved all support for it there instead of forms itself.
Again, because it's the admin, I personally don't need pipeline optimizations.
Overriding templates in the admin doesn't work very well in my experience. It's hard to override specific things and the templates are pretty undocumented so we don't maintain backwards compatibility very well. It's a little MVC-punching, but it's pretty nice to simply add a WYSIWYG field on your model and be able to get a nice widget in the admin automatically.
comment:13 by , 10 years ago
Replying to collinanderson:
As a data point, I use it a bunch. As Mark said on django-developers, it's especially helpful for the admin. I'd be fine if we moved all support for it there instead of forms itself.
I've just run into a situation where I am using CSS to brand the admin and the media class is not being inserted in the base templates. I would rather not have to override the templates since all I want to do is just add a custom CSS file to every page. I interpreted the documentation of the custom AdminSite feature to suggest that defining a Media class in my custom site would achieve this. At the moment, not so much.
Effectively adding {{ media.css }}
into the extrastyles
block of base.html
addresses much of the issue. Unfortunately login
and the delete_*confirm
templates (at least) would need additional modification since they derive from base_site.html
rather than base.html
.
Ideally adding 'media': self.media
to the each_context
function would also be required, but I can already override that in my custom AdminSite.
I don't have a full understanding of the distinction between these inheritance choices to know what the right change would be in that context. I wouldn't want to dive in and change the inheritance without knowing why they are base_site
vs base
templates.
Even if this functionality was moved into a Static
class rather than Media
, for me the feature is desirable, it's a very convenient way to brand the admin, but it's implementation at the moment is not consistent, from what I am seeing.
I was going to add a ticket for this consistency issue, but if this is a better ticket on which to hang the issue I'm happy.
comment:14 by , 10 years ago
I also think that it is valuable to have the functionality in Django Admin. Like mentioned above, it allows to add enhancements to certain views easily while not having to override the admin templates. I think that there are many small projects that are maintained through Django Admin and being able to drop in some Javascript for certain views is essential, at least for me. I would miss this functionality if it was gone completely (instead of keeping it or deprecating it in favor of something more elaborate).
comment:15 by , 9 years ago
Summary: | Rename Media to Static → Rename Form Media to Static (or get rid of Form Media completely?) |
---|
James Aylett (on stage at duth)'s solution "that will solve everything" (he says it came to him at 1:30 in the morning and he hasn't fully thought through it :):
{% load assets %} <html> <head> {% asset_tag css %} {% asset css "site-chrome" %} {% asset css "article-page" %} </head> <body> <h1>My heading</h1> {% asset css "search-box" %} <form action='{% url "search" %}'> <!-- ... --> </form> </body> </html>
comment:17 by , 6 years ago
Cc: | added |
---|
comment:18 by , 18 months ago
Cc: | added |
---|
I requested discussion of this issue on the Django forum in the hopes of moving this issue forward one way or another.
comment:19 by , 18 months ago
Cc: | added |
---|
Just throwing this out there on the stoop to see if the cat licks it up: is there any evidence that Media objects are actually being used?
I added this feature years ago, but it's not one of my proudest API moments - it didn't really solve the problem I was trying to fix.
Rather than go through a complicated rename process, should we just be deprecating it altogether?