notebook icon indicating copy to clipboard operation
notebook copied to clipboard

Remove custom CSS handling in the notebook repo

Open jtpio opened this issue 1 year ago • 9 comments
trafficstars

The latest JupyterLab 4.1 pre-release includes a new functionality for loading custom CSS: https://github.com/jupyterlab/jupyterlab/pull/14743

The same feature was added in Notebook 7 for parity with the classic notebook:

  • https://github.com/jupyter/notebook/pull/6841
  • https://github.com/jupyter/notebook/pull/6919

Now that this is available in JupyterLab, we should look into removing the custom handling here and use the one from JupyterLab instead.

And maybe still keep the default to True in Notebook 7 (the default is False in JupyterLab).

We can also keep the existing documentation in Notebook 7 for visibility.

jtpio avatar Dec 06 '23 09:12 jtpio

cc @RRosio in case you would like to take a look!

jtpio avatar Dec 06 '23 09:12 jtpio

Thank you @jtpio! I will tackle removing the handling!

RRosio avatar Dec 06 '23 16:12 RRosio

After taking a look, using either of the LabApp.custom_css/LabServerApp.cusom_css flags and setting them to True, the custom CSS is applied thanks to the JupyterLab handler. However, I am having trouble modifying the default to True in Notebook. Before going any further with trying this, I was wondering if this seems like a valid approach to you @jtpio?

RRosio avatar Dec 07 '23 05:12 RRosio

Now wondering if setting the default to True in Notebook but False in JupyterLab could cause issues. Since the two apps would be running on the same Jupyter Server.

Also JupyterLab seems to be adding the handler only if the trait is set here:

https://github.com/jupyterlab/jupyterlab/blob/b20bc015328689ebda9c95fbc545c6aed0f26c82/jupyterlab/labapp.py#L748-L758

So the Notebook app would have to set the new default early for that handler to be added. Also enabling custom CSS in Notebook would enable it for JupyterLab.

jtpio avatar Dec 07 '23 09:12 jtpio

It would be easier and more consistent if both Notebook and JupyterLab would behave the same by default, and controlled with the same trait.

Unless we can seamlessly keep the trait defined here:

https://github.com/jupyter/notebook/blob/644c393580e69cbc9bc3feae10c9a3b0376e20d3/notebook/app.py#L251-L257

jtpio avatar Dec 07 '23 09:12 jtpio

Thank you for your replies @jtpio! I have tried to think through how Notebook and JupyterLab might be controlled by the same trait but have different details. At the moment I am trying to answer these questions for myself:

  • how to keep the Notebook custom_css trait as you have linked, so it remains and somehow provides an override for the JupyterLab declared custom_css trait
    • default the Notebook custom_css trait to True
    • find where I can make that override happen so that it is early enough to actually enable the handler

I am trying on my end to see if there is a way I can make that override early.

RRosio avatar Dec 12 '23 17:12 RRosio

Pinging @Zsailer here from the jupyter_server meeting

RRosio avatar Dec 14 '23 16:12 RRosio

Looking a bit into this, one challenge may be that both JupyterNotebookApp and LabApp are based on LabServerApp, instead of having JupyterNotebookApp being based on LabApp.

So the notebook package depends on jupyterlab, but the Notebook Jupyter Server ExtensionApp does not depend on the JupyterLab ExtensionApp. They are both Jupyter Server ExtensionApp running side by side.

So in the long run, maybe it could make sense to move the custom CSS feature to jupyterlab_server?

For the time being (Notebook 7.1), maybe the easiest would be to check whether self.custom_css is set to True before adding the handler here?

https://github.com/jupyter/notebook/blob/e8633bd8d7f66093753bee8a433ff1fd16eeaf07/notebook/app.py#L343

jtpio avatar Feb 01 '24 15:02 jtpio

Looking into this in https://github.com/jupyter/notebook/pull/7233.

jtpio avatar Feb 02 '24 16:02 jtpio