damianavila / RISE

RISE: "Live" Reveal.js Jupyter/IPython Slideshow Extension
Other
3.67k stars 414 forks source link

Control buttons of nbtutor disappear when going in RISE live show mode #587

Closed Naereen closed 3 years ago

Naereen commented 3 years ago

Hello there, I'm opening this issue here, but I don't expect a reaction from RISE, but more from the other side, of the nbtutor extension developpers. But opening this issue here means that if someone encounters the same bug as I did, they will be able to know why!


This extension works very well for me, and I'm a big fan and intense user of the RISE jupyter extension to teach with Jupyter notebook being displayed in full screen as slides.

As shown in this demo: Peek 21-02-2021 03-04

using nbtutor from the RISE live show mode (regardless of being full-screen) works from the backend side, but not from the frontend UI side: the control buttons are hidden! But exiting the RISE mode shows that the buttons are there, they work and everything is fine.

Naereen commented 3 years ago

So, I found what to change on RISE css, just delete or disable this rule!

.rise-enabled.ctb_global_show .ctb_hideshow.ctb_show {
    display: none;
}

Peek 21-02-2021 03-22

So it's this line: https://github.com/damianavila/RISE/blob/ed64f2fe81b7d26ee34c96f4dfdfbf74cdae7fb7/classic/src/less/main.less#L111

.rise-enabled {
  [...]
  &.ctb_global_show {
    .ctb_hideshow.ctb_show { display: none; }
    .ctb_show ~ .text_cell_render { border-style: none; }
  }
Naereen commented 3 years ago

I guess the easiest way to go, if someone encounters the same issue, is to use Stylus

And this CSS userstyle:

@-moz-document regexp("https?://localhost:[0-9]*/notebooks/.*#/slide.*") {
/* New CSS style to show Jupyter celltoolbar when RISE live show mode is enabled.
 * See https://github.com/lgpage/nbtutor/issues/42
 * Released under the term of the MIT Licence (https://lbesson.mit-license.org/)
 * By Lilian Besson, © 2021
 * To use with Stylus, see https://userstyles.org
**/
.rise-enabled.ctb_global_show .ctb_hideshow.ctb_show {
    display: inherit !important;
}
}

source

parmentelat commented 3 years ago

Hi

this is interesting; I had looked into nbtutor a - rather long - while back, but it was not ready back then so I went for ipythontutor, a magic that embeds iframes to pythontutor; I'll look into nbtutor again then

meanwhile... I expect the rule is here for a reason, and that was probably because 'something' was using that area in a way deemed too intrusive to the slide mode but I can't remember what this 'something' could have been; a nbextension most likely but there are so many of them.. I'd like to hear from @damianavila about this, if he can remember why the rule is here in the first place

@Naereen have you carefully compared the display with and without your patch on various notebooks and in various situations (like, changing the ViewCell toolbar setting) ?

parmentelat commented 3 years ago

also please note there was an issue #360 filed on a very similar subject

Naereen commented 3 years ago

Hi @parmentelat, thanks for your reply!

I admire your rigor and the fact that you thought about the purpose of this CSS rule, and why RISE was hiding cell toolbar initially. Giving my unperfect knowledge of RISE but good knowledge of CSS, I'm pretty confident to say that changing just this CSS rule will change nothing but the thing I claimed it changes: hide/show the cell toolbars when in RISE show mode.

Naereen commented 3 years ago

Peek 21-02-2021 16-12 Full demonstration (of a notebook which does not use nbtutor!)

parmentelat commented 3 years ago

thanks for clarifying; indeed displaying the toolbar unconditionnally would be wrong I guess, b/c it is intrusive to add this extra height everywhere

maybe we could think of some mechanism to let users decide on which cells they are willing to pay for that extra space ?

the usual path goes through cell metadata; not sure that's the smartest thing to do though; other proposals are welcome; @damianavila would you like to chime in ?

Naereen commented 3 years ago

maybe we could think of some mechanism to let users decide on which cells they are willing to pay for that extra space ?

Maybe, it's a possibility. Not sure how hard it would be to implement this, and whom could benefit from it. It's starting to be a very specific need.

parmentelat commented 3 years ago

It's starting to be a very specific need.

well, you were the one to bring that up, weren't you ? ;-)

Naereen commented 3 years ago

It's starting to be a very specific need. well, you were the one to bring that up, weren't you ? ;-)

Yes indeed, but I'm afraid that adding a new cell metadata will be much more complicated to use than my "off/on" Stylus extension.

parmentelat commented 3 years ago

all right, fair enough, let's close this then

thanks for the tip, that I'm sure others will find helpful !

Naereen commented 3 years ago

Thanks!

This idea of customizing tiny bits of RISE (or jupyter) using Stylus userstyle is something I never thought about before, but I like it!

damianavila commented 3 years ago

I'd like to hear from @damianavila about this, if he can remember why the rule is here in the first place

Well, @Naereen actually said a part of it above :wink:

IMHO, the only reason why this CSS rule was here, is because when presenting in RISE mode, you want the screen to be as uncluttered and clean of distracting things as possible, and win vertical/horizontal space as much as possible! Removing the cell toolbar is the best default choice for RISE!

But there is another piece here... changing types (slide, subslide, etc) on the flight (in the slideshow view) is problematic and this is why we only allow "design" on the notebook view mode...

the usual path goes through cell metadata; not sure that's the smartest thing to do though; other proposals are welcome; @damianavila would you like to chime in ?

I feel the default is still right... but maybe we can allow somehow to show other celltool bars? That feels complex from the top of my head... and since we have a workaround :wink:

Naereen commented 3 years ago

I feel the default is still right... but maybe we can allow somehow to show other celltool bars? That feels complex from the top of my head... and since we have a workaround wink

Hi @damianavila thanks for your feedback. I'm not at all aware of the implementations details of RISE, but from what I know about Jupyter notebooks, it's probably hard to "allow somehow to show other celltool bars", I would suggest to not even try.