davidedmundson added a comment.

  I love use of layouts for everything, so in general ++

INLINE COMMENTS

> CompactApplet.qml:34
>      subText: plasmoid.toolTipSubText
> -    location: if (plasmoid.parent && plasmoid.parent.parent.objectName === 
> "hiddenTasksColumn" && plasmoid.location !== PlasmaCore.Types.LeftEdge) {
> -                return PlasmaCore.Types.RightEdge;

When you say it did nothing, You'd need to test every hidden applet whilst 
having the panel on the left.

See D1253 <https://phabricator.kde.org/D1253>

If it ain't broke...

> PlasmoidItem.qml:45
>          if (applet && mouse.button === Qt.LeftButton) {
> -            applet.expanded = true;
> +            applet.expanded = !applet.expanded;
>          }

Huh? how did this work before?

> main.qml:66
>  
> +    // Shouldn't it be part of Qt?
> +    function findParentNamed(object, objectName) {

Not really.
Using objectNames is a bit of an anti pattern, especially when QML has so the 
built-in component scope hierachy.

We use it in the system tray already, and it's arguably no worse than the 
existing applet.parent.parent.

So fine here, but only because the system is mad.

REPOSITORY
  R120 Plasma Workspace

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

To: ratijastk, #vdg, #plasma, broulik, mart, hein
Cc: davidedmundson, ngraham, ndavis, anthonyfieroni, plasma-devel, jraleigh, 
GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

Reply via email to