D25590: support for user removing background and automatic shadow

[David Edmundson]

davidedmundson added subscribers: ndavis, davidedmundson.
davidedmundson requested changes to this revision.
davidedmundson added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> colorscope.cpp:52
>  
> -    QQuickItem *parentItem = qobject_cast<QQuickItem *>(parentObject);
> -    if (parentItem) {
> -        connect(parentItem, &QQuickItem::parentChanged, this, &ColorScope::checkColorGroupChanged);
> -    } else if (m_parent) {
> -        m_parent->installEventFilter(this);
> +    if (parentObject && qobject_cast<QQuickItem *>(parentObject)) {
> +        connect(static_cast<QQuickItem *>(parentObject), &QQuickItem::parentChanged,

It' be better to split this colour change stuff.

It's effectively an unrelated bugfix/cleanup

> colorscope.cpp:110
> +        if (!s) {
> +            // Make sure QppletInterface always has a ColorScope
> +            s = static_cast<ColorScope *>(qmlAttachedPropertiesObject<ColorScope>(candidate, qobject_cast<PlasmaQuick::AppletQuickItem *>(candidate)));

typo

> configuration-icons.svg:17
>     version="1.1"
> -   inkscape:version="0.48.5 r10040"
> -   sodipodi:docname="configuration-icons.svgz">
> +   inkscape:version="0.92.2 5c3e80d, 2017-08-06"
> +   sodipodi:docname="configuration-icons.svg">

@ndavis can you review this icon change please

> plasma.h:284
> +        ShadowBackground = 4, /**< The applet won't have a svg background but a drop shadow of its content done via a shader */
> +        ImmutableBackground = 8, /** The user shouldn't have the possibility to  */
>          DefaultBackground = StandardBackground /**< Default settings: both standard background */

the user shouldn't have the possibility to.....

> appletinterface.cpp:201-202
> +    int value = hintEnum.keyToValue(hintsString.constData());
> +    if (value > 0) {
> +        m_effectiveBackgroundHints = Plasma::Types::BackgroundHints(value);
> +        m_effectiveBackgroundHintsInitialized = true;

0 == NoBackground which is a perfectly valid entry

I think we need to use the
keyToValue(QString, bool*) overload and check the return value.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: mart, #plasma, davidedmundson
Cc: davidedmundson, ndavis, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20191128/2512e741/attachment-0001.html>