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

Desktop: Add solarized themes #1733

Merged
merged 1 commit into from Jul 21, 2019

Conversation

@ShaneKilkelly
Copy link
Contributor

commented Jul 14, 2019

Description

This PR adds two themes to the desktop client: Solarized Light and Solarized Dark.

Solarized homepage: https://ethanschoonover.com/solarized/

Screenshots

Solarized Light

Screenshot from 2019-07-14 09-55-15

Screenshot from 2019-07-14 09-55-45

Solarized Dark

Screenshot from 2019-07-14 09-56-27

Screenshot from 2019-07-14 09-57-04

Testing

I've tested the desktop client, but I couldn't get the android toolchain installed to test the mobile client.

Color Palette

For reference, this is the solarized color palette, in pseudo-css form:

solarized {
    Base03:  #002b36;  // dark background
    Base02:  #073642;
    Base01:  #586e75;
    Base00:  #657b83;  // light text
    Base0:   #839496;
    Base1:   #93a1a1;  // dark text
    Base2:   #eee8d5;
    Base3:   #fdf6e3;  // light background
    Yellow:  #b58900;
    Orange:  #cb4b16;
    Red:     #dc322f;
    Magenta: #d33682;
    Violet:  #6c71c4;
    Blue:    #268bd2
    Cyan:    #2aa198
    Green:   #859900
}

@ShaneKilkelly ShaneKilkelly force-pushed the ShaneKilkelly:sk-add-solarized-themes branch from 43489f7 to ec18c0f Jul 14, 2019

@ShaneKilkelly

This comment has been minimized.

Copy link
Contributor Author

commented Jul 14, 2019

⚠️ Squashed and force-pushed to tidy up git history

@laurent22

This comment has been minimized.

Copy link
Owner

commented Jul 14, 2019

That's brilliant, thanks. I'll do some tests with it and get back to you.

@tessus

This comment has been minimized.

Copy link
Collaborator

commented Jul 15, 2019

@ShaneKilkelly don't worry about the git history. When we merge PRs we squash commits anyway.

@tessus tessus added the desktop label Jul 15, 2019

@ShaneKilkelly

This comment has been minimized.

Copy link
Contributor Author

commented Jul 15, 2019

@tessus thanks, good to know :)

@laurent22 laurent22 merged commit 3d498e7 into laurent22:master Jul 21, 2019

@laurent22

This comment has been minimized.

Copy link
Owner

commented Jul 21, 2019

Looks good, thanks @ShaneKilkelly! Only small issue I noticed is that you didn't implement the depthColor property (which gives a different colour depending on the depth of a sub-notebook), and that creates an error. I'm going to make it optional for now, but feel free to create another PR with this property if you want to.

@tessus

This comment has been minimized.

Copy link
Collaborator

commented Jul 21, 2019

IMO depthColor should not be optional. Without it a theme is not complete.

@ShaneKilkelly

This comment has been minimized.

Copy link
Contributor Author

commented Jul 22, 2019

@laurent22 thanks! I'll take a look at the depthColor setting and open another PR.

@laurent22

This comment has been minimized.

Copy link
Owner

commented Jul 29, 2019

IMO depthColor should not be optional. Without it a theme is not complete.

It's true, but actually it's the theme logic that wasn't right. Normally all themes should inherit from the light theme, and just override the relevant properties. I've changed it like this, so it means if the theme doesn't set depthColor, a default would be used instead of crashing as before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.