works with ipywidgets7rc0 #16
Conversation
|
This may break some themes, like the default Jupyter theme that ipywidgets uses. In that case, we were using the default embed url, which does not require requirejs, since requirejs broke the page. Can we add a config option for this plugin to choose between the requirejs and standard embedding, and switch the includes based on that? For the standard widget docs, I'd set that config to not be the requirejs option. |
README.md
Outdated
|
|
||
| You conf.py has two extra configuration options: | ||
|
|
||
| * jupyter_sphinx_require_url: url for `require.js` (if your theme already provides this, set it to False or '') |
There was a problem hiding this comment.
How do I say that I want to not use require (i.e., my theme doesn't provide it, and I don't want it on the page)
There was a problem hiding this comment.
I think I misunderstood it them, if this param is set to a falsy, it will not include require, and choose the DEFAULT_EMBED_SCRIPT_URL default url.
There was a problem hiding this comment.
Ah, the docs there seemed to indicate that requirejs must be on the page. Let's reword, then. Perhaps:
* jupyter_sphinx_require_url: url for `require.js` (if requirejs is not needed or your theme already provides this, set it to False or '')
How do I say that my theme already provides require, so I must use the require embedding URL?
Thinking about it more, how about the following options:
- jupyter_sphinx_use_requirejs = True / False, defaults to True
- jupyter_sphinx_requirejs_url = some url, or the default of '' to automatically include
- jupyter_sphinx_embed_url = some url, or the default of '' to automatically choose an embed url based on jupyter_sphinx_use_requirejs
There was a problem hiding this comment.
hmm, you still can't say "use the requirejs embedding, but don't include requirejs" - so maybe the requirejs url can be set to one value to indicate to automatically include a default version, a string for a specific version, and some other value to indicate to not include it.
There was a problem hiding this comment.
What about:
- jupyter_sphinx_embed_url: if not set will be chosen based on next config value (jupyter_sphinx_embed_with_require)
- jupyter_sphinx_embed_with_require: True/False will fill in jupyter_sphinx_embed_url if not set
- jupyter_sphinx_require_url: url to require.js, or empty to not include it.
README.md
Outdated
| @@ -70,6 +70,13 @@ The directives have the following options: | |||
| Button() | |||
| ``` | |||
|
|
|||
| ### Configuration | |||
|
|
|||
| You conf.py has two extra configuration options: | |||
There was a problem hiding this comment.
Yes, optional indeed
|
@maartenbreddels is it possible to add option for adding app.add_stylesheet('https://cdnjs.cloudflare.com/ajax/libs/jqueryui/1.12.0/jquery-ui.css') |
|
@maartenbreddels and @jasongrout what are status of this PR? |
|
I believe stylesheets can be added in sphinx, I don't think this is the place to add this. Apart from that I'm ok with someone merging this ;-) I've been using it for ipyvolume succesfully. |
yes, I tried your branch and work nicely with nglview too. |
|
This looks good to me. Any blocker on your side @jasongrout? |
|
I don't recall any outstanding objection. |
|
If everyone is OK with this, would it be OK to merge and release? :) |
|
better squash this |
|
@jasongrout / @SylvainCorlay I think only owners have merge right on this repo. Would you consider merging as both of you seem to think it is good to go? |
|
Wow, I thought that this was merged. Doing this now. |
|
Any block for a |
|
The only issue I know of is a css issue. If you open http://ipyvolume.readthedocs.io/en/latest/ you can see the text starts grey, then when the css is loaded turns blue. I've seen this also on other rtd pages, so it's not ipyvolume specific, but i'm not sure it has anything to do with jupyter-sphinx. |
|
@maartenbreddels I think that is unrelated to this repo: jupyter-widgets/ipywidgets#1673 Once that gets fixed it should propagate directly, right? |
|
Ah yes, lost track of that, so unrelated. |
|
@SylvainCorlay Considering that the issue Maarten mentioned was unrelated to this package and that he has used this in production by linking his branch I think it should be ready for a 0.1.2 release. |
|
ok, making the release now. |
No description provided.