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

Reply via email to