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

Force light mode for notebook outputs #1812

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gabalafou
Copy link
Collaborator

@gabalafou gabalafou commented May 14, 2024

If you compare the following screenshots, you will find that the some of the Xarray output is invisible in dark mode.

This PR makes a stronger attempt at forcing notebook outputs into light mode by resetting all of the color variables on the notebook output element to light mode.

Before, screenshots

Xarray light mode Xarray dark mode
xarray-light xarray-dark

@gabalafou gabalafou mentioned this pull request May 14, 2024
15 tasks
@gabalafou gabalafou marked this pull request as draft May 14, 2024 10:31
@gabalafou
Copy link
Collaborator Author

Switching to draft mode while I work out the merge conflicts

@Carreau
Copy link
Collaborator

Carreau commented May 23, 2024

Should there be an upstream issue in xarray as well ? I'm going to guess this also affect Lab users using dark theme, doesn't it ?

@gabalafou
Copy link
Collaborator Author

No, it doesn't affect JupyterLab users using dark theme.

I think this is because JupyterLab's dark mode CSS is more robust for various widgets than our CSS.

See:

@gabalafou
Copy link
Collaborator Author

gabalafou commented May 23, 2024

Should there be an upstream issue in xarray

That said, there are some Xarray CSS rules that are not friendly in dark mode. Here's a screenshot from JupyterLab:

The color of the text "10 20" and the file icon is way too dark to contrast well with a dark background.

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

Successfully merging this pull request may close these issues.

2 participants