D13232: introduce Custom color set

[Kai Uwe Broulik]

broulik added inline comments.

INLINE COMMENTS

> kirigamiplugin.cpp:172
>  
> +    //qmlRegisterUncreatableType<Kirigami::ThemeOverride>(uri, 2, 5, "ThemeOverride", "Cannot create objects of type ThemeOverride, use it as an attached poperty");
> +

What's this?

> basictheme.cpp:138
> +                        //TODO: primary, accent and background
> +                        QMetaObject::invokeMethod(basicThemeDeclarative()->instance(this), "__propagateTextColor", Q_ARG(QVariant, QVariant::fromValue(this->parent())), Q_ARG(QVariant, textColor()));
> +                        QMetaObject::invokeMethod(basicThemeDeclarative()->instance(this), "__propagateBackgroundColor", Q_ARG(QVariant, QVariant::fromValue(this->parent())), Q_ARG(QVariant, backgroundColor()));

Probably prints warnings for themes that don't have this?

> platformtheme.cpp:314
> +#define PROPAGATECUSTOMCOLOR(colorName, color)\
> +          if (colorSet() == Custom) {\
> +              for (PlatformTheme *t : d->m_childThemes) {\

Can this lead to issues with non-deterministic setting of properties? Ie. say `Kirigami.Theme.textColor` is evaluated before `Kirigami.Theme.colorSet: Kirigami.Theme.Custom`?

> platformtheme.h:79
>       */
> -    Q_PROPERTY(QColor textColor READ textColor NOTIFY colorsChanged)
> +    Q_PROPERTY(QColor textColor READ textColor WRITE setTextColor NOTIFY colorsChanged)
>  

Do they need a `RESET`?

> platformtheme.h:220
>      void inheritChanged(bool inherit);
> +    void colorOverridesChanged(const QJsonObject &overrides);
>  

Where is this used?

REPOSITORY
  R169 Kirigami

REVISION DETAIL
  https://phabricator.kde.org/D13232

To: mart, #kirigami, broulik
Cc: plasma-devel, apol, davidedmundson, mart, hein
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180601/7325bbbb/attachment.html>