[SystemTray] Bunch of fixes and improvements in QML

Summary:
Few unrelated fixes, bunched together:
* move some code from main.qml to PlasmoidItem where it belongs
* easier to understand item size calculation
* where possible, replace parent.parent comparisons with a property in AbstractItem
* get rid of marginHints
* Qt 5.14 compatibility fix in configuration
* formatting etc

Test Plan: Everything should work as before, this is just refactoring.

Reviewers: #plasma_workspaces, #plasma, davidedmundson, ngraham, broulik

Reviewed By: ngraham

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D27085

applets/systemtray/package/contents/applet/CompactApplet.qml

@@ -31,7 +31,7 @@ PlasmaCore.ToolTipArea {
     icon: plasmoid.icon
     mainText: plasmoid.toolTipMainText
     subText: plasmoid.toolTipSubText
-    location: if (plasmoid.parent && plasmoid.parent.parent.objectName === "hiddenTasksColumn" && plasmoid.location !== PlasmaCore.Types.LeftEdge) {
+    location: if (plasmoid.parent && plasmoid.parent.inHiddenLayout && plasmoid.location !== PlasmaCore.Types.LeftEdge) {
                 return PlasmaCore.Types.RightEdge;
               } else {
                 return plasmoid.location;

applets/systemtray/package/contents/ui/ConfigEntries.qml

@@ -179,14 +179,14 @@ ColumnLayout {
                             return 0
                         }
 
-                        property var currentValue: model[currentIndex].value
+                        property var myCurrentValue: model[currentIndex].value
 
                         onActivated: {
                             var shownIndex = cfg_shownItems.indexOf(itemId)
                             var hiddenIndex = cfg_hiddenItems.indexOf(itemId)
                             var extraIndex = cfg_extraItems.indexOf(itemId)
 
-                            switch (currentValue) {
+                            switch (myCurrentValue) {
                             case "auto":
                                 if (shownIndex > -1) {
                                     cfg_shownItems.splice(shownIndex, 1)
@@ -257,7 +257,7 @@ ColumnLayout {
                         Component.onCompleted: itemsList.keySequenceColumnWidth = Math.max(implicitWidth, itemsList.keySequenceColumnWidth)
 
                         visible: isPlasmoid
-                        enabled: visibilityComboBox.currentValue !== "disabled"
+                        enabled: visibilityComboBox.myCurrentValue !== "disabled"
                         keySequence: model.applet ? model.applet.globalShortcut : ""
                         onKeySequenceChanged: {
                             if (model.applet && keySequence !== model.applet.globalShortcut) {

applets/systemtray/package/contents/ui/ExpandedRepresentation.qml

@@ -98,8 +98,8 @@ ColumnLayout {
 
         PlasmaComponents.ToolButton {
             id: pinButton
-            Layout.preferredHeight: Math.round(units.gridUnit * 1.25)
+            implicitHeight: Math.round(units.gridUnit * 1.25)
-            Layout.preferredWidth: Layout.preferredHeight
+            implicitWidth: implicitHeight
             checkable: true
             checked: plasmoid.configuration.pin
             onToggled: plasmoid.configuration.pin = checked
@@ -116,7 +116,6 @@ ColumnLayout {
         HiddenItemsView {
             id: hiddenItemsView
             Layout.fillWidth: !activeApplet
-            Layout.preferredWidth: activeApplet ? iconColumnWidth : -1
             Layout.fillHeight: true
         }
 

applets/systemtray/package/contents/ui/HiddenItemsView.qml

@@ -27,8 +27,8 @@ import org.kde.plasma.extras 2.0 as PlasmaExtras
 PlasmaExtras.ScrollArea {
     id: hiddenTasksView
 
-    visible: !activeApplet || activeApplet.parent.parent == hiddenTasksColumn
+    visible: !root.activeApplet || (root.activeApplet.parent && root.activeApplet.parent.inHiddenLayout)
-    width: activeApplet ? iconColumnWidth : parent.width
+    implicitWidth: root.activeApplet ? iconColumnWidth : parent.width
     property alias layout: hiddenTasksColumn
     //Useful to align stuff to the column of icons, both in expanded and shrink modes
     property int iconColumnWidth: root.hiddenItemSize + highlight.marginHints.left + highlight.marginHints.right
@@ -64,9 +64,7 @@ PlasmaExtras.ScrollArea {
                 spacing: units.smallSpacing
                 width: parent.width
                 property Item hoveredItem
-                property alias marginHints: highlight.marginHints
+                readonly property int iconItemHeight: root.hiddenItemSize + highlight.marginHints.top + highlight.marginHints.bottom
-
-                objectName: "hiddenTasksColumn"
             }
         }
     }

applets/systemtray/package/contents/ui/PlasmoidPopupsContainer.qml

@@ -43,7 +43,6 @@ StackView {
             activeApplet.fullRepresentationItem.anchors.centerIn = undefined;
             activeApplet.fullRepresentationItem.anchors.fill = undefined;
 
-
             mainStack.replace({item: activeApplet.fullRepresentationItem, immediate: !dialog.visible, properties: {focus: true}});
         } else {
             mainStack.replace(emptyPage);

applets/systemtray/package/contents/ui/items/AbstractItem.qml

@@ -24,19 +24,16 @@ import org.kde.plasma.components 2.0 as PlasmaComponents
 PlasmaCore.ToolTipArea {
     id: abstractItem
 
-    height: effectiveItemSize + marginHints.top + marginHints.bottom
+    height: inVisibleLayout ? visibleLayout.iconSize : hiddenLayout.iconItemHeight
-    width: labelVisible ? parent.width : effectiveItemSize + marginHints.left + marginHints.right
+    width: inVisibleLayout ? visibleLayout.iconSize : hiddenLayout.width
 
-    property real effectiveItemSize: hidden ? root.hiddenItemSize : root.itemSize
     property string itemId
     property string category
     property alias text: label.text
-    property bool hidden: parent.objectName == "hiddenTasksColumn"
-    property QtObject marginHints: parent.marginHints
-    property bool labelVisible: abstractItem.hidden && !root.activeApplet
     property Item iconItem
-    //PlasmaCore.Types.ItemStatus
+    property int /*PlasmaCore.Types.ItemStatus*/ status
-    property int status
+    readonly property bool inHiddenLayout: effectiveStatus === PlasmaCore.Types.PassiveStatus
+    readonly property bool inVisibleLayout: effectiveStatus === PlasmaCore.Types.ActiveStatus
     property QtObject model
 
     signal clicked(var mouse)
@@ -57,14 +54,14 @@ PlasmaCore.ToolTipArea {
         }
     }
 
-    /* subclasses need to assign to this tiiltip properties
+    /* subclasses need to assign to this tooltip properties
     mainText:
     subText:
     icon:
     */
 
     location: {
-        if (abstractItem.parent && abstractItem.parent.objectName === "hiddenTasksColumn") {
+        if (inHiddenLayout) {
             if (LayoutMirroring.enabled && plasmoid.location !== PlasmaCore.Types.RightEdge) {
                 return PlasmaCore.Types.LeftEdge;
             } else if (plasmoid.location !== PlasmaCore.Types.LeftEdge) {
@@ -87,7 +84,7 @@ PlasmaCore.ToolTipArea {
     }
 
     onContainsMouseChanged: {
-        if (hidden && containsMouse) {
+        if (inHiddenLayout && containsMouse) {
             root.hiddenLayout.hoveredItem = abstractItem
         }
     }
@@ -154,8 +151,8 @@ PlasmaCore.ToolTipArea {
             leftMargin: iconItem ? iconItem.width + units.smallSpacing : 0
             verticalCenter: parent.verticalCenter
         }
-        opacity: labelVisible ? 1 : 0
+        opacity: visible ? 1 : 0
-        visible: abstractItem.hidden
+        visible: abstractItem.inHiddenLayout && !root.activeApplet
         Behavior on opacity {
             NumberAnimation {
                 duration: units.longDuration

applets/systemtray/package/contents/ui/items/PlasmoidItem.qml

@@ -37,6 +37,10 @@ AbstractItem {
     status: applet ? applet.status : PlasmaCore.Types.UnknownStatus
     active: root.activeApplet !== applet
 
+    Component.onDestruction: {
+        applet = null
+    }
+
     onClicked: {
         if (applet && mouse.button === Qt.LeftButton) {
             applet.expanded = true;
@@ -49,7 +53,7 @@ AbstractItem {
     }
     onContextMenu: {
         if (applet) {
-            plasmoid.nativeInterface.showPlasmoidMenu(applet, 0, plasmoidContainer.hidden ? applet.height : 0);
+            plasmoid.nativeInterface.showPlasmoidMenu(applet, 0, plasmoidContainer.inHiddenLayout ? applet.height : 0);
         }
     }
 
@@ -58,12 +62,33 @@ AbstractItem {
             applet.width = height
         }
     }
+
+    //This is to make preloading effective, minimizes the scene changes
+    function preloadFullRepresentationItem(fullRepresentationItem) {
+        if (fullRepresentationItem && applet.fullRepresentationItem.parent === null) {
+            fullRepresentationItem.width = expandedRepresentation.width
+            fullRepresentationItem.width = expandedRepresentation.height
+            fullRepresentationItem.parent = preloadedStorage;
+        }
+    }
+
     onAppletChanged: {
+        if (applet) {
+            applet.parent = plasmoidContainer
+            applet.anchors.left = plasmoidContainer.left
+            applet.anchors.top = plasmoidContainer.top
+            applet.anchors.bottom = plasmoidContainer.bottom
+            applet.width = plasmoidContainer.height
+            applet.visible = true
+            plasmoidContainer.visible = true
+
+            preloadFullRepresentationItem(applet.fullRepresentationItem)
+        }
         if (!applet) {
             plasmoidContainer.destroy();
-            print("applet destroyed")
         }
     }
+
     Connections {
         target: applet
         onActivated: plasmoidContainer.activated()
@@ -72,19 +97,23 @@ AbstractItem {
             if (expanded) {
                 var oldApplet = root.activeApplet;
                 root.activeApplet = applet;
-                if (oldApplet) {
+                if (oldApplet && oldApplet !== applet) {
                     oldApplet.expanded = false;
                 }
                 dialog.visible = true;
                 plasmoidContainer.activated()
 
             } else if (root.activeApplet === applet) {
-                if (!applet.parent.hidden) {
+                if (!inHiddenLayout) {
                     dialog.visible = false;
                 }
                 //if not expanded we don't have an active applet anymore
                 root.activeApplet = null;
             }
         }
+
+        onFullRepresentationItemChanged: {
+            preloadFullRepresentationItem(fullRepresentationItem)
+        }
     }
 }

applets/systemtray/package/contents/ui/items/StatusNotifierItem.qml

@@ -77,7 +77,7 @@ AbstractItem {
         var pos = plasmoid.nativeInterface.popupPosition(taskIcon, mouse.x, mouse.y);
 
         switch (mouse.button) {
-        case Qt.LeftButton: {
+        case Qt.LeftButton:
             var service = statusNotifierSource.serviceForSource(DataEngineSource);
             var operation = service.operationDescription("Activate");
             operation.x = pos.x;
@@ -92,7 +92,6 @@ AbstractItem {
             });
             taskIcon.activated()
             break;
-        }
         case Qt.RightButton:
             openContextMenu(pos);
             break;

applets/systemtray/package/contents/ui/main.qml

@@ -146,27 +146,6 @@ MouseArea {
     Containment.onAppletAdded: {
         //Allow the plasmoid expander to know in what window it will be
         var plasmoidContainer = plasmoidItemComponent.createObject(invisibleEntriesContainer, {"x": x, "y": y, "applet": applet});
-
-        applet.parent = plasmoidContainer
-        applet.anchors.left = plasmoidContainer.left
-        applet.anchors.top = plasmoidContainer.top
-        applet.anchors.bottom = plasmoidContainer.bottom
-        applet.width = plasmoidContainer.height
-        applet.visible = true
-        plasmoidContainer.visible = true
-        
-        //This is to make preloading effective, minimizes the scene changes
-        if (applet.fullRepresentationItem) {
-            applet.fullRepresentationItem.width = expandedRepresentation.width
-            applet.fullRepresentationItem.width = expandedRepresentation.height
-            applet.fullRepresentationItem.parent = preloadedStorage;
-        } else {
-            applet.fullRepresentationItemChanged.connect(function() {
-                applet.fullRepresentationItem.width = expandedRepresentation.width
-                applet.fullRepresentationItem.width = expandedRepresentation.height
-                applet.fullRepresentationItem.parent = preloadedStorage;
-            });
-        }
     }
 
     //being there forces the items to fully load, and they will be reparented in the popup one by one, this item is *never* visible
@@ -175,9 +154,6 @@ MouseArea {
         visible: false
     }
 
-    Containment.onAppletRemoved: {
-    }
-
     Connections {
         target: plasmoid
         onUserConfiguringChanged: {
@@ -260,18 +236,11 @@ MouseArea {
 
             delegate: StatusNotifierItem {}
         }
-        //NOTE: this exists mostly for not causing reference errors
-        property QtObject marginHints: QtObject {
-            property int left: 0
-            property int top: 0
-            property int right: 0
-            property int bottom: 0
-        }
     }
 
     CurrentItemHighLight {
         visualParent: tasksRow
-        target: root.activeApplet && root.activeApplet.parent.parent == tasksRow ? root.activeApplet.parent : root
+        target: root.activeApplet && root.activeApplet.parent && root.activeApplet.parent.inVisibleLayout ? root.activeApplet.parent : root
         location: plasmoid.location
     }
 
@@ -327,14 +296,7 @@ MouseArea {
         y: Math.round(height/2 - childrenRect.height/2)
         x: (expander.visible && LayoutMirroring.enabled ? expander.width : 0) + Math.round(width/2 - childrenRect.width/2)
 
-
+        readonly property var iconSize: root.itemSize + units.smallSpacing
-        //Do spacing with margins, to correctly compute the number of lines
-        property QtObject marginHints: QtObject {
-            property int left: Math.round(units.smallSpacing / 2)
-            property int top: Math.round(units.smallSpacing / 2)
-            property int right: Math.round(units.smallSpacing / 2)
-            property int bottom: Math.round(units.smallSpacing / 2)
-        }
 
         //add doesn't seem to work used in conjunction with stackBefore/stackAfter
         /*add: Transition {