Add settings to override theme font sizes

[telamonian]

References

#6908
telamonian/theme-darcula#4

Since I released a theme a while ago, I have heard a number of very strong opinions about what the correct size is for various fonts. Since there's clearly no one best solution, I think it's about time we added some general font size settings.

Code changes

Various minor changes to ThemeManager and related code.

User-facing changes

There are now settings for changing the font sizes, and corresponding items in the settings menu and the theme palette category. These settings override the font sizes present in the themes. For good measure, I also added items for toggling the scrollbar theming.

Moved theme-related palette items from Settings category to new Theme category.

Backwards-incompatible changes

None

[jupyterlab-dev-mode]

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link:

[telamonian]

I'm reasonably happy with this PR as is. Paging @ian-r-rose could you please give this a once-over?

[telamonian]

The major issue that needed review (how/where to validate the CSS overrides) has been resolved (by adding "additionalProperties": false to the overrides JSON schema; see #7004).

The PR should be good to go now. Can I please get a fellow dev to look at/sign off on this? The "Increase/Decrease X Font Size" menu items are working quite nicely (eg they're able to read a theme's default settings and start from there, they have a nice behavior if someone sets their font sizes in units of "em", etc).

[telamonian]

...and now for good measure I've added strict validation of the CSS override values (whereas the keys are handled by the JSON schema validation).

@jasongrout @afshin Does my validation implementation seem reasonable?

[jasongrout]

Backwards-incompatible changes

Moved theme-related palette items from Settings category to new Theme category.

Is this a UX change, or an API change? If it is just a UX change, we don't consider it backwards incompatible, and it should be listed in the User-facing changes.

[telamonian]

If it is just a UX change, we don't consider it backwards incompatible, and it should be listed in the User-facing changes.

Makes sense. I only changed the palette items, not the underlying commands, so that should just be a UX change. I've updated the notes in my first post accordingly.

[blink1073]

Code changes look good to me, I am testing this locally now.

[blink1073]

I tried changing the font sizes in the settings editor using em but it had no effect. Should this have worked?

{
    // Theme
    // @jupyterlab/apputils-extension:themes
    // Theme manager settings.
    // *************************************

    // Theme CSS Overrides
    // Override theme CSS variables by setting key-value pairs here
    "overrides": {
        "ui-font-size1": "3em",
        "code-font-size": "14px",
        "content-font-size1": "2em"
    },

    // Scrollbar Theming
    // Enable/disable styling of the application scrollbars
    "theme-scrollbars": true
}

[telamonian]

I think I know where the em setting may have broken. Not sure about the scrollbars toggle. I'll look into it

[telamonian]

@blink1073 Hmm, now I can't reproduce your trouble with the em unit, either. Annoying. Did the px unit work for changing your font sizes?

I copy/pasted the settings you provided, and then inspected the CSS of a filebrowser item in devtools. The font-size is definitely being calculated as I expected (ie as the product of the font size inherited by the element and the magnitude of the em setting). Could you please try your em settings again and then tell me what it says about how the font-size of a filebrowser item is being calculated?

[telamonian]

Ah, crud. It's another set of these bugs. I just tested the PR again in Firefox, and now I see exactly what you mean, @blink1073

And now I'm kicking myself for not testing out the font size changing stuff in multiple browsers. The old scrollbar theming stuff, which is basically being re-used as is, was tested in multiple browsers, so it's weird that that broke.

[telamonian]

I was validating the font size values against the CSS size property, which apparently is not a valid property in Firefox (but it is in Chrome).

I tested a fix on Chrome/Firefox/Safari and pushed it. In Firefox, the scrollbar theming is partially broken: only the horimzontal scroll bar will change color correctly. Some CSS stuff might need some freshening up.

[telamonian]

@blink1073 All of the issues you found should now be fixed, so give the PR another test.

The bugs were partially due to a rotten CSS selector, and partially due to the differences between CSS handling in specific browsers. This raises the question of how to write tests for the theming settings stuff, but that can be another PR

[blink1073]

Works well in Firefox and Chromium on Linux Mint. Thanks!

[blink1073]

This will go into the next RC, which I plan to cut tomorrow.