Skip to content

Adding jlab theme support #35

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

Merged
merged 1 commit into from
Sep 9, 2019
Merged

Adding jlab theme support #35

merged 1 commit into from
Sep 9, 2019

Conversation

martinRenou
Copy link
Member

@martinRenou martinRenou commented Aug 30, 2019

gridtheme

@jtpio
Copy link
Member

jtpio commented Aug 30, 2019

Looks good!

This will probably need a dependency on the JupyterLab ThemeManager and using the themeChanged signal to be able to switch themes on the fly.

@martinRenou
Copy link
Member Author

Thanks! I'll give it a try :)

@martinRenou
Copy link
Member Author

This will probably need a dependency on the JupyterLab ThemeManager and using the themeChanged signal to be able to switch themes on the fly.

The thing is we want to support the classical Notebook. Do you know if that would play well with this?

@jtpio
Copy link
Member

jtpio commented Aug 30, 2019

Right, maybe handling that in the activate function and initializing the widget with the themeManager if it exists (or listening to the signal there).

If it doesn't exist (in the case of the classic notebook), then it would default to the light theme.

@martinRenou martinRenou changed the title [WIP] Adding jlab theme support Adding jlab theme support Sep 3, 2019
@martinRenou
Copy link
Member Author

Thanks! I added support for changing the theme dynamically, listening to the ThemeChanged signal

theme

@kaiayoung
Copy link
Contributor

Looks good overall! We may want to experiment with the specific colors/styles being used, but that can be done in a follow-up PR. More specifically, I'm thinking of making the header cells more visually distinct from the body cells. I'm also curious if the border outside the grid is something new, I don't recall seeing that before?

@martinRenou
Copy link
Member Author

I'm also curious if the border outside the grid is something new, I don't recall seeing that before?

I think it was there before, only less visible (because of its color I guess) :) It comes from the css/datagrid.css stylesheet on the repo that defines a border around the grid. I don't remember where this comes from, I think I copied it from the phosphorjs datagrid example. I guess it would be prettier if we remove it, what do you think?

@martinRenou
Copy link
Member Author

Actually without the border the header cells does not get have a border anymore
border

This comes from the p-Datagrid CSS class that is defined in PhosphorJS, and we override it using the CSS variables.
Maybe we could make it less visible, but we might want to keep it.

@martinRenou
Copy link
Member Author

martinRenou commented Sep 5, 2019

@kaiayoung Do you like this one better (header background color darker):

test
test1

Or maybe this one (header background color darker with dark borders):

test2
test3

Same as before but without the outside border:

test4
test5

@martinRenou martinRenou merged commit 042f7f8 into jupyter-widgets:master Sep 9, 2019
@martinRenou martinRenou deleted the jlab_theme branch September 9, 2019 07:03
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.

4 participants