Inherit terminal theme from lab theme

[gnestor]

This provides the terminal with a theme based on the current lab theme's CSS variables and removes the setting, command, and menu item for toggling the terminal theme light/dark. This simplifies things for the user and has the added benefit of the terminal theme playing nice with themes other than the official light/dark.

Another option: We can provide a new CSS variable or set of variables for the terminal theme. This would allow themes to have a little more control over the terminal theme (vs. defaulting to general variables like --jp-layout-color0 and --jp-font-color0). I'm starting work on refactoring the CSS theme variables (#5549) and this may be a good place to refactor.

[jasongrout]

Cool, thanks for working on this!

[ian-r-rose]

This is cool @gnestor. At some point somebody suggested (I forget who) that we provide three options for the terminal: light, dark, or "inherit from JupyterLab theme". Do you think that would be appropriate here?

I know that I kind of like to have a dark terminal theme, even if I am using the light JupyterLab theme.

[gnestor]

Per @ian-r-rose's suggestion, I added a command that allows users to select light, dark, or inherit terminal theme. It's available from the "Settings" menu and the command palette.

[ellisonbg]

Love this!

[ian-r-rose]

This looks great!

[ian-r-rose]

I guess I'm just naturally a bit conservative about defaults: any reason to change this from "dark"?

[gnestor]

@ellisonbg What do you think?

[ellisonbg]

Good point. What about having another setting of "opposite" (dark terminal on the light theme) and having that as the default? The reason we have had a dark terminal on a light theme previously is to help users understand that it is different from other UIs that look similar (like a test editor).

[gnestor]

True. I'm not a fan on light terminal on dark theme. Give it a try. I'm ok either way 👌

[jasongrout]

What's the current status of this PR? Is there still work that you are planning on doing, or has everything been addressed?

[gnestor]

The only thing to resolve is whether or not the "inherit" (inherit the lab theme's color for the terminal theme) should be the default or whether we should leave "dark" as the default.

[jasongrout]

+1 to inherit by default, but having the option to be dark/light explicitly.

[gnestor]

I agree. Feel free to merge and we can always change the default if need be 👍

[ian-r-rose]

Okay, we can move forwards with this and change the default back to "dark" if people complain.

[gnestor]

Ok 👌

I rebased from master once more so let's see if we can't get these checks to fully pass...

[ian-r-rose]

Looks like legitimate test failures.

[gnestor]

@ian-r-rose We're in the JupyterLab meeting and it sounds like this @jupyterlab/apputils ClientSession #restartKernel() should restart if the user accepts the dialog failure (https://github.com/jupyterlab/jupyterlab/blob/master/tests/test-apputils/src/clientsession.spec.ts#L375-L395) is unrelated to any changes in this PR. Do you concur? If so, then @jasongrout says it's ok to merge.

[ian-r-rose]

I thought I saw terminal failures here.

[gnestor]

🤔

[ian-r-rose]

Yep, just checked, there are legitimate terminal failures.

[gnestor]

I'm not seeing it, can you share a link?

[ian-r-rose]

https://dev.azure.com/jupyterlab/jupyterlab/_build/results?buildId=1244&view=logs

…

On Wed, Apr 3, 2019 at 9:59 AM Grant Nestor ***@***.***> wrote: I'm not seeing it, can you share a link? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5964 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFdoN37i3yewS2Kzf9VXGR7K6S6p4sEHks5vdN38gaJpZM4atCmM> .