Review Request 128181: Remove a lot of duplication between both AppletInterface constructors

[Kai Uwe Broulik]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128181/#review96484
-----------------------------------------------------------



> connect(applet(), &Plasma::Applet::contextualActionsAboutToShow,
>            this, &AppletInterface::contextualActionsAboutToShow);
>
>connect(applet(), &Plasma::Applet::activated,
>        this, &AppletInterface::activated);

This should work with both applet types, though?

> connect(applet(), &Plasma::Applet::configurationRequiredChanged,
>        this, [this](bool needsConfig, const QString &reason) {
>            emit configurationRequiredChanged();
>            emit configurationRequiredReasonChanged();
>        });

So, that's where the configuration required thing comes into play but this doesn't fix the property access which tries to read from appletInterface() which is null becuase it never sets/creates one and the thing in applet() is protected.

Also, can we just use delegating constructors instead? (Not sure we can in frameworks, though)

- Kai Uwe Broulik


On Juni 14, 2016, 9:03 vorm., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128181/
> -----------------------------------------------------------
> 
> (Updated Juni 14, 2016, 9:03 vorm.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> Also the two were slightly out of sync leading to some connections not
> being in each constructor.
> 
> 
> Diffs
> -----
> 
>   src/scriptengines/qml/plasmoid/appletinterface.h 6a64594cd53cecfbc99ad7562706e1a78d60c138 
>   src/scriptengines/qml/plasmoid/appletinterface.cpp df2d642445bdcc81d7fed463cb38c337380ea25b 
> 
> Diff: https://git.reviewboard.kde.org/r/128181/diff/
> 
> 
> Testing
> -------
> 
> previous code.
> 
> 
> FIRST CTOR ONLY  (normal mode)
>     connect(applet(), &Plasma::Applet::contextualActionsAboutToShow,
>             this, &AppletInterface::contextualActionsAboutToShow);
> 
>     connect(applet(), &Plasma::Applet::activated,
>             this, &AppletInterface::activated);
> 
> 
>     connect(this, &AppletInterface::expandedChanged, [=](bool expanded) {
>         //if both compactRepresentationItem and fullRepresentationItem exist,
>         //the applet is in a popup
>         if (expanded) {
>             if (compactRepresentationItem() && fullRepresentationItem() &&
>                 fullRepresentationItem()->window() && compactRepresentationItem()->window() &&
>                 fullRepresentationItem()->window() != compactRepresentationItem()->window() &&
>                 fullRepresentationItem()->parentItem()) {
>                 fullRepresentationItem()->parentItem()->installEventFilter(this);
>             } else if (fullRepresentationItem() && fullRepresentationItem()->parentItem()) {
>                 fullRepresentationItem()->parentItem()->removeEventFilter(this);
>             }
>         }
>     });
> }
> 
> SECOND CTOR ONLY (special comic applet path)
> 
>     connect(applet(), &Plasma::Applet::configurationRequiredChanged,
>             this, [this](bool needsConfig, const QString &reason) {
>                 emit configurationRequiredChanged();
>                 emit configurationRequiredReasonChanged();
>             });
> 
>     }
> 
> DUPLICATED:
>     qmlRegisterType<QAction>();
> 
>     connect(this, &AppletInterface::configNeedsSaving,
>             applet(), &Plasma::Applet::configNeedsSaving);
>     connect(applet(), &Plasma::Applet::immutabilityChanged,
>             this, &AppletInterface::immutabilityChanged);
> 
>     connect(applet(), &Plasma::Applet::userConfiguringChanged,
>             this, &AppletInterface::userConfiguringChanged);
> 
>     connect(applet(), &Plasma::Applet::statusChanged,
>             this, &AppletInterface::statusChanged);
> 
>     connect(applet(), &Plasma::Applet::destroyedChanged,
>             this, &AppletInterface::destroyedChanged);
> 
> 
>     connect(applet(), &Plasma::Applet::titleChanged,
>             this, &AppletInterface::titleChanged);
> 
>     connect(applet(), &Plasma::Applet::iconChanged,
>             this, &AppletInterface::iconChanged);
> 
>     connect(applet(), &Plasma::Applet::busyChanged,
>             this, &AppletInterface::busyChanged);
> 
>     connect(appletScript(), &DeclarativeAppletScript::formFactorChanged,
>             this, &AppletInterface::formFactorChanged);
>     connect(appletScript(), &DeclarativeAppletScript::locationChanged,
>             this, &AppletInterface::locationChanged);
>     connect(appletScript(), &DeclarativeAppletScript::contextChanged,
>             this, &AppletInterface::contextChanged);
> 
>     if (applet()->containment()) {
>         connect(applet()->containment(), &Plasma::Containment::screenChanged,
>         this, &ContainmentInterface::screenChanged);
>     }
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160614/0f7553b6/attachment.html>