holoviz-dev / nbsite

Build a tested, sphinx-based website from notebooks
https://nbsite.holoviz.org
BSD 3-Clause "New" or "Revised" License
28 stars 13 forks source link

Added support for :no_interactivity_warning: directive option #191

Closed jlstevens closed 3 years ago

jlstevens commented 3 years ago

Extends the directive with the :no_interactivity_warning: as follows:

.. notebook:: example ../examples/Development.ipynb
    :offset: 0
    :no_interactivity_warning:

The CSS is specified in https://github.com/pyviz-dev/sphinx_holoviz_theme/pull/16 and obviously needs to be improved but here is the idea (when :no_interactivity_warning: is not specified):

image

We should now decide what the message should be exactly and what link(s) it should have.

jbednar commented 3 years ago

And yes, the CSS needs improvement; it looks for instance like it will overlap the main text? I think it should be in a separate colored box, not interfering with the main content, and a nice balance between ignorable once read and noticeable at first.

jlstevens commented 3 years ago

Generating the suggested message as of the last commit:

image

For this to work, the build command needs the necessary arguments e.g:

nbsite build --org pyviz-dev --project-name nbsite --binder top

Note that if you left click, you see the notebook JSON hence the need to right-click to download (maybe solvable with some JS?):

image

If you supply --binder none (or don't specify) the message adjusts accordingly:

image

jbednar commented 3 years ago

Thanks. Presumably the message isn't meant to overlap other screen elements, right? It certainly looks bad when it does.

The message should probably say "Right click to download and run locally" so that it makes sense when the binder link isn't there for comparison.

The "Right click" bit is definitely wordy and distracting, not to mention not necessarily true on every platform. It does seem like it ought to open as a download link automatically; viewing the JSON is almost never useful.

jlstevens commented 3 years ago

Presumably the message isn't meant to overlap other screen elements, right?

Agreed - that is the sort of CSS fix that is still needed.

The message should probably say "Right click to download and run locally" so that it makes sense when the binder link isn't there for comparison.

Sure. I'll update the message there.

The "Right click" bit is definitely wordy and distracting, not to mention not necessarily true on every platform.

I think it is true on all platforms even though it is wordy. Hopefully some JS can help address this though I also note this is nothing new as you can see by looking at the bottom of the hvplot Introduction for instance:

jlstevens commented 3 years ago

Tests are passing again. As for avoiding right clicking, I've come up with the following JavaScript example (e.g you can paste into your web console to test):

function download_file(filename, data) {
  let a = document.createElement('a');
  a.style = "display: none";
  let blob = new Blob([data], {type: "application/octet-stream"});
  let url = window.URL.createObjectURL(blob);
  a.href = url;
  a.download = filename;
  document.body.appendChild(a);
  a.click();
  // See https://stackoverflow.com/questions/30694453 for Firefox support
  setTimeout(function(){
    document.body.removeChild(a);
    window.URL.revokeObjectURL(url);
  }, 100);
  document.body.removeChild(a);
  window.URL.revokeObjectURL(url);
}
const url = 'https://raw.githubusercontent.com/pyviz-dev/nbsite/master/examples/Usage.ipynb';
fetch(url).then(d => d.text()).then(d => download_file("test.ipynb", d));

The anchor would then run this JS to download on click.

That said, I'm not convinced that this is better than telling people to right click and download...

philippjfr commented 3 years ago

I think right-click to save is preferable over some JS hackery, I usually prefer that anyway because I don't want stuff sent straight to my download folder. I'd rather save it where I want it in the first place.

jlstevens commented 3 years ago

I'm going to merge and cut a dev release to test. Happy to open a new PR if we decide on new improvements to implement.