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

Load custom CSS #6841

Merged
merged 16 commits into from Jun 9, 2023
Merged

Load custom CSS #6841

merged 16 commits into from Jun 9, 2023

Conversation

RRosio
Copy link
Collaborator

@RRosio RRosio commented Apr 17, 2023

  • Placeholder custom CSS file
  • Link stylesheet in template files
  • Add handler for static file
  • Update scripts involving static files

To Fix
Current error is Refused to apply style from 'http://localhost:8888/custom/custom.css' because its MIME type ('text/html') is not a supported stylesheet MIME type, and strict MIME checking is enabled.

@jtpio jtpio added this to the 7.0 milestone Apr 20, 2023
@jtpio
Copy link
Member

jtpio commented Apr 20, 2023

Thanks @RRosio for looking into this!

Current error is Refused to apply style from 'http://localhost:8888/custom/custom.css' because its MIME type ('text/html') is not a supported stylesheet MIME type, and strict MIME checking is enabled.

Maybe it's because the handler gives a 404? (which would be an HTML page). What does the network tab of the dev tools say?

@RRosio
Copy link
Collaborator Author

RRosio commented Apr 21, 2023

Maybe it's because the handler gives a 404? (which would be an HTML page). What does the network tab of the dev tools say?

Hi @jtpio! Thank you for taking a look here! Yes that is exactly the issue I overlooked! I updated the href for the file to href="{{page_config['fullStaticUrl'] | e}}/custom/custom.css" and it now grabs the template CSS file I added in /notebook/static/custom successfully, but does not apply the styles from the CSS file in my .jupyter/custom directory. I have been trying to understand how that CSS file in my jupyter config paths is supposed to override the template CSS file in /notebook/static/custom, but am not sure about it yet. I can't seem to find much about how that configuration is set...

@jtpio
Copy link
Member

jtpio commented May 14, 2023

is supposed to override the template CSS file in /notebook/static/custom, but am not sure about it yet

My guess is that it is added alongside the existing template CSS file? So that it doesn't need to override, but instead provide additional CSS rules.

@RRosio
Copy link
Collaborator Author

RRosio commented Jun 1, 2023

I did some brief investigations on the follow-up question to add the option of enabling/disabling this functionality. My current understanding is that the CLI flag and the user setting options could look a bit like the following:

CLI Flag

  • It would entail adding a CLI option, and passing that to page_config. Then read the appropriate page_config property on the front end HTML templates to determine whether or not to add href links. If href links are added, the request is handled by the CustomCssHandler.

User Setting

  • Add SettingsEditor option (as a menu item): Custom CSS: On/Off and a user clicks on the button to update the boolean flag of the extension. On (True) then add the href link for custom CSS which is handled by the CustomCssHandler. If it is later switched to Off (False), an event listener waiting for a change in this value catches the change, inserts the href links, and then refreshes (or prompts the user to refresh) the page.

I'm not certain some of these steps are entirely viable so if something stands out in these options I appreciate feedback! In particular I am wondering about if and how I can access the values set in the settings editor from the html templates.

@RRosio
Copy link
Collaborator Author

RRosio commented Jun 6, 2023

PR currently implements a CLI flag for custom CSS, with a True value default. One required check is failing: Test Lint, due to the following

notebook/app.py: note: In member "_prepare_templates" of class "JupyterNotebookApp":
notebook/app.py:306: error: Cannot determine type of "jinja2_env"  [has-type]
            self.jinja2_env.globals.update(custom_css=self.custom_css)
            ^~~~~~~~~~~~~~~
Installing missing stub packages:
/home/runner/.local/share/hatch/env/virtual/notebook/yXCDlSUQ/typing/bin/python -m pip install types-PyYAML types-Pygments types-Send2Trash types-cffi types-colorama types-decorator types-jsonschema types-paramiko types-psutil types-python-dateutil types-pywin32 types-requests types-setuptools types-six


Found 1 error in 1 file (checked 6 source files)
Error: Process completed with exit code 1

@jtpio
Copy link
Member

jtpio commented Jun 6, 2023

Maybe it's because ExtensionAppJinjaMixin is missing?

According to the docs: https://jupyter-server.readthedocs.io/en/latest/developers/extensions.html#jinja-templating-from-frontend-extensions

This adds a <name>_jinja2_env setting to Tornado Web Server’s settings that will be used by request handlers.

@RRosio
Copy link
Collaborator Author

RRosio commented Jun 6, 2023

Maybe it's because ExtensionAppJinjaMixin is missing?

Oh thank you for pointing that out @jtpio! I will update with the import and check on the CI job result!

@RRosio RRosio marked this pull request as ready for review June 6, 2023 09:04
@RRosio
Copy link
Collaborator Author

RRosio commented Jun 6, 2023

I ended up adding a type hint to resolve the issue. It seems like all the checks look okay now so I've gone ahead and un-drafted the PR.

After merging main to this branch, there are two checks failing, UI Tests (firefox) and the RTD build

@RRosio RRosio changed the title [WIP] Loading custom CSS Load custom CSS Jun 6, 2023
@RRosio RRosio marked this pull request as draft June 6, 2023 09:26
@jtpio
Copy link
Member

jtpio commented Jun 6, 2023

Ah nice looking good!

Curious if the error was relevant? Does the functionality work with the type ignore comment?

@RRosio
Copy link
Collaborator Author

RRosio commented Jun 6, 2023

Curious if the error was relevant? Does the functionality work with the type ignore comment?

I think that it is okay, locally I didn't experience any errors. My thinking for it is that in the function where I am calling this line I use the super() function to rely on the LabServerApp's dependency on the ExtensionAppJinjaMixin which as you mentioned declares the jinja2_env settings.
The original error while linting on the CI Job complains about not being able to detect the type of jinja2_env at the line self.jinja2_env.globals.update(custom_css=self.custom_css). It seems like it then tries to pull stub files, to detect the type(?), but is not able to find the relevant files. Silencing the type checking error doesn't cause a visible loss of the expected functionality in my local environment (from what I understand) so I thought it might be the way to go?

@RRosio RRosio closed this Jun 6, 2023
@RRosio RRosio reopened this Jun 6, 2023
@RRosio
Copy link
Collaborator Author

RRosio commented Jun 6, 2023

Currently, only the RTD CI Job is failing with the following traceback:

python -m pip install --upgrade --upgrade-strategy eager --no-cache-dir ./.[docs]
Processing /home/docs/checkouts/readthedocs.org/user_builds/jupyter-notebook/checkouts/6841
  Installing build dependencies: started
  Installing build dependencies: finished with status 'done'
  Getting requirements to build wheel: started
  Getting requirements to build wheel: finished with status 'done'
  Installing backend dependencies: started
  Installing backend dependencies: finished with status 'done'
  Preparing metadata (pyproject.toml): started
  Preparing metadata (pyproject.toml): finished with status 'error'
  error: subprocess-exited-with-error
  
  × Preparing metadata (pyproject.toml) did not run successfully.
  │ exit code: 1
  ╰─> [20 lines of output]
      INFO:hatch_jupyter_builder.utils:Running jupyter-builder
      INFO:hatch_jupyter_builder.utils:Building with hatch_jupyter_builder.npm_builder
      INFO:hatch_jupyter_builder.utils:With kwargs: {'build_cmd': 'build:prod', 'editable_build_cmd': 'build', 'source_dir': 'packages', 'build_dir': 'notebook/static', 'npm': 'jlpm'}
      INFO:hatch_jupyter_builder.utils:No build required
      Traceback (most recent call last):
        File "/home/docs/checkouts/readthedocs.org/user_builds/jupyter-notebook/envs/6841/lib/python3.9/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 353, in <module>
          main()
        File "/home/docs/checkouts/readthedocs.org/user_builds/jupyter-notebook/envs/6841/lib/python3.9/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 335, in main
          json_out['return_val'] = hook(**hook_input['kwargs'])
        File "/home/docs/checkouts/readthedocs.org/user_builds/jupyter-notebook/envs/6841/lib/python3.9/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 152, in prepare_metadata_for_build_wheel
          whl_basename = backend.build_wheel(metadata_directory, config_settings)
        File "/tmp/pip-build-env-pqx0sq25/overlay/lib/python3.9/site-packages/hatchling/build.py", line 56, in build_wheel
          return os.path.basename(next(builder.build(wheel_directory, ['standard'])))
        File "/tmp/pip-build-env-pqx0sq25/overlay/lib/python3.9/site-packages/hatchling/builders/plugin/interface.py", line 150, in build
          build_hook.initialize(version, build_data)
        File "/tmp/pip-build-env-pqx0sq25/normal/lib/python3.9/site-packages/hatch_jupyter_builder/plugin.py", line 93, in initialize
          ensure_targets(config.ensured_targets)
        File "/tmp/pip-build-env-pqx0sq25/normal/lib/python3.9/site-packages/hatch_jupyter_builder/utils.py", line 237, in ensure_targets
          raise ValueError(msg)
      ValueError: Ensured target "notebook/labextension/static/style.js" does not exist
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

× Encountered error while generating package metadata.
╰─> See above for output.

note: This is an issue with the package mentioned above, not pip.
hint: See above for details.

It doesn't seem like any of the changes introduced in this PR would cause this issue, but I can see that it is not typical for this failure to appear.

The line right before the error is INFO:hatch_jupyter_builder.utils:No build required and the error itself indicates to me that the /notebook/labextension directory where the Notebook extension build should be, is not present. I'm not sure what causes hatch to recognize whether a build is required or not...

@jtpio
Copy link
Member

jtpio commented Jun 7, 2023

Hmm that's strange. I triggered a new CI run on #6887 and the docs build seems to be passing there.

@@ -26,6 +26,10 @@ and editing settings is similar for all the Jupyter applications.
> - [traitlets](https://traitlets.readthedocs.io/en/latest/config.html#module-traitlets.config)
> provide a low-level architecture for configuration.

### Disabling Custom CSS
Copy link
Member

Choose a reason for hiding this comment

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

As a follow-up (could be another PR), maybe it could be interesting to document the feature more (for example in the user guide), to show an example of such custom.css file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you @jtpio I can follow this PR up with one that includes the example!

@RRosio
Copy link
Collaborator Author

RRosio commented Jun 8, 2023

I believe the reason for the RTD build failing was due to me adding the placeholder CSS file in the notebook/static/custom and since notebook/static is the build directory for hatch-jupyter-builder, it was taking the existence of that directory to mean that there was no need to build the package. I am mitigating this by changing the directory where I put the placeholder CSS file to be /notebook/custom/custom.css. Not sure if this is the best way to approach that so I am open to suggestions. I am un-drafting this as well!

@RRosio RRosio marked this pull request as ready for review June 8, 2023 01:23
@jtpio
Copy link
Member

jtpio commented Jun 8, 2023

There is indeed some logic in the hatch-jupyter-builder plugin to ensure some target files have are available, but not sure these would be the cause of the issue above:

notebook/pyproject.toml

Lines 165 to 168 in 245bc87

ensured-targets = [
"notebook/labextension/static/style.js",
"notebook/static/bundle.js"
]

@jtpio
Copy link
Member

jtpio commented Jun 8, 2023

Just tried from this branch and it seems to be working well!

I created .jupyter/custom/custom.css with the following content for testing:

#top-panel-wrapper, #jp-top-bar {
  background-color: blue!important;
}

And then simply refreshed the page to see the changes:

image

@RRosio
Copy link
Collaborator Author

RRosio commented Jun 8, 2023

There is indeed some logic in the hatch-jupyter-builder plugin to ensure some target files have are available, but not sure these would be the cause of the issue above:

notebook/pyproject.toml

Lines 165 to 168 in 245bc87

ensured-targets = [
"notebook/labextension/static/style.js",
"notebook/static/bundle.js"
]

Yeah I am not entirely sure what is going on but the RTD build with only the addition of the placeholder file was failing: https://readthedocs.org/projects/jupyter-notebook/builds/20949880/. Removing that or changing the position of the file has been allowing the RTD build to pass consistently...
The two lines right before the error were what led me to thinking about the directory since they mention it

INFO:hatch_jupyter_builder.utils:With kwargs: {'build_cmd': 'build:prod', 'editable_build_cmd': 'build', 'source_dir': 'packages', 'build_dir': 'notebook/static', 'npm': 'jlpm'}
INFO:hatch_jupyter_builder.utils:No build required

I thought maybe it was getting stuck at the following build hook?

[tool.hatch.build.hooks.jupyter-builder.build-kwargs]
build_cmd = "build:prod"
editable_build_cmd = "build"
source_dir = "packages"
build_dir = "notebook/static"
npm = "jlpm"

Copy link
Member

@jtpio jtpio left a comment

Choose a reason for hiding this comment

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

Thanks!

@jtpio
Copy link
Member

jtpio commented Jun 9, 2023

Ah looks like there is a small conflict to fix. Otherwise this should be good to go 👍

@RRosio
Copy link
Collaborator Author

RRosio commented Jun 9, 2023

Thank you @jtpio! I have just resolved that conflict!

@jtpio
Copy link
Member

jtpio commented Jun 9, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants