Skip to content

qml: refactor color system

Pierre Lamot requested to merge chub/vlc:qml/color-refactor into master

First I really apologize for the humongous patch, I know that it's tedious to review, hopefully that's a pretty boring patch.

Provide contextual color

The main idea is to provide objects that can be instantiated if the different component that requires color theme.

Theses Contexts defines what palette they use (the standard one of the dark one in the player), what color set they should use (is it a button, a slider, etc..), and the state of the widget (hovered, focused, pressed, disabled). The context defines some color properties that may be usable in the current widgets (different colors for foregrounds, backgrounds, and decorations)

For the defaults theme provided by our designers, this allows to have fine tuned colors per component and to ensure that every widgets are homogeneous as we ensure that widgets of the same kind use the same colorset.

For the system themes, it maps fairly well to the palettes:

How actual color would be determined

  • For each color, we build a key representing the color, the key is build from the color set (button, window, view, tabbutton, etc..), the section (background/foreground), the name (primary/secondary/...) and the state (normal, hovered, focused, pressed, disabled).

  • all colors are stored in a map associating the color key to its value

  • When a color is required for a particular context, we look in the table for the key. There is a fallback mechanism, if the key doesn't exists for a given state, we try to rebuild the key for the Normal state. then if the key doesn't exist for this component we rebuild the key for the View component (first with the actual state then with the Normal State). if every thing fails we return a crappy color (magenta) to visually indicate that something needs to be fixed.

  • On the QML Side, we instantiate a ColorContext object for each component we want to theme, and we extract colors from it. there are 3 main sets of colors:

    • fg for foreground colors, the sub colors are primary (the main color), secondary (for component that requires a second color), hightlight (for selection), link (for links), positive/neutral/

    • bg expose the same set of color but for background

    • decoration colors. theses are directly accessible in the ColorContext object. border, separator, accent, shadow and visualFocus

    ColorContext have a palette property that defines which palette should be use (dark palette for the player or default palette), a colorset to define what is the current color set (Button/View/Item/Slider/etc...) and some state (enabled, focused, hovered, pressed)

    When a color change due to either a state change or a palette change, the color property change is signaled and the color will be changed through property bindings

Sample usage:

Button {
    id: control

    //usually expose the ColorContext as a property, so parents can
    //override properties of the context
    readonly property ColorContext colorContext: ColorContext {
        id: theme

        //palette is usually intherited
        //palette: VLCStyle.palette

        //indicate which set of color should be applied
        colorSet: ColorContext.ToolButton

        //state of the widget
        enabled: control.enabled
        hovered: control.hovered
        focused: control.visualFocus
        pressed: control.down
    }

    color: theme.bg.primary

    Widgets.BodyLabel {
        color: theme.fg.primary
    }

    Widgets.BodyLabel {
        color: theme.fg.secondary
    }
}

Automatic ColorContext inheritance

In order to avoid passing every parameter though down to each children (the palette and colorset), I implemented an automatic property inheritance for the color context. If a property is not defined (either the palette, the colorset or the state) it will try to find a parent that defines this property and inherits from it, in other words it tries to find the first parent that have a ColorContext child, and use its properties for what have not been defined explicitly in the local Context. When a parent context is updated, changes are signaled to the children context that updates themselves accordingly then notify their own children

FAQ

  • Why using the inheritance mechanism ? is reliable ? is it expensive ?

    My first implementation used explicit parameter passing from top to bottom. I thought that this was a bit annoying to use in practice.

    In practice, the hierarchy is browsed once (in both sides) when the object is instantiated and when Objects are re-parented. The colorset is never changed, state is made explicit upon creation, and the palette changes (when switching theme in the player or when the user changes its preferences) need to be propagated anyway.

  • Are there some gotcha with this inheritance system ?

    The inheritance works by looking up in the QQuickItem tree, so if nodes are not parented to a QQuickItem, the inheritance will be broken. I found two cases where this happens: in Popups (popups are not QQuickItem), and when using ObjectModel

    the other pitfall is that the inheritance expect Items to have only one ColorContext, if your Item needs to use different colorset, they shouldn't be put at the same level.

  • Can this be done with attached properties ?

    I tried, but attached properties objects are only instantiated when a property is assigned, so if you only read colors in an object, this didn't worked properly, this also tend to create many unnecessary ColorContext objects

  • Why not having a state for checked, selected, indeterminate, etc...

    theses state are orthogonal to the disablednormalfocus/hoverpressed cycle. Handling them in the color context would probably lead to a state combination explosion, and be more complex to handle in a generic manner. I preferred to keep it simple and handle these at the widget level using secondary/highlight/etc.. colors

  • XXX widget should have its own color set. We should have primary/error/xxx colors for border too. We should differentiate branding and accent colors etc...

    Yeah probably this is not meant to be definitive

Edited by Pierre Lamot

Merge request reports