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

Loads p5.min.js for CodeFrame iframes using manually created script #224

Merged
merged 5 commits into from
Apr 23, 2024

Conversation

howard-e
Copy link
Contributor

Address #213.

This PR updates how CodeFrame created iframes make use of p5.min.js.

It avoids requiring a potentially cached version of the p5.min.js script, by instead passing the text content of the script directly to the iframe to be included in a newly created script element.

…5.min.js.

Avoids requiring a cached version of script by passing the text content of the script directly to the iframe.
@howard-e howard-e requested a review from stalgiag April 23, 2024 19:17
@stalgiag
Copy link
Collaborator

@howard-e I think that the build is failing because client:load is now necessary on the CodeFrame inside the SketchEmbed component. This is untested.

@howard-e
Copy link
Contributor Author

@stalgiag 4b4be8d should address that build error.

It also reflects our earlier discussion on handling a future optimization effort where client:only will be need to be included on <SketchEmbed>.

// from running `draw` every frame, which helps performance
// on pages with multiple sketches, and causes sketch
// animations only to start when they become visible.
useLayoutEffect(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice addition!

Copy link
Collaborator

@stalgiag stalgiag left a comment

Choose a reason for hiding this comment

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

Works well! Thank you!

@stalgiag stalgiag merged commit 409b81e into main Apr 23, 2024
4 checks passed
@stalgiag stalgiag deleted the issue-213 branch April 23, 2024 23:42
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.

2 participants