[WIP] Split out embed manager #1394

pbugnion · 2017-05-29T13:52:52Z

This is a very tentative PR to split out the embed manager from the jupyter-js-widgets package into its own embed-jupyter-widgets package. This PR arose from both comments in the code and a discussion around PR #1380.

Prior to this PR, to embed the widgets you had to include https://unpkg.com/jupyter-js-widgets@~3.0.0-alpha.6/dist/embed.js in the HTML. This distributed both the embed widget manager and the JS for the default widgets present in Jupyter. After this PR, you will have to include https://unpkg.com/embed-js-widgets@*/dist/index.js. The default jupyter widgets lose their special status: they are now bundled as an AMD library like any other set of custom widgets. They are then fetched from unpkg (or locally) dynamically when they are requested in the embed state.

This changes several things, some of which I was very unsure of, so it would be good to get some feedback:

the URL for the embed bundle is now different. This will break existing code that doesn't explicitly set the version number. This is particularly insidious because the old url https://unpkg.com/jupyter-js-widgets@~3.0.0-alpha.6/dist/embed.js still works: it just now releases an AMD packaged library which cannot be used by itself.
presumably this will affect the release process.
the web4 example is currently somewhat broken: we can easily load the new embed-jupyter-widgets as a local dependency, but I think it makes sense to load the core widgets from unpkg -- since this is what users will do most of the time. This won't work until there is a new version of the library on unpkg though.
the generateEmbedScript in embed-helper seems to be unused. Should it be removed?

AFAICT, this is what's left to do:

Update documentation on embedding
Actually generate a new release of jupyter-js-widgets on unpkg and check that embedding still works.
Generate an alternative style sheet for embedding? In particular, we could make the disconnected symbol go away.
Update the code snippet that the user is supposed to copy and paste when they click on Embed Widgets in the menu so it points to the new package.
Add method to retrieve URL.

It's now the main purpose of that module

This removes the 'privileged' position of the widgets defined in 'jupyter-js-widgets'.

This is useful until the release of jupyter-js-widgets stabilises

maartenbreddels · 2017-05-29T13:59:53Z

It would be really helpful to have a say ipywidgets.embed.get_embed_url() for #1387 and an alternative to jupyter/jupyter-sphinx#13

pbugnion · 2017-05-29T14:09:45Z

@maartenbreddels That makes sense. Do you see this just returning a hard-coded URL (maybe with the version dynamically included)?

pbugnion · 2017-05-30T06:29:46Z

To test this PR:

Create an HTML document with some widget state in a new directory $TEST_DIR:

<html>
    <head>
        <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css">
        <script src="./embed-jupyter-widgets.js"></script>

        <script type="application/vnd.jupyter.widget-state+json">

         {
             "version_major": 2,
             "version_minor": 0,
             "state": {
                 "1ba8af4bbc874fe581360d8f6734079d": {
                     "model_name": "SliderStyleModel",
                     "model_module": "jupyter-js-widgets",
                     "model_module_version": "3.0.0",
                     "state": {
                         "description_width": ""
                     }
                 },
                 "635ea1d3185746ef88b2a9453cdbf7ad": {
                     "model_name": "LayoutModel",
                     "model_module": "jupyter-js-widgets",
                     "model_module_version": "3.0.0",
                     "state": {}
                 },
                 "7c1d41e9e96b4f94ae9219b31257cd2e": {
                     "model_name": "IntSliderModel",
                     "model_module": "jupyter-js-widgets",
                     "model_module_version": "3.0.0",
                     "state": {
                         "style": "IPY_MODEL_1ba8af4bbc874fe581360d8f6734079d",
                         "layout": "IPY_MODEL_635ea1d3185746ef88b2a9453cdbf7ad"
                     }
                 }
             }
         }

        </script>

        <!-- <style> -->
        <!-- .jupyter-widgets-disconnected::before { -->
        <!-- content: "" ; -->
        <!-- } -->
        <!-- </style> -->

    </head>

    <body>
        <h1>Output embedding example</h1>
        <div id="widget-embedded-here">
            <script type="application/vnd.jupyter.widget-view+json">
             {
                 "model_id": "7c1d41e9e96b4f94ae9219b31257cd2e",
                 "version_minor": "0",
                 "version_major": "2"
             }
            </script>
        </div>
    </body>
</html>

Compile the core widgets bundle by running npm run build in jupyter-js-widgets. This generates an AMD library in dist/embed.js. Move embed.js to the directory $TEST_DIR and rename it jupyter-js-widgets so it matches the module_name that the widgets require.
Compile the embed manager by running npm run build in embed-jupyter-widgets. This generates a top-level library in dist/index.js. Move this to $TEST_DIR and rename it embed-jupyter-widgets.js so it gets picked up by the script tags.

jasongrout · 2017-05-30T17:45:30Z

@pbugnion - are you saying this PR is ready to review (i.e., remove the WIP in the title)?

pbugnion · 2017-05-30T17:55:29Z

Thanks! It's ready for review, but there's definitely still some work that needs doing (I've highlighted what's left to do in checkboxes in the PR description). I just don't feel confident addressing what's left to do without you (or someone else) validating the approach so far.

jasongrout

I think this general approach looks good. Since you have to import jupyter-js-widgets anyway for the manager base class, you might just include jupyter-js-widgets by default at this point.

jasongrout · 2017-05-30T17:51:33Z

embed-jupyter-widgets/package.json

+  "dependencies": {
+    "@phosphor/widgets": "^1.2.0",
+    "font-awesome": "^4.7.0",
+    "jupyter-js-widgets": "file:../jupyter-js-widgets"


Just like the other managers (widgetsnbextension, jupyterlab_widgets) rely on a published version of jupyter-js-widgets, we'll probably update this when we release to point to a release instead of a file path.

jasongrout · 2017-05-30T20:58:50Z

@pbugnion - are you around sometime today to have a video chat, or maybe in-person chat?

pbugnion · 2017-05-31T04:39:50Z

A video chat sounds good, time zone issues notwithstanding (I live in London). I suppose mornings or early afternoons in EST would work best.

pbugnion · 2017-05-31T04:52:06Z

Those changes look great, thanks!

pbugnion · 2017-05-31T05:13:17Z

How were you thinking of loading the embed-jupyter-widgets version? The easiest seems to be to just add a (fixed) dependency on embed-jupyter-widgets in widgetsnbextension once embed-juptyer-widgets has been released, and load the version from there?

jasongrout · 2017-05-31T05:24:08Z

How were you thinking of loading the embed-jupyter-widgets version? The easiest seems to be to just add a (fixed) dependency on embed-jupyter-widgets in widgetsnbextension once embed-juptyer-widgets has been released, and load the version from there?

If you're talking about what version to specify in widgetsnbextension, yes, for now hardcoding the embed-jupyter-widgets version there.

jasongrout · 2017-05-31T05:24:46Z

What do you think about calling it jupyter-widgets-htmlmanager?

pbugnion · 2017-05-31T05:27:07Z

jupyter-widgets-htmlmanager sounds good!

pbugnion · 2017-05-31T05:28:38Z

To avoid merge conflicts or duplicating work you might be doing, I might try and tackle the documentation for this, unless you think there's something more pressing?

jasongrout · 2017-05-31T05:29:57Z

I'm about ready to merge this. Just contemplating the name of the package...

jasongrout · 2017-05-31T14:34:44Z

Thanks again @pbugnion for taking this on!

pbugnion added 11 commits May 28, 2017 11:50

Initial commit for new embed-jupyter-widgets package

edb6ce8

Remove dead code from config

fd64756

Only bundle the widgets themselves in jupyter-js-widgets

23d4281

Remove embed source from jupyter-js-widgets

0f767a8

Correctly bundle jupyter-js-widgets

b75012e

Rename embed bundle to index

2de7803

It's now the main purpose of that module

Avoid auto-injecting jupyter-js-widgets bundle into DOM

05a928d

This removes the 'privileged' position of the widgets defined in 'jupyter-js-widgets'.

Load jupyter-js-widgets locally temporarily

059aafc

This is useful until the release of jupyter-js-widgets stabilises

Embed styles with native widgets

35860f2

Hack web4 example to still work with the current embed bundle

0a22195

Add .gitignore to new embed bundle

e08b32a

maartenbreddels mentioned this pull request May 29, 2017

Embed code generation from Python #1387

Merged

3 tasks

jasongrout added this to the 7.0 milestone May 30, 2017

Add newline

305ba4f

jasongrout reviewed May 30, 2017

View reviewed changes

jasongrout added 9 commits May 30, 2017 14:14

Clean up files in the embed manager separation.

1e2819b

Clean up dependencies in jupyter-js-widgets

bd75793

Delete the embedding webpack build from jupyter-js-widgets.

2b5c180

Restore the shortcut loading jupyter-js-widgets in the embedded manager.

3086a9b

Make ajv work with schema draft 4

c22f411

console.error schema errors.

d83d165

Only fall back to unpkg.com if there was a module timeout in require.

b40cd22

Make the embed jupyter widget manager version 0.1.0 - let’s start fresh.

bafbd2c

Move the web4 example to the embed package.

f31c6bf

jasongrout added 2 commits May 30, 2017 18:52

Document how the widgetsnbextension is not working right now.

cd7def1

Rename web5 example since it’s no longer in order.

0acecae

jasongrout mentioned this pull request May 31, 2017

Split manager and widgets #1396

Closed

Fix more mentions of the web4 and web5 tests.

e3933ba

pbugnion mentioned this pull request May 31, 2017

[WIP] Docs for new embed package #1397

Closed

jasongrout added 2 commits May 31, 2017 06:22

Move the embed manager to jupyter-widgets-htmlmanager

65cdafc

Change embed-jupyter-widgets to jupyter-widgets-htmlmanager

288f29a

jasongrout merged commit 92ef3e8 into jupyter-widgets:master May 31, 2017

This was referenced May 31, 2017

Naming fix for the htmlmanager #1399

Merged

Big Split(tm) Figure out the development workflow after the split #1151

Closed

github-actions bot added the resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Feb 14, 2021

github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Split out embed manager #1394

[WIP] Split out embed manager #1394

pbugnion commented May 29, 2017 •

edited

Loading

maartenbreddels commented May 29, 2017

pbugnion commented May 29, 2017

pbugnion commented May 30, 2017 •

edited

Loading

jasongrout commented May 30, 2017

pbugnion commented May 30, 2017

jasongrout left a comment

jasongrout May 30, 2017

jasongrout commented May 30, 2017

pbugnion commented May 31, 2017

pbugnion commented May 31, 2017

pbugnion commented May 31, 2017

jasongrout commented May 31, 2017

jasongrout commented May 31, 2017

pbugnion commented May 31, 2017

pbugnion commented May 31, 2017

jasongrout commented May 31, 2017

jasongrout commented May 31, 2017

[WIP] Split out embed manager #1394

[WIP] Split out embed manager #1394

Conversation

pbugnion commented May 29, 2017 • edited Loading

maartenbreddels commented May 29, 2017

pbugnion commented May 29, 2017

pbugnion commented May 30, 2017 • edited Loading

jasongrout commented May 30, 2017

pbugnion commented May 30, 2017

jasongrout left a comment

Choose a reason for hiding this comment

jasongrout May 30, 2017

Choose a reason for hiding this comment

jasongrout commented May 30, 2017

pbugnion commented May 31, 2017

pbugnion commented May 31, 2017

pbugnion commented May 31, 2017

jasongrout commented May 31, 2017

jasongrout commented May 31, 2017

pbugnion commented May 31, 2017

pbugnion commented May 31, 2017

jasongrout commented May 31, 2017

jasongrout commented May 31, 2017

pbugnion commented May 29, 2017 •

edited

Loading

pbugnion commented May 30, 2017 •

edited

Loading