Fixed #28377 -- Made combining form Media retain relative asset order.
Thanks Florian Apolloner, Mariusz Felisiak, and Tim Graham for reviews.
This commit is contained in:
Johannes Hoppe
Tim Graham
parent
f86b6f351d
commit
c19b56f633
|
|
@ -304,7 +304,7 @@ class InlineAdminFormSet:
|
|||
|
||||
@property
|
||||
def media(self):
|
||||
media = self.opts.media + self.formset.media
|
||||
media = self.formset.media + self.opts.media
|
||||
for fs in self:
|
||||
media = media + fs.media
|
||||
return media
|
||||
|
|
|
|||
|
|
@ -579,9 +579,9 @@ class ModelAdmin(BaseModelAdmin):
|
|||
def media(self):
|
||||
extra = '' if settings.DEBUG else '.min'
|
||||
js = [
|
||||
'core.js',
|
||||
'vendor/jquery/jquery%s.js' % extra,
|
||||
'jquery.init.js',
|
||||
'core.js',
|
||||
'admin/RelatedObjectLookups.js',
|
||||
'actions%s.js' % extra,
|
||||
'urlify.js',
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ Form Widget classes specific to the Django admin site.
|
|||
import copy
|
||||
|
||||
from django import forms
|
||||
from django.conf import settings
|
||||
from django.db.models.deletion import CASCADE
|
||||
from django.urls import reverse
|
||||
from django.urls.exceptions import NoReverseMatch
|
||||
|
|
@ -22,7 +23,14 @@ class FilteredSelectMultiple(forms.SelectMultiple):
|
|||
"""
|
||||
@property
|
||||
def media(self):
|
||||
js = ["core.js", "SelectBox.js", "SelectFilter2.js"]
|
||||
extra = '' if settings.DEBUG else '.min'
|
||||
js = [
|
||||
'vendor/jquery/jquery%s.js' % extra,
|
||||
'jquery.init.js',
|
||||
'core.js',
|
||||
'SelectBox.js',
|
||||
'SelectFilter2.js',
|
||||
]
|
||||
return forms.Media(js=["admin/js/%s" % path for path in js])
|
||||
|
||||
def __init__(self, verbose_name, is_stacked, attrs=None, choices=()):
|
||||
|
|
@ -43,7 +51,13 @@ class FilteredSelectMultiple(forms.SelectMultiple):
|
|||
class AdminDateWidget(forms.DateInput):
|
||||
@property
|
||||
def media(self):
|
||||
js = ["calendar.js", "admin/DateTimeShortcuts.js"]
|
||||
extra = '' if settings.DEBUG else '.min'
|
||||
js = [
|
||||
'vendor/jquery/jquery%s.js' % extra,
|
||||
'jquery.init.js',
|
||||
'calendar.js',
|
||||
'admin/DateTimeShortcuts.js',
|
||||
]
|
||||
return forms.Media(js=["admin/js/%s" % path for path in js])
|
||||
|
||||
def __init__(self, attrs=None, format=None):
|
||||
|
|
@ -56,7 +70,13 @@ class AdminDateWidget(forms.DateInput):
|
|||
class AdminTimeWidget(forms.TimeInput):
|
||||
@property
|
||||
def media(self):
|
||||
js = ["calendar.js", "admin/DateTimeShortcuts.js"]
|
||||
extra = '' if settings.DEBUG else '.min'
|
||||
js = [
|
||||
'vendor/jquery/jquery%s.js' % extra,
|
||||
'jquery.init.js',
|
||||
'calendar.js',
|
||||
'admin/DateTimeShortcuts.js',
|
||||
]
|
||||
return forms.Media(js=["admin/js/%s" % path for path in js])
|
||||
|
||||
def __init__(self, attrs=None, format=None):
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ from django.contrib.admin import ModelAdmin
|
|||
from django.contrib.gis.admin.widgets import OpenLayersWidget
|
||||
from django.contrib.gis.db import models
|
||||
from django.contrib.gis.gdal import OGRGeomType
|
||||
from django.forms import Media
|
||||
|
||||
spherical_mercator_srid = 3857
|
||||
|
||||
|
|
@ -46,10 +47,7 @@ class GeoModelAdmin(ModelAdmin):
|
|||
@property
|
||||
def media(self):
|
||||
"Injects OpenLayers JavaScript into the admin."
|
||||
media = super().media
|
||||
media.add_js([self.openlayers_url])
|
||||
media.add_js(self.extra_js)
|
||||
return media
|
||||
return super().media + Media(js=[self.openlayers_url] + self.extra_js)
|
||||
|
||||
def formfield_for_dbfield(self, db_field, request, **kwargs):
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ HTML Widget classes
|
|||
import copy
|
||||
import datetime
|
||||
import re
|
||||
import warnings
|
||||
from contextlib import suppress
|
||||
from itertools import chain
|
||||
|
||||
|
|
@ -33,19 +34,23 @@ __all__ = (
|
|||
MEDIA_TYPES = ('css', 'js')
|
||||
|
||||
|
||||
class MediaOrderConflictWarning(RuntimeWarning):
|
||||
pass
|
||||
|
||||
|
||||
@html_safe
|
||||
class Media:
|
||||
def __init__(self, media=None, **kwargs):
|
||||
if media:
|
||||
media_attrs = media.__dict__
|
||||
def __init__(self, media=None, css=None, js=None):
|
||||
if media is not None:
|
||||
css = getattr(media, 'css', {})
|
||||
js = getattr(media, 'js', [])
|
||||
else:
|
||||
media_attrs = kwargs
|
||||
|
||||
self._css = {}
|
||||
self._js = []
|
||||
|
||||
for name in MEDIA_TYPES:
|
||||
getattr(self, 'add_' + name)(media_attrs.get(name))
|
||||
if css is None:
|
||||
css = {}
|
||||
if js is None:
|
||||
js = []
|
||||
self._css = css
|
||||
self._js = js
|
||||
|
||||
def __str__(self):
|
||||
return self.render()
|
||||
|
|
@ -88,24 +93,48 @@ class Media:
|
|||
return Media(**{str(name): getattr(self, '_' + name)})
|
||||
raise KeyError('Unknown media type "%s"' % name)
|
||||
|
||||
def add_js(self, data):
|
||||
if data:
|
||||
for path in data:
|
||||
if path not in self._js:
|
||||
self._js.append(path)
|
||||
@staticmethod
|
||||
def merge(list_1, list_2):
|
||||
"""
|
||||
Merge two lists while trying to keep the relative order of the elements.
|
||||
Warn if the lists have the same two elements in a different relative
|
||||
order.
|
||||
|
||||
def add_css(self, data):
|
||||
if data:
|
||||
for medium, paths in data.items():
|
||||
for path in paths:
|
||||
if not self._css.get(medium) or path not in self._css[medium]:
|
||||
self._css.setdefault(medium, []).append(path)
|
||||
For static assets it can be important to have them included in the DOM
|
||||
in a certain order. In JavaScript you may not be able to reference a
|
||||
global or in CSS you might want to override a style.
|
||||
"""
|
||||
# Start with a copy of list_1.
|
||||
combined_list = list(list_1)
|
||||
last_insert_index = len(list_1)
|
||||
# Walk list_2 in reverse, inserting each element into combined_list if
|
||||
# it doesn't already exist.
|
||||
for path in reversed(list_2):
|
||||
try:
|
||||
# Does path already exist in the list?
|
||||
index = combined_list.index(path)
|
||||
except ValueError:
|
||||
# Add path to combined_list since it doesn't exist.
|
||||
combined_list.insert(last_insert_index, path)
|
||||
else:
|
||||
if index > last_insert_index:
|
||||
warnings.warn(
|
||||
'Detected duplicate Media files in an opposite order:\n'
|
||||
'%s\n%s' % (combined_list[last_insert_index], combined_list[index]),
|
||||
MediaOrderConflictWarning,
|
||||
)
|
||||
# path already exists in the list. Update last_insert_index so
|
||||
# that the following elements are inserted in front of this one.
|
||||
last_insert_index = index
|
||||
return combined_list
|
||||
|
||||
def __add__(self, other):
|
||||
combined = Media()
|
||||
for name in MEDIA_TYPES:
|
||||
getattr(combined, 'add_' + name)(getattr(self, '_' + name, None))
|
||||
getattr(combined, 'add_' + name)(getattr(other, '_' + name, None))
|
||||
combined._js = self.merge(self._js, other._js)
|
||||
combined._css = {
|
||||
medium: self.merge(self._css.get(medium, []), other._css.get(medium, []))
|
||||
for medium in self._css.keys() | other._css.keys()
|
||||
}
|
||||
return combined
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -554,6 +554,11 @@ Miscellaneous
|
|||
* Renamed ``BaseExpression._output_field`` to ``output_field``. You may need
|
||||
to update custom expressions.
|
||||
|
||||
* In older versions, forms and formsets combine their ``Media`` with widget
|
||||
``Media`` by concatenating the two. The combining now tries to :ref:`preserve
|
||||
the relative order of elements in each list <form-media-asset-order>`.
|
||||
``MediaOrderConflictWarning`` is issued if the order can't be preserved.
|
||||
|
||||
.. _deprecated-features-2.0:
|
||||
|
||||
Features deprecated in 2.0
|
||||
|
|
|
|||
|
|
@ -305,6 +305,42 @@ specified by both::
|
|||
<script type="text/javascript" src="http://static.example.com/actions.js"></script>
|
||||
<script type="text/javascript" src="http://static.example.com/whizbang.js"></script>
|
||||
|
||||
.. _form-media-asset-order:
|
||||
|
||||
Order of assets
|
||||
---------------
|
||||
|
||||
The order in which assets are inserted into the DOM if often important. For
|
||||
example, you may have a script that depends on jQuery. Therefore, combining
|
||||
``Media`` objects attempts to preserve the relative order in which assets are
|
||||
defined in each ``Media`` class.
|
||||
|
||||
For example::
|
||||
|
||||
>>> from django import forms
|
||||
>>> class CalendarWidget(forms.TextInput):
|
||||
... class Media:
|
||||
... js = ('jQuery.js', 'calendar.js', 'noConflict.js')
|
||||
>>> class TimeWidget(forms.TextInput):
|
||||
... class Media:
|
||||
... js = ('jQuery.js', 'time.js', 'noConflict.js')
|
||||
>>> w1 = CalendarWidget()
|
||||
>>> w2 = TimeWidget()
|
||||
>>> print(w1.media + w2.media)
|
||||
<script type="text/javascript" src="http://static.example.com/jQuery.js"></script>
|
||||
<script type="text/javascript" src="http://static.example.com/calendar.js"></script>
|
||||
<script type="text/javascript" src="http://static.example.com/time.js"></script>
|
||||
<script type="text/javascript" src="http://static.example.com/noConflict.js"></script>
|
||||
|
||||
Combining ``Media`` objects with assets in a conflicting order results in a
|
||||
``MediaOrderConflictWarning``.
|
||||
|
||||
.. versionchanged:: 2.0
|
||||
|
||||
In older versions, the assets of ``Media`` objects are concatenated rather
|
||||
than merged in a way that tries to preserve the relative ordering of the
|
||||
elements in each list.
|
||||
|
||||
``Media`` on Forms
|
||||
==================
|
||||
|
||||
|
|
|
|||
|
|
@ -1,3 +1,5 @@
|
|||
import warnings
|
||||
|
||||
from django.forms import CharField, Form, Media, MultiWidget, TextInput
|
||||
from django.template import Context, Template
|
||||
from django.test import SimpleTestCase, override_settings
|
||||
|
|
@ -106,7 +108,7 @@ class FormsMediaTestCase(SimpleTestCase):
|
|||
class MyWidget3(TextInput):
|
||||
class Media:
|
||||
css = {
|
||||
'all': ('/path/to/css3', 'path/to/css1')
|
||||
'all': ('path/to/css1', '/path/to/css3')
|
||||
}
|
||||
js = ('/path/to/js1', '/path/to/js4')
|
||||
|
||||
|
|
@ -237,9 +239,9 @@ class FormsMediaTestCase(SimpleTestCase):
|
|||
w8 = MyWidget8()
|
||||
self.assertEqual(
|
||||
str(w8.media),
|
||||
"""<link href="http://media.example.com/static/path/to/css1" type="text/css" media="all" rel="stylesheet" />
|
||||
"""<link href="/path/to/css3" type="text/css" media="all" rel="stylesheet" />
|
||||
<link href="http://media.example.com/static/path/to/css1" type="text/css" media="all" rel="stylesheet" />
|
||||
<link href="/path/to/css2" type="text/css" media="all" rel="stylesheet" />
|
||||
<link href="/path/to/css3" type="text/css" media="all" rel="stylesheet" />
|
||||
<script type="text/javascript" src="/path/to/js1"></script>
|
||||
<script type="text/javascript" src="http://media.other.com/path/to/js2"></script>
|
||||
<script type="text/javascript" src="https://secure.other.com/path/to/js3"></script>
|
||||
|
|
@ -312,9 +314,9 @@ class FormsMediaTestCase(SimpleTestCase):
|
|||
w11 = MyWidget11()
|
||||
self.assertEqual(
|
||||
str(w11.media),
|
||||
"""<link href="http://media.example.com/static/path/to/css1" type="text/css" media="all" rel="stylesheet" />
|
||||
"""<link href="/path/to/css3" type="text/css" media="all" rel="stylesheet" />
|
||||
<link href="http://media.example.com/static/path/to/css1" type="text/css" media="all" rel="stylesheet" />
|
||||
<link href="/path/to/css2" type="text/css" media="all" rel="stylesheet" />
|
||||
<link href="/path/to/css3" type="text/css" media="all" rel="stylesheet" />
|
||||
<script type="text/javascript" src="/path/to/js1"></script>
|
||||
<script type="text/javascript" src="http://media.other.com/path/to/js2"></script>
|
||||
<script type="text/javascript" src="https://secure.other.com/path/to/js3"></script>
|
||||
|
|
@ -341,9 +343,9 @@ class FormsMediaTestCase(SimpleTestCase):
|
|||
w12 = MyWidget12()
|
||||
self.assertEqual(
|
||||
str(w12.media),
|
||||
"""<link href="http://media.example.com/static/path/to/css1" type="text/css" media="all" rel="stylesheet" />
|
||||
"""<link href="/path/to/css3" type="text/css" media="all" rel="stylesheet" />
|
||||
<link href="http://media.example.com/static/path/to/css1" type="text/css" media="all" rel="stylesheet" />
|
||||
<link href="/path/to/css2" type="text/css" media="all" rel="stylesheet" />
|
||||
<link href="/path/to/css3" type="text/css" media="all" rel="stylesheet" />
|
||||
<script type="text/javascript" src="/path/to/js1"></script>
|
||||
<script type="text/javascript" src="/path/to/js4"></script>"""
|
||||
)
|
||||
|
|
@ -396,7 +398,7 @@ class FormsMediaTestCase(SimpleTestCase):
|
|||
class MyWidget3(TextInput):
|
||||
class Media:
|
||||
css = {
|
||||
'all': ('/path/to/css3', 'path/to/css1')
|
||||
'all': ('path/to/css1', '/path/to/css3')
|
||||
}
|
||||
js = ('/path/to/js1', '/path/to/js4')
|
||||
|
||||
|
|
@ -441,7 +443,7 @@ class FormsMediaTestCase(SimpleTestCase):
|
|||
class MyWidget3(TextInput):
|
||||
class Media:
|
||||
css = {
|
||||
'all': ('/path/to/css3', 'path/to/css1')
|
||||
'all': ('path/to/css1', '/path/to/css3')
|
||||
}
|
||||
js = ('/path/to/js1', '/path/to/js4')
|
||||
|
||||
|
|
@ -518,3 +520,25 @@ class FormsMediaTestCase(SimpleTestCase):
|
|||
media = Media(css={'all': ['/path/to/css']}, js=['/path/to/js'])
|
||||
self.assertTrue(hasattr(Media, '__html__'))
|
||||
self.assertEqual(str(media), media.__html__())
|
||||
|
||||
def test_merge(self):
|
||||
test_values = (
|
||||
(([1, 2], [3, 4]), [1, 2, 3, 4]),
|
||||
(([1, 2], [2, 3]), [1, 2, 3]),
|
||||
(([2, 3], [1, 2]), [1, 2, 3]),
|
||||
(([1, 3], [2, 3]), [1, 2, 3]),
|
||||
(([1, 2], [1, 3]), [1, 2, 3]),
|
||||
(([1, 2], [3, 2]), [1, 3, 2]),
|
||||
)
|
||||
for (list1, list2), expected in test_values:
|
||||
with self.subTest(list1=list1, list2=list2):
|
||||
self.assertEqual(Media.merge(list1, list2), expected)
|
||||
|
||||
def test_merge_warning(self):
|
||||
with warnings.catch_warnings(record=True) as w:
|
||||
warnings.simplefilter('always')
|
||||
self.assertEqual(Media.merge([1, 2], [2, 1]), [1, 2])
|
||||
self.assertEqual(
|
||||
str(w[-1].message),
|
||||
'Detected duplicate Media files in an opposite order:\n1\n2'
|
||||
)
|
||||
|
|
|
|||
Loading…
Reference in New Issue