Dark themes

[thomascastleman]

This pull request adds support for several dark themes for the CPO editor. These are accessible from a new dropdown in the Bonnie menu.

The themes we've included so far are: base16-dark, material-darker, monokai, and panda-syntax. (these have been adapted from this list: https://codemirror.net/demo/theme.html)

Format of themes

The theme stylesheets can be found in /src/web/css/themes. We've designed the themes in a particular format, using CSS variables to make it easy to change/add new themes. There are three parts to a theme:

1. Color palette: Variables that supply "friendly" names for the color codes used by this theme.
2. Color assignments: This assigns specific variable names (used in overrides to control usually several elements of the editor) to colors (from the palette).
3. Overrides: These styles reference the variables from the color assignments to re-style the editor in a consistent way. (at the moment these are identical across all themes)

Creating a new theme

1. Copy template.css into a new file, giving it the name of your theme.
2. Replace all instances of "template" with your new theme's name
3. At the top, add to the color palette the colors you want to use, giving them friendly names
4. Fill in the color assignments, deciding which colors should be used for what parts of the editor/syntax.
5. Add a <link> to your sheet in src/web/editor.html, and an <option> to the theme selector (#theme-select).
6. Done! (you don't need to edit the overrides unless you want to alter the groups of elements that are styled by a particular variable)

Persistence

When a user sets the theme, it is saved in localSettings, so CPO will remember which theme you used last so you don't have to keep switching to it.

[blerner]

This looks pretty good! Some further improvements that would make it better yet:

• Modifying editor.css to use these variables as well (basically, make the default colors be a theme, themselves)
• Saving the current theme works well. But when you reload CPO, it starts off in the default theme before snapping to the previously selected theme. I wonder if there's an earlier moment in the startup sequence where you could call your applyTheme function -- @jpolitz I'm not sure when that would be, though, do you have any ideas? It would need to be basically before we load cpo-main.jarr, since that takes a huge amount of time. I guess maybe when we create the spinning Bonnie logo? But does our localStorage exist yet?
• Fixing the brightness of highlighted spans in error messages. This is controlled by output-ui.js, and that 74 is the lightness of the color. Now, 74 is just too bright for your lighter-colored text on a dark background, but making it a darker level (e.g. 40) is too dark for the default theme:

lightness\theme	Light theme	Dark theme
40 brightness
74 brightness

To handle this, you'll need to tweak output-ui and/or repl-ui to be aware of themes, and know which lightness level to use. (@jswrenn Do you happen to know what the effective range of valid lightness levels are? It seems to be quite finicky (74 works, 75 doesn't!), and I don't understand why...) We need to think about how to do this well, since it will require communicating between cpo-main.js (where applyTheme lives) and repl-ui (where the call to outputUI.makePalette() lives) and output-ui (which will need to know what lightness level to use)...

• Images with transparent backgrounds look pretty weird now, and some of them are generated by Pyret rather than by students: try running

  include chart
  from-list.box-plot([list: [list: 1, 2, 3, 4], [list: 1, 2, 3, 4, 5], [list: 10, 11]])

  in various themes to see what I mean. Note that when you use a darker theme and click on the image (to pop open a dialog), that dialog still has a light background, and so the image is legible again. @jpolitz do we want to enforce an opaque background on the thumbnail image here (and possibly on all thumbnail images?), or do we want to dark-theme the popup window too?

[schanzer]

Why not at least turn the theme on by default for browsers that have been set to dark mode?

[blerner]

Because there are multiple themes, not just "the theme", and because for consistency's sake, we want the teacher to be able to say "Does everyone see this?" Let students explore the Bonnie menu and play with themes, but let's not make that the default thing they see.

[thomascastleman]

Sounds good and I'll wait to see what Joe has to say about those.

One thing about default theme though, is I would recommend against trying to define it in terms of the theme template. The overrides in the template are sort of geared toward dark themes (i.e. check block backgrounds are set to a shade of the background color, while the text color is set by another variable; for that matter, several things are reduced to a shade of the background color, where in default theme they would be various other colors).

As such, it might not be worth trying to adapt the default theme to the template format if we like the way it looks presently. What are your thoughts @blerner ?

[blerner]

I think parts 2 and 3 of your template (defining friendly variable names, and defining colors solely in terms of those variables) are useful. Essentially, it gives us a sanity check that if ever we want to specify a color literal in the editor.css file, we should reconsider, and see if there's a color variable that we should use instead.

[blerner]

Follow up from #335: When looking at the <body> class attribute, though: on the very first load of CPO into the browser, I see <body class style>, and in particular there is no theme class chosen, not even "default". You can repro this by loading CPO into an incognito window in both Chrome or Firefox. I think you're just missing a check for "if the theme is undefined, make it default"

[thomascastleman]

Here are some updated screenshots from the latest updates to code snippets under highlights. Readability has been improved:

Code in unfocused errors stays light, but highlighted text darkens to contrast with the added background.

When code is highlighted, a darker version of the syntax highlighting from the theme is applied:

As an aside, @blerner the build has been breaking (since d6d5486) and I'm not sure why--it seems to error on npm install -g heroku-cli, but I'm not sure how that would have been affected by what I've edited.

[blerner]

Seems to be a travis caching issue; I've cleared the cache (I think) and am restarting the build. I don't think it's anything to do with your code.

[blerner]

@jpolitz have you seen this sort of error before? I searched and found https://github.com/nodejs/help/issues/2874, but I tried clearing the cache and it didn't work...

[jpolitz]

I'm not sure, but this step of the build succeeds on horizon. I notice that the style-updates branch is significantly behind horizon; is that intentional? It has a number of conflicts with it, so I'm concerned about how that will shake out long-term.

[blerner]

It's not intentional. Realistically, the changes here are fairly self-contained (though they do touch a lot of editor.css, naturally), and could probably be rebased onto a more horizon without too much difficulty.

[blerner]

@thomascastleman can you try that? If not, I can't today, but hopefully will have time later in the week.

[jpolitz]

It looks like this is an issue that's new in the master build that is fixed on horizon. I'll check horizon's travis/package.json history to see if there's an obvious fix.

[thomascastleman]

@blerner I tried rebasing style-updates onto horizon but there were a few conflicts in places I didn't edit and wasn't sure how to resolve--maybe you could have a look at it?

[blerner]

I wasn't able to get a clean rebase, so I did a merge instead. Not ideal, but it'll do.

[blerner]

An additional readability issue in error highlighting, for all the dark themes:

The green strings are roughly impossible to read against the highlights. One possibility is to add a (non-standard, but pretty well supported) CSS property:

body:not(.default) .CodeMirror .bg-highlighted.cm-string {
  -webkit-text-stroke: 0.15px black;
}

which produces instead of

Or, we could use text-shadow, which is standardized and supported, to simulate it:

body:not(.default) .CodeMirror .bg-highlighted.cm-string {
  text-shadow: -0.25px 0.25px 0px gray, 0.25px 0.25px 0px black, 0.25px -0.25px 0px gray, -0.25px -0.25px 0px black
}

which produces

[sorawee]

I somehow find the current green string easier to read than the text-stroke/text-shadow-ed variant.

[blerner]

Remember that the highlights you get are randomly colored, so be sure to try it a bunch of times to decide. Also, the screenshots above are 2x normal resolution (because hidpi is still weird for computers to get right), and at larger resolutions I agree with you. But when the characters are only ~12px tall, the green-on-bluegreen combos are really hard to read.

[thomascastleman]

@blerner Another possibility would be to significantly darken the string itself while it's under highlights, seen below:

This is 12% lightness, so I could even darken it more if you think it's still not readable enough. I'd vouch for this fix though because the outline/text-shadow appears to give the strings a "fuzzy" look at 14px size on my machine.

What do you think?

[sorawee]

I think this is much better than outline/text-shadow, due to the absence of "fuzzy" look as you described.

[blerner]

Darkening strings further is fine by me.

[jpolitz]

The horizon rebase/merge looks clean to me now. I named the default style "Ensign", which is cute because it's a name for a junior officer and also the name of the flag flown on a seafaring vessel (https://en.wikipedia.org/wiki/Ensign).

[jpolitz]

Going to deploy this on horizon now, monitor and try it out a bit there, then we can release later this week.

[jpolitz]

Great work @thomascastleman & co