Skip to content
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

Add javascript library and css stylesheet loading to JS class. #1410

Merged
merged 2 commits into from Mar 3, 2012

Conversation

ellisonbg
Copy link
Member

I have added a lib and css keyword argument to the Javascript display class. The lib option allows you to specify javascript libraries to load asynchronously before the JS code is run. The css option allows you to specify css stylesheet that should be loaded. Both the lib and css arguments are sequences of URLs to load the resources from. Use like this:

Javascript("""<js code goes here>""",
lib=['http://mbostock.github.com/d3/d3.js'],
css=['http://mbostock.github.com/d3/ex/force.css']
)

Most importantly, the js code run by the object can use the loaded js library as well as the css styles.

r += self.data
if self.lib:
for i in range(len(self.lib)):
r+= lib_t2
Copy link
Member

Choose a reason for hiding this comment

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

I think that r += lib_t2 * len(self.lib) would make sense to replace the if statement and python for-loop here (lines 410-412).

the same could be done with the css and lib for-loops above by converting them to a tuple, like r += css_t * len(self.css) % tuple(self.css) and slightly cleaner if css was already a tuple. It may not by worth it in this case to convert those, although the whole function would have a bit more consistency to it - and look like this:

     def _repr_javascript_(self):
        r  = css_t  * len(self.css) % tuple(self.css)
        r += lib_t1 * len(self.lib) % tuple(self.lib)
        r += self.data
        r += lib_t2 * len(self.lib)
        return r

@minrk
Copy link
Member

minrk commented Feb 18, 2012

Just to be sure, is this what is supposed to happen when you give more than one js lib?

In [11]: js = Javascript("""var a=[1,2,3];""", lib=["http://1.js","http://2.js"])
In [12]: print js._repr_javascript_()
$.getScript("http://1.js", function () {
$.getScript("http://2.js", function () {
var a=[1,2,3];});
});

If that is indeed correct, it seems to serialize the loading of multiple scripts unnecessarily.

@ellisonbg
Copy link
Member Author

jQuery doesn't have a way of loading multiple script simultaneously.
There was an issue opened for this:

http://bugs.jquery.com/ticket/9320

And a gist that implemented the capability, but it wasn't excepted.
We could include the patch though.

Cheers,

Brian

On Sat, Feb 18, 2012 at 3:29 PM, Min RK
reply@reply.github.com
wrote:

Just to be sure, is this what is supposed to happen when you give more than one js lib?

In [11]: js = Javascript("""var a=[1,2,3];""", lib=["http://1.js","http://2.js"])
In [12]: print js._repr_javascript_()
$.getScript("http://1.js", function () {
$.getScript("http://2.js", function () {
var a=[1,2,3];});
});

If that is indeed correct, it seems to serialize the loading of multiple scripts unnecessarily.

---
Reply to this email directly or view it on GitHub:
https://github.com/ipython/ipython/pull/1410#issuecomment-4038297

Brian E. Granger
Cal Poly State University, San Luis Obispo
bgranger@calpoly.edu and ellisonbg@gmail.com

@minrk
Copy link
Member

minrk commented Feb 19, 2012

We could include the patch though.

No, I don't want to include any unnecessary patches. Besides, I doubt too many people are going to be loading more than one or two scripts in one of these calls, so it's not a huge deal. I just wanted to understand better, and now I do. Thanks!

ellisonbg added a commit that referenced this pull request Mar 3, 2012
Add javascript library and css stylesheet loading to JS class.
@ellisonbg ellisonbg merged commit 2ded070 into ipython:master Mar 3, 2012
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Add javascript library and css stylesheet loading to JS class.
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.

None yet

3 participants