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

Remove custom CSS handling in the notebook repo #7175

Open
jtpio opened this issue Dec 6, 2023 · 9 comments
Open

Remove custom CSS handling in the notebook repo #7175

jtpio opened this issue Dec 6, 2023 · 9 comments
Assignees
Milestone

Comments

@jtpio
Copy link
Member

jtpio commented Dec 6, 2023

The latest JupyterLab 4.1 pre-release includes a new functionality for loading custom CSS: jupyterlab/jupyterlab#14743

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

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 jtpio added this to the 7.1 milestone Dec 6, 2023
@jupyterlab-probot jupyterlab-probot bot added the status:Needs Triage Applied to issues that need triage label Dec 6, 2023
@jtpio
Copy link
Member Author

jtpio commented Dec 6, 2023

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

@RRosio
Copy link
Collaborator

RRosio commented Dec 6, 2023

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

@RRosio RRosio self-assigned this Dec 6, 2023
@RRosio
Copy link
Collaborator

RRosio commented Dec 7, 2023

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?

@jtpio
Copy link
Member Author

jtpio commented Dec 7, 2023

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
Copy link
Member Author

jtpio commented Dec 7, 2023

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:

notebook/notebook/app.py

Lines 251 to 257 in 644c393

custom_css = Bool(
True,
config=True,
help="""Whether custom CSS is loaded on the page.
Defaults to True and custom CSS is loaded.
""",
)

@RRosio
Copy link
Collaborator

RRosio commented Dec 12, 2023

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
Copy link
Collaborator

RRosio commented Dec 14, 2023

Pinging @Zsailer here from the jupyter_server meeting

@jtpio
Copy link
Member Author

jtpio commented Feb 1, 2024

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?

self.handlers.append(("/custom/custom.css", CustomCssHandler))

@jtpio
Copy link
Member Author

jtpio commented Feb 2, 2024

Looking into this in #7233.

@jtpio jtpio modified the milestones: 7.1.0, 7.1.x Feb 13, 2024
@jtpio jtpio modified the milestones: 7.1.x, 7.3.0 May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants