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

New approach for JupyterLab 3 #605

Closed
wants to merge 49 commits into from

Conversation

fcollonval
Copy link
Collaborator

@fcollonval fcollonval commented Nov 15, 2021

This is a reboot of #381 - in order to move forward on jupyter/notebook#6220

The approach taken differs from #381 as it creates a stand-alone app (similarly to retrolab - JLab remix) in order to:

  • Deal with the DOM manipulation induced by RevealJS that don't play nicely in webapp such JupyterLab
  • Add the opportunity to spin a notebook focusing on slideshow experience

What it does:

  • Add button in JupyterLab notebook toolbar to open the RISE view of the current notebook
  • Support federated extension in RISE mode (in particular ipywidgets are working)
  • Restructure completely the package to be a single python package with multiple npm packages to ship a single package for the classical notebook and JupyterLab
  • Support ipywidget
  • Run on Binder for the classical notebook and JupyterLab (but failed to open the standalone app from JupyterLab)

What it partly do:

  • Create a ugly slideshow with no configuration

What needs to be done:

  • Improve styling
  • properly handle all kinds of cells (subslide, fragment, ...)
  • Render slideshow in side-by-side panel within JupyterLab
  • Create custom notebook factory to remove the toolbar Explicitly dispose the toolbar (avoid dealing with specific factory)
  • Turn off virtual rendering Wait for fully rendering before converting to slideshow (placeholder cannot be removed due to DOM hierarchy change for Reveal)
  • Turn off heading collapse feat.
  • Check Jupyter Widget interact
  • Implement as much features as possible from classical:
  • x [out of scope] Allow to edit style info from cell toolbar (updating extension??)
  • x [out of scope] [optional] Bring something like split-cell classic extension
  • Smart execution
  • Documentation
  • CI job to at least test it installs

Help will be really appreciate to cover the above points - don't hesitate to open PRs against this PR branch.

rise_demo_20211120

@fcollonval
Copy link
Collaborator Author

Binder Link

@parmentelat
Copy link
Collaborator

sounds cool, but the binder looks broken, or what am I missing ?

404-not-found

@fcollonval
Copy link
Collaborator Author

Unfortunately it does not work on Binder... needs to set the redirection.

@parmentelat
Copy link
Collaborator

no big deal if binder won't work
this is going to give us an opportunityto run the code from source
how should we go about doing that ?

@damianavila
Copy link
Owner

damianavila commented Nov 15, 2021

This is AWESOME, @fcollonval!!
I will try to digest the content ASAP and provide feedback!!
At first, the standalone app is an interesting compromise to get people running on JLab ASAP.
Super excited about this, thanks again for the contribution!

@fcollonval
Copy link
Collaborator Author

sounds cool, but the binder looks broken, or what am I missing ?

@parmentelat I fixed Binder if you want to try.

@parmentelat
Copy link
Collaborator

@fcollonval this looks just great, it is several orders of magnitude better than anything we've been trying to do as a jlab extension; many thanks !

I have to honestly admit that I have not been able to cope with the jlab extension dev paradigm(s); so how do I setup a dev environment to proceed on your track ?

I tried this just now:

git switch jlab3
git rev-parse HEAD
→ 42a9d9aa501894ee2d02782c08d364c287d9a5f9
conda create -y -n rise python=3.9
conda activate rise
pip install jupyterlab
pip install .
jupyter lab --version
→ 3.2.3
jupyter lab

but unsurprisingly it does not seem to work, I can't see the RISE button and am getting this screenshot below

it feels real dumb, please enlighten me :)

image

@parmentelat
Copy link
Collaborator

on a totally different level, one thing that I had at some point considered - before realizing that developing a jlab extension was way beyond my skills - was to have the regular notebook and the rise view in 2 separate tabs
this way I thought ideally we would have gotten the notebook view and the rise view side-by-side, so as to, for example, have a live preview of the rendered slides while tweaking the notebook

this was imho the most important breakthrough that porting to a jlab extension could bring us

I'm not quite sure this is doable at all though, so if this rings any bell wrt this new angle, please elaborate on the feasibility of that idea :)

@fcollonval
Copy link
Collaborator Author

fcollonval commented Nov 16, 2021

how do I setup a dev environment to proceed on your track ?

Here are the commands to execute (I updated the dev note):

    yarn install
    yarn run build
    pip install -e .
    jupyter server extension enable rise
    jupyter serverextension enable rise
    jupyter labextension develop --overwrite .
    jupyter-nbextension install rise --py --sys-prefix --symlink
    jupyter-nbextension enable rise --py --sys-prefix

the notebook view and the rise view side-by-side

This should be achievable with the new approach by rendering the presentation in a IFrame like the voila preview

Edit: correct command for dev mode

@parmentelat
Copy link
Collaborator

Hi
thanks for the dev recipe
I've just given that a try and am getting mixed results
I do see the new extension button now, but that gives me a 404 not found - just like the first gif above in fact
not sure how to troubleshoot that issue though

@fcollonval
Copy link
Collaborator Author

You may need to install the server extension for the old notebook based server with:

jupyter server extension enable rise

@parmentelat
Copy link
Collaborator

yes, this last command does the trick !

@damianavila
Copy link
Owner

@damianavila I discuss with @parmentelat about creating a dedicated Python package for the JupyterLab version (keep everything in this repo but with two python packages).

I think that is a good idea.

The idea is to publish and iterate on the jupyterlab version in a simple way for users not comfortable with JavaScript tooling. Are you ok with that idea?

Can you please expand on that idea?
See my comment here about how I envision the next steps for RISE: #614 (comment) and #614 (comment).

Do you agree/disagree? Thoughts?

If so, should we merge the iteration on the master branch of this repo or iterate on a separate branch (I would prefer using master to reduce complexity for contributors)?

Which iteration are you referring to? The one creating the 2 packages?

Would you be willing to extend my rights on this repo to help with the maintenance?

OBVIOUSLY, already done!!

@damianavila
Copy link
Owner

Maybe we should continue the above conversation in #614, so we keep this one specifically for the content of this PR...

@datakurre
Copy link

@fcollonval Should it be already possible to run this with JupyterLite with some configuration? (With defaults I do get the button, but it eventually fails due to backend server route not being there with lite.)

@bollwyvl
Copy link
Contributor

run this with JupyterLite

I'd wager this wouldn't work in Lite yet.

As the actual renderer is a whole separate lumino app/shell (akin to retro or lab) it would need a separate app/slides/index.ts which was aware of the shortcomings of the browser (e.g. can't list files in a directory) and worked around them (replacing the fetch implementation), but still able to load federated extensions like widgets.

The iframe preview capability from within lab might work, but many HTTP hosts have issues with same-origin iframes, no matter what we set on the sandbox... while the notebook contents might update if changed, the kernel state would not be shared until we figure out a way to launch them inside serviceworkers, or somehow coordinated iframes.

That being said, having a /slides/?path=Notebook.ipynb would be something that we should strongly consider enabling in the core lite repo: the nominal cost for the baseline app would be very low likely (~100kb), as all the apps (lab, retro and repl) are all part of a single shared webpack build, and the actual rendering parts would work as "just" a federated extension.

Copy link

@krassowski krassowski left a comment

Choose a reason for hiding this comment

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

I gave it a spin today, it's really cool! I noted one problem which likely needs fixing before a release - when other extensions are installed they can get added into the RISE preview area (I suggested a fix in the comment).

Otherwise I am happy with the UX. I would be even more happy if the font size in RISE and style were more consistent with notebook (e.g. lists and tables are very small while other markdown text is huge by comparison) but this might be for another time.

/**
* The Rise application shell token.
*/
export const IRetroShell = new Token<RiseShell>('rise-application:IRiseShell');

Choose a reason for hiding this comment

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

Suggested change
export const IRetroShell = new Token<RiseShell>('rise-application:IRiseShell');
export const IRiseShell = new Token<RiseShell>('rise-application:IRiseShell');

// pass no-op
}
BoxLayout.setStretch(widget, 1);
(this.layout as BoxLayout).addWidget(widget);
Copy link

@krassowski krassowski Jun 21, 2022

Choose a reason for hiding this comment

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

I believe that the intention was that only custom notebook would get added, but my citation manager reference browser gets added:

Screenshot from 2022-06-21 21-02-34

This happens after the following call: app.shell.add(referenceBrowser, 'left', { rank: 850 }); in jupyterlab-citation-manager. You would have noted this with other federated extensions like jupyterlab-git which also add themselves to the shell in the left area, but you likely did not because they failed to activate (e.g. Plugin '@jupyterlab/git:plugin' failed to activate.) as they requires tokens which are not provided in this context, whereas jupyterlab-citation-manager is not fussy and has all tokens (except for INotebookTracker) optional.

Choose a reason for hiding this comment

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

I would suggest to add if (area == 'rise') { or at the very least if (area == 'main') {.

// Remove the toolbar
notebookPanel.toolbar.dispose();

app.shell.add(notebookPanel);
Copy link

@krassowski krassowski Jun 21, 2022

Choose a reason for hiding this comment

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

Maybe

Suggested change
app.shell.add(notebookPanel);
app.shell.add(notebookPanel, 'rise');

see my comment on packages/application/src/app/index.ts.

Comment on lines +291 to +293
`<input name="renderOnSave" type="checkbox"></input>${trans.__(
'Render on Save'
)}`

Choose a reason for hiding this comment

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

I would suggest adding a class name + style to move the checkbox a little bit down (e.g. top: 4px; position: relative;) as it is currently out of line:

Screenshot from 2022-06-21 21-07-11

* extend examples/README to test tables

* tentative fix for <li> elements that are too small
@YiqinZhang
Copy link

I had a problem with the chalkboard. The two buttons only work as jumping to the first slide, and the pen did not show up. I use Chrom(103.0.5060.53) and Firefox(102.0 (64-bit)) on Fedora36, and installed packages are listed below:

IPython          : 8.4.0
ipykernel        : 6.15.0
ipywidgets       : 7.7.1
jupyter_client   : 7.3.4
jupyter_core     : 4.10.0
jupyter_server   : 1.18.0
jupyterlab       : 3.1.18
nbclient         : 0.6.4
nbconvert        : 6.5.0
nbformat         : 5.4.0
notebook         : 6.4.12
qtconsole        : not installed
traitlets        : 5.3.0

Also, I found that the 'Notes only mode' does not work in the speaker's view. The Notes cells didn't show in the speaker notes but appeared on the slideshow. The same situation happens in the Binder as well.

And out of curiosity, are there any updates on the black theme?

Many thanks!

@connorferster
Copy link

connorferster commented Jul 12, 2022

Nothing productive to add. I just want to say I am very excited for this! Thank you @fcollonval @damianavila @parmentelat @krassowski for all of your work on this.

@YiqinZhang
Copy link

YiqinZhang commented Jul 29, 2022

I found that the 'Notes only mode' does not work in the speaker's view. The Notes cells didn't show in the speaker notes but appeared on the slideshow. The same situation happens in the Binder as well.

I've tried to fix showing notes in Speak notes Speaker notes here. fcollonval#5
@fcollonval

PS: After reinstalling the chalkboard patch, the chalkboard and draw functions perform properly.

Co-authored-by: YiqinZhang <yiqinzhang@crc-dot1x-nat-10-239-213-212.bu.edu>
@fcollonval
Copy link
Collaborator Author

Thanks a lot @YiqinZhang I merged your PR

@jim-smith
Copy link

Problem opening second notebook as RISE presentation

hi all,
really excited about this as the loveliness of RISE for my teaching is stopping me fully making the transition to jupyterlab (Which I use for research and day-to-day stuff).

After following the right instructions (readme in the branch) I got this up and running nicely ...
but it only works for the first notebook I open .

After that, when I try to open a second, then select the rise button I see this message in the terminal from which I started Jupiter lab

  • [IPKernelApp] ERROR | No such comm target registered: jupyter.widget.control

Closing Jupyter and restarting lets me open the second notebook quite happily, - it doesn't seem to be anything to do with specific notebooks, just that closing one and starting another somehow loses track of the widgets ...

Also it doesn't work if I leave the first notebook running in rise mode then open another notebook and try to switch to rise in that.

Any help gratefully accepted as my weekly lecture tend to be split into three notebooks to match the short video chunks I make from them. (obviously I can shutdown and restart the Jupiter server as a workaround but this is not ideal)

@YiqinZhang
Copy link

I tried the keyboard shortcut and found both the keyboards "," for Hide/show RISE buttons and "?" for Help did not work as expected. In the notebook version, we can use "," to toggle all RISE buttons, but the hard-wired keyboard binding did not work in Jlab. Any thoughts about that?

@YiqinZhang
Copy link

I tried to fix toggling all RISE buttons so that the RISE controls won't cover the slide contents. And then bind keyboard "m" to hide/show buttons on the slideshow. fcollonval#6

@nthiery
Copy link

nthiery commented Oct 2, 2022

Hi!

Thanks so much for your work on rise on jupyterlab; we transitioned our
large classes here in Orsay to JupyterLab this semester, but we still need
to get back to Jupyter for our lectures. Not super convenient, but it's ok.

Where it gets a bit more complicated is that, in the spring semester, it's not
just the lecturers that use Jupyter slides, but also students for their project
presentations.

Do you have an idea at this stage whether there is a chance for rise to
be functional in JupyterLab by then, even with some rough corners? No
pressure intended; it's just to know and start planning a plan B if needed.

Thanks in advance!
Nicolas

@echarles
Copy link

Hi there, is anyone actively working on this? If not, any continue the work with more forces?

@damianavila
Copy link
Owner

Sorry for the long silence folks, I did not have time to push forward on this one, regrettably.
I do not want to make any promises but I plan to spend a few cycles working on this one in the coming weeks.

@fcollonval, do you think you will have some cycles to review my changes in the coming weeks? Just trying to understand if I will have feedback from you or if I would need to figure out things for myself 😉.

Btw, I would also like to discuss with you how the story would look like for the new notebook...
Since the approach here is based on a stand-alone app, it should be relatively straightforward to "migrate" it to the new notebook, right?

@fcollonval
Copy link
Collaborator Author

@fcollonval, do you think you will have some cycles to review my changes in the coming weeks? Just trying to understand if I will have feedback from you or if I would need to figure out things for myself wink.

I'll find time for that.

Btw, I would also like to discuss with you how the story would look like for the new notebook... Since the approach here is based on a stand-alone app, it should be relatively straightforward to "migrate" it to the new notebook, right?

Yes it should work (hopefully out of the box) in notebook v7.

@fcollonval
Copy link
Collaborator Author

Closing this as I extracted the plugin to its own repository (as the path forward was suggesting building a separate python package).

The Python package has been publish on PyPI.org: https://pypi.org/project/jupyterlab-rise
The repository is: https://github.com/jupyterlab-contrib/rise

@LucaSoato
Copy link

Hi everyone,
do you think it's compatible with JupyterLab 4?
Thanks for the great contribution there, guys ❤️

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.