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

Fix jquery and element var #1152

Merged
merged 4 commits into from
Dec 18, 2019

Conversation

maartenbreddels
Copy link
Collaborator

Fixes #1148 and alternative to #1149.

jQuery support was removed in #1056 based on what we've done with voila. However, since we now have the two template (classic and lab) it makes sense that classic stays as compatible as it is with the classical notebook.

Therefore I've restored the jQuery library, and made the template exposes the element variable again (used for js library/code).

However, we want to get rid of jQuery, so in the lab template, we simply point the element variable to the DOM Node, which has an API similar, meaning the example library mentioned in #1149 works with both the classical and lab template.

The original issue in #1148 was solved by removing the unnecessary div element, and point element to the same DOM element as what would happen in the classical notebook.

<script type="text/javascript">
var element_id = '#{{ div_id }}';
var element = $('#{{ div_id }}');
Copy link
Contributor

Choose a reason for hiding this comment

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

I was under the impression that jupyter added this automatically as part of the script below. I'll try to confirm that by running this branch locally

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that would be great if you could test it. But this part was only for widgets, for JS it was still correct.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I sadly have no experience (yet) with widgets

Copy link
Contributor

@MSeal MSeal left a comment

Choose a reason for hiding this comment

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

Thanks for getting this change in @maartenbreddels ! I'll post on the original issue threads to ask people to try testing the change out to see if it resolves the issue smoothly for them.

@MSeal MSeal merged commit 5d34201 into jupyter:master Dec 18, 2019
@MSeal MSeal added this to the 6.0 milestone Sep 8, 2020
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.

Reveal.js does not render Javascript output correctly
3 participants