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 functionality and documentation #14743

Merged
merged 24 commits into from Nov 29, 2023

Conversation

RRosio
Copy link
Contributor

@RRosio RRosio commented Jun 23, 2023

References

Fixes #12318
Related to #8457

Code changes

The code adds a link to the custom CSS file in the template html page and a handler to either serve the user's local custom CSS file or the placeholder file. There is also a flag added to allow disabling the default behavior that loads custom CSS.

User-facing changes

Users can apply custom styling. I tested this PR by adding some color to the Toolbar etc.

jupyter_styles_custom_css

Backwards-incompatible changes

None applicable that I can think of.

@jupyterlab-probot
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@github-actions github-actions bot added documentation tag:CSS For general CSS related issues and pecadilloes Design System CSS labels Jun 23, 2023
jupyterlab/handlers/custom_css_handler.py Fixed Show fixed Hide fixed
jupyterlab/labapp.py Fixed Show resolved Hide resolved
@jtpio jtpio added this to the 4.1.0 milestone Jun 23, 2023
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks for upstreaming this @RRosio

This is a great start but looking at the classical notebook code, I have a couple of questions.

I also think this resolves #14319 but not fully #8457 (that one will requires also adding data attributes on cells).

jupyterlab/labapp.py Outdated Show resolved Hide resolved
jupyterlab/labapp.py Fixed Show resolved Hide resolved
jupyterlab/labapp.py Outdated Show resolved Hide resolved
RRosio and others added 5 commits June 29, 2023 11:37
Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>
Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>
jupyterlab/labapp.py Outdated Show resolved Hide resolved
Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

Thanks @RRosio

To make things easier, I pushed two commits to

  • Fix the galata tests
  • Use those to produce the documentation
  • Fix the Python test
  • Switch the default behavior to not load custom.css

To explain how I did the integration test,

  1. I'm activating the feature when starting the server only for the documentation test through the command line option.
  2. I capture the custom/custom.css request to return a mocked content.

Copy link
Member

Choose a reason for hiding this comment

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

The lack on contrast cause by these shadows really makes your eyes burn ;)

Copy link
Member

@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.

Thank you @RRosio and @fcollonval! This looks good to me.

@krassowski
Copy link
Member

krassowski commented Nov 27, 2023

The test failure will need to be addressed before merging though - it looks related to different new line encoding:

FAILED jupyterlab/tests/test_custom_css_handler.py::test_CustomCssHandler - AssertionError: assert 'body #top-pa...derline;\r\n}' == 'body #top-pa...underline;\n}'
  - body #top-panel-wrapper,
  + body #top-panel-wrapper,

  ?                         +
  - #jp-top-bar {
  + #jp-top-bar {

  ?              +
  -   background-color: #aecad4 !important;
  +   background-color: #aecad4 !important;

  ?                                        +
  - }
  + }

  ?  +
  - 
  + 

  - body h1 {
  + body h1 {

  ?          +
  -   font-size: 22px;
  +   font-size: 22px;

  ?                   +
  -   margin-bottom: 40px;
  +   margin-bottom: 40px;

  ?                       +
  -   color: #10929e;
  +   color: #10929e;

  ?                  +
  -   text-decoration: underline;
  +   text-decoration: underline;

  ?                              +
    }

Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com>
@krassowski
Copy link
Member

krassowski commented Nov 29, 2023

    [documentation] › test/documentation/custom_css.test.ts:15:7 › Use custom CSS layout › should apply custom CSS to main page 
    [documentation] › test/documentation/custom_css.test.ts:33:7 › Use custom CSS layout › should apply custom CSS to h1 headings 

The failures seem related - the snapshots were not updated. I am doing it manually right now.

Actual Expected
image image

@fcollonval
Copy link
Member

The snapshots discrepancies are due to the branch not being up-to-date with the main branch I think.

@RRosio
Copy link
Contributor Author

RRosio commented Nov 29, 2023

The snapshots discrepancies are due to the branch not being up-to-date with the main branch I think.

Would it be okay to update the branch then?

@fcollonval
Copy link
Member

Would it be okay to update the branch then?

Now that Mike updated manually the snapshots it should not be needed.

@krassowski
Copy link
Member

Now that you said it, I decided to merge main anyways just to be sure.

@RRosio
Copy link
Contributor Author

RRosio commented Nov 29, 2023

One of the visual regression documentation tests is listed as flaky now rather than failing:

[documentation] › test/documentation/custom_css.test.ts:33:7 › Use custom CSS layout › should apply custom CSS to h1 headings 

I saw a few other PRs with some of the same failures so I'm under the impression that the other test failures do not seem to be related?

@krassowski
Copy link
Member

Remaining failures are unrelated, merging - thanks again!

@krassowski krassowski merged commit 7622f44 into jupyterlab:main Nov 29, 2023
75 of 77 checks passed
chisangajm3 added a commit to chisangajm3/jupyterlab that referenced this pull request Dec 18, 2023
* upstream/main: (7628 commits)
  Adopt ruff format (jupyterlab#15499)
  Bump scipy from 1.11.3 to 1.11.4 (jupyterlab#15474)
  [pre-commit.ci] pre-commit autoupdate (jupyterlab#15491)
  Pin `actions/labeler` to v4 to fix failing CI action (jupyterlab#15496)
  Add npm provenance issue to the list of release postmortems (jupyterlab#15493)
  Fix URLs in debugger-extension (jupyterlab#15462)
  Bump tj-actions/changed-files from 40.0.2 to 40.2.0 (jupyterlab#15471)
  Bump dessant/lock-threads from 4 to 5 (jupyterlab#15472)
  Bump pandas from 2.1.2 to 2.1.3 (jupyterlab#15473)
  Bump actions/github-script from 6 to 7 (jupyterlab#15470)
  Bump matplotlib from 3.7.2 to 3.8.2 (jupyterlab#15475)
  Bump rjsf to 5.13.4 (jupyterlab#15469)
  Don't play with the focus when handling focus event (jupyterlab#15408)
  [ci skip] Publish 4.1.0a4
  Updated light theme visited link colour to make text visible (jupyterlab#15406)
  Load custom CSS functionality and documentation (jupyterlab#14743)
  Added alt descriptions to a few icon and images (jupyterlab#15265)
  More robust galata/UI tests (jupyterlab#15355)
  Fix Shift + L not working in stdin (jupyterlab#15440)
  Upgrade releaser workflows for silent support (jupyterlab#15446)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for custom.css
4 participants