-
-
Notifications
You must be signed in to change notification settings - Fork 404
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
Improved handling of JS and CSS dependencies #426
Changes from all commits
12d582b
8a779a4
94dd0c4
9616d44
54ff7a1
9fa79c9
c4c1e3e
2455dba
1b7bde3
af3a8a5
3a17f2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -123,10 +123,16 @@ class Renderer(Exporter): | |
# Define appropriate widget classes | ||
widgets = {'scrubber': ScrubberWidget, 'widgets': SelectionWidget} | ||
|
||
js_dependencies = ['https://code.jquery.com/jquery-2.1.4.min.js', | ||
'https://cdnjs.cloudflare.com/ajax/libs/require.js/2.1.20/require.min.js'] | ||
core_dependencies = {'jQueryUI': {'js': ['https://code.jquery.com/ui/1.10.4/jquery-ui.min.js'], | ||
'css': ['https://code.jquery.com/ui/1.10.4/themes/smoothness/jquery-ui.css']}} | ||
|
||
css_dependencies = ['https://maxcdn.bootstrapcdn.com/bootstrap/3.3.5/css/bootstrap.min.css'] | ||
extra_dependencies = {'jQuery': {'js': ['https://code.jquery.com/jquery-2.1.4.min.js']}, | ||
'underscore': {'js': ['https://cdnjs.cloudflare.com/ajax/libs/underscore.js/1.8.3/underscore-min.js']}, | ||
'require': {'js': ['https://cdnjs.cloudflare.com/ajax/libs/require.js/2.1.20/require.min.js']}, | ||
'bootstrap': {'css': ['https://maxcdn.bootstrapcdn.com/bootstrap/3.3.6/css/bootstrap.min.css']}} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good. I suspect it might have been possible to avoid the deep nesting with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would also need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I have no real objection to leaving it as it is. |
||
# Any additional JS and CSS dependencies required by a specific backend | ||
backend_dependencies = {} | ||
|
||
def __init__(self, **params): | ||
super(Renderer, self).__init__(**params) | ||
|
@@ -238,18 +244,8 @@ def static_html(self, obj, fmt=None, template=None): | |
supplied format. Allows supplying a template formatting string | ||
with fields to interpolate 'js', 'css' and the main 'html'. | ||
""" | ||
css_html, js_html = '', '' | ||
js, css = self.embed_assets() | ||
for url in self.js_dependencies: | ||
js_html += '<script src="%s" type="text/javascript"></script>' % url | ||
js_html += '<script type="text/javascript">%s</script>' % js | ||
|
||
for url in self.css_dependencies: | ||
css_html += '<link rel="stylesheet" href="%s">' % url | ||
css_html += '<style>%s</style>' % css | ||
|
||
js_html, css_html = self.html_assets() | ||
if template is None: template = static_template | ||
|
||
html = self.html(obj, fmt) | ||
return template.format(js=js_html, css=css_html, html=html) | ||
|
||
|
@@ -340,10 +336,13 @@ class needed to render it with the current renderer. | |
|
||
|
||
@classmethod | ||
def embed_assets(cls): | ||
def html_assets(cls, core=True, extras=True, backends=None): | ||
""" | ||
Returns JS and CSS and for embedding of widgets. | ||
""" | ||
if backends is None: | ||
backends = [cls.backend] if cls.backend else [] | ||
|
||
# Get all the widgets and find the set of required js widget files | ||
widgets = [wdgt for r in Renderer.__subclasses__() | ||
for wdgt in r.widgets.values()] | ||
|
@@ -358,7 +357,26 @@ def embed_assets(cls): | |
if f is not None ) | ||
widgetcss = '\n'.join(open(find_file(path, f), 'r').read() | ||
for f in css if f is not None) | ||
return widgetjs, widgetcss | ||
|
||
dependencies = {} | ||
if core: | ||
dependencies.update(cls.core_dependencies) | ||
if extras: | ||
dependencies.update(cls.extra_dependencies) | ||
for backend in backends: | ||
dependencies['backend'] = Store.renderers[backend].backend_dependencies | ||
|
||
js_html, css_html = '', '' | ||
for _, dep in sorted(dependencies.items(), key=lambda x: x[0]): | ||
for js in dep.get('js', []): | ||
js_html += '\n<script src="%s" type="text/javascript"></script>' % js | ||
for css in dep.get('css', []): | ||
css_html += '\n<link rel="stylesheet" href="%s">' % css | ||
|
||
js_html += '\n<script type="text/javascript">%s</script>' % widgetjs | ||
css_html += '\n<style>%s</style>' % widgetcss | ||
|
||
return js_html, css_html | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks good. If necessary, could one day be made to spit out a dictionary of resource locations if we ever needed them without the surrounding HTML tags. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might as well just access the core and extra dependencies directly. |
||
|
||
|
||
@classmethod | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,15 +24,15 @@ | |
</div> | ||
</div> | ||
<script> | ||
/* Load JQuery UI CSS */ | ||
// Slider JS Block START | ||
function loadcssfile(filename){ | ||
var fileref=document.createElement("link") | ||
fileref.setAttribute("rel", "stylesheet") | ||
fileref.setAttribute("type", "text/css") | ||
fileref.setAttribute("href", filename) | ||
document.getElementsByTagName("head")[0].appendChild(fileref) | ||
} | ||
loadcssfile("https://code.jquery.com/ui/1.10.4/themes/ui-lightness/jquery-ui.css"); | ||
loadcssfile("https://code.jquery.com/ui/1.10.4/themes/smoothness/jquery-ui.css"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Change of theme? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yea, the slide bar on this one is white, gives a bit more contrast. |
||
/* Check if jQuery and jQueryUI have been loaded | ||
otherwise load with require.js */ | ||
var jQuery = window.jQuery, | ||
|
@@ -102,7 +102,7 @@ | |
} else { | ||
var dim_val = vals[ui.value]; | ||
} | ||
$('#textInput{{ id }}_{{ widget_data['dim'] }}').val(dim_val); | ||
$('#textInput{{ id }}_{{ widget_data['dim'] }}').val((dim_val*1).toString()); | ||
anim{{ id }}.set_frame(dim_val, {{ widget_data['dim_idx'] }}); | ||
if (Object.keys(next_vals).length > 0) { | ||
var new_vals = next_vals[dim_val]; | ||
|
@@ -141,8 +141,9 @@ | |
} | ||
} | ||
}); | ||
$('#textInput{{ id }}_{{ widget_data['dim'] }}').val(vals[0]); | ||
$('#textInput{{ id }}_{{ widget_data['dim'] }}').val((vals[0]*1).toString()); | ||
}); | ||
// Slider JS Block END | ||
</script> | ||
{% elif widget_data['type']=='dropdown' %} | ||
<div class="form-group control-group" style='{{ widget_data['visibility'] }}'> | ||
|
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.
Definitely cleaner. I assume you'll collect the core renderer dependencies together later...