FEATURE: User selectable color schemes (PR #10544)

This lets administrators mark color schemes as user_selectable, which then are displayed as options in the user’s Preferences > Interface screen. Users can pick a regular color scheme and (if dark color schemes are available) a dark mode color scheme.

screencast 2020-08-21 14-52-30

Behind the scenes this PR:

  • adds a user_selectable column to the color_schemes table
  • adds a color_scheme_id column to user_options (a dark_scheme_id column was already added a few weeks ago)
  • allows the regular/dark scheme preference to be stored in a cookie or in the user_options table (similar to theme/text size choices)
  • adds a color-scheme-picker.js library that can be used to build similar widgets in plugins/theme components (for example, to toggle color scheme choices in the hamburger menu)
  • adds a new color-scheme-stylesheet route that facilitates previewing users’ color scheme choices on-the-fly


Why is this set up to do a no-op here?

I’m mostly concerned about the two set operations that do nothing. Otherwise this is in pretty good shape.

You should be able to say this.session.userColorSchemeId without importing it.

pretty sure you can one line it:

showColorSchemeSelector: reads(“userSelectableColorSchemes.length”)

do we need the get here ?

selectedColorSchemeNoneLabel: i18n(“user.color_schemes.default_description”),

think we can get rid of the get here

findBy(“id”, this.themeId)

It’s a nitpick and up to you, but I tend to avoid creating a variable in this case, I think the fact that you have to grep for two cases instead of one now largely surpasses the 5 chars

Yeah, no good reason. My goal was to have selectedColorSchemeId take the value from the session as its default without updating the source, but it wasn’t done right.

I have refactored this now, with help from @jjaffeux, it’s much cleaner, the values are set in the controller’s init(). Another nicety: the session values don’t need to be transiting via the preferences-interface route.