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 foundational support for loading p5 addons in preview-frame.ts. #63

Merged
merged 2 commits into from
Sep 11, 2016

Conversation

toolness
Copy link
Owner

@toolness toolness commented Aug 28, 2016

This adds foundational support for loading multiple libraries which depend on p5.js in preview-frame.ts. It effectively implements option 3 described in #53 (comment).

The biggest volatility here is that it uses an undocumented feature of the p5 constructor whereby if a sketch function isn't passed in as the first argument, p5 will initialize itself in global mode. If we can make this a documented, supported use case in p5 core, that will ensure the widget continues to work as p5 matures. I've filed processing/p5.js#1570 for this.

Thoughts @kaganjd ?

}

let global = window as PreviewFrameWindow;

function loadP5(version: string, cb?: () => void) {
let url = '//cdnjs.cloudflare.com/ajax/libs/p5.js/' + version + '/p5.js';
function loadScript(url, cb?: () => void) {

Choose a reason for hiding this comment

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

o hai :-) just a drive-by suggestion here: might be nice to listen for script load errors and log something to the console for debugging

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ah good idea! I'll add that--this will also be useful for when we allow people to specify arbitrary URLs to third-party add-on libraries, since we want to provide good feedback if they misspell any of those URLs or something.

Choose a reason for hiding this comment

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

Cool! Glad my drive-by review was helpful ^_^

On Monday, August 29, 2016, Atul Varma [email protected] wrote:

In lib/preview-frame.ts
#63 (comment):

}

let global = window as PreviewFrameWindow;

-function loadP5(version: string, cb?: () => void) {

  • let url = '//cdnjs.cloudflare.com/ajax/libs/p5.js/' + version + '/p5.js';
    +function loadScript(url, cb?: () => void) {

Ah good idea! I'll add that--this will also be useful for when we allow
people to specify arbitrary URLs to third-party add-on libraries, since we
want to provide good feedback if they misspell any of those URLs or
something.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/toolness/p5.js-widget/pull/63/files/ffe52963c21320e73664190dcfa70c2878b3a50e#r76695989,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAF4jGGEq79d0FNqZtg7XCRb6v1kKU5tks5qk1ZqgaJpZM4Ju51N
.

@kaganjd
Copy link
Contributor

kaganjd commented Aug 30, 2016

@toolness Awesome! I'm still wrapping my head around the concept and syntax here, but I get that this lets us call p5 global mode after we're sure everything else has loaded. Seems more elegant than a bunch of iframes too. I'll stay tuned for the response to processing/p5.js#1570 :)

@toolness toolness merged commit 9e7a745 into master Sep 11, 2016
@toolness toolness deleted the preview-frame-addon-plumbing branch September 11, 2016 16:20
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.

3 participants