Improve layout of Effect View

* use frame in the scroll area
* remove needless anchoring for an Effect
* use one RowLayout for one Effect row
* add a left and right padding using the normal spacing
* Use a ColumnLayout for the center element consisting of
  ** name
  ** description
  ** (info)
  ** (video)
* Video moved into an own component
* Animations removed

REVIEW: 116693

kcmkwin/kwincompositing/qml/Effect.qml

@@ -23,195 +23,113 @@ import QtQuick 2.1
 import QtQuick.Controls 1.1
 import QtQuick.Layouts 1.0
 import org.kde.kwin.kwincompositing 1.0
-import QtMultimedia 5.0 as Multimedia
-import org.kde.qtextracomponents 2.0 as QtExtra
 
-Item {
+Rectangle {
     id: item
     width: parent.width
-    height: 40
+    height: childrenRect.height
+    color: item.ListView.isCurrentItem ? effectView.backgroundActiveColor : index % 2 ? effectView.backgroundNormalColor : effectView.backgroundAlternateColor
     signal changed()
     property alias checked: effectStatusCheckBox.checked
 
-    Rectangle {
+    MouseArea {
-        id: background
+        anchors.fill: parent
-        color: item.ListView.isCurrentItem ? effectView.backgroundActiveColor : index % 2 ? effectView.backgroundNormalColor : effectView.backgroundAlternateColor
+        onClicked: {
-        anchors.fill : parent
+            effectView.currentIndex = index;
+        }
+    }
 
-        RowLayout {
+    RowLayout {
-            id: rowEffect
+        id: rowEffect
-            anchors.fill: parent
+        width: parent.width - 2 * spacing
-            CheckBox {
+        x: spacing
-                function isDesktopSwitching() {
+        CheckBox {
-                    if (model.ServiceNameRole == "kwin4_effect_slide") {
+            function isDesktopSwitching() {
-                        return true;
+                if (model.ServiceNameRole == "kwin4_effect_slide") {
-                    } else if (model.ServiceNameRole == "kwin4_effect_fadedesktop") {
+                    return true;
-                        return true;
+                } else if (model.ServiceNameRole == "kwin4_effect_fadedesktop") {
-                    } else if (model.ServiceNameRole == "kwin4_effect_cubeslide") {
+                    return true;
-                        return true;
+                } else if (model.ServiceNameRole == "kwin4_effect_cubeslide") {
-                    } else {
+                    return true;
-                        return false;
+                } else {
-                    }
+                    return false;
-                }
-                function isWindowManagementEnabled() {
-                    if (model.ServiceNameRole == "kwin4_effect_dialogparent") {
-                        windowManagementEnabled = effectStatusCheckBox.checked;
-                        return windowManagementEnabled = effectStatusCheckBox.checked && windowManagementEnabled;
-                    } else if (model.ServiceNameRole == "kwin4_effect_desktopgrid") {
-                        windowManagementEnabled = effectStatusCheckBox.checked;
-                        return windowManagementEnabled = effectStatusCheckBox.checked && windowManagementEnabled;
-                    } else if (model.ServiceNameRole == "kwin4_effect_presentwindows") {
-                        windowManagementEnabled = effectStatusCheckBox.checked;
-                        return windowManagementEnabled = effectStatusCheckBox.checked && windowManagementEnabled;
-                    }
-                    return windowManagementEnabled;
-                }
-
-                id: effectStatusCheckBox
-                property bool windowManagementEnabled;
-                checked: model.EffectStatusRole
-                exclusiveGroup: isDesktopSwitching() ? desktopSwitching : null
-
-                onCheckedChanged: item.changed()
-                Connections {
-                    target: searchModel
-                    onDataChanged: {
-                        effectStatusCheckBox.checked = model.EffectStatusRole;
-                    }
                 }
             }
-
+            function isWindowManagementEnabled() {
-            Item {
+                if (model.ServiceNameRole == "kwin4_effect_dialogparent") {
-                id: effectItem
+                    windowManagementEnabled = effectStatusCheckBox.checked;
-                width: effectView.width - effectStatusCheckBox.width - aboutButton.width - configureButton.width
+                    return windowManagementEnabled = effectStatusCheckBox.checked && windowManagementEnabled;
-                height: item.height
+                } else if (model.ServiceNameRole == "kwin4_effect_desktopgrid") {
-                anchors.top: rowEffect.top
+                    windowManagementEnabled = effectStatusCheckBox.checked;
-                anchors.left: effectStatusCheckBox.right
+                    return windowManagementEnabled = effectStatusCheckBox.checked && windowManagementEnabled;
-                MouseArea {
+                } else if (model.ServiceNameRole == "kwin4_effect_presentwindows") {
-                    id: area
+                    windowManagementEnabled = effectStatusCheckBox.checked;
-                    width: effectView.width - effectStatusCheckBox.width
+                    return windowManagementEnabled = effectStatusCheckBox.checked && windowManagementEnabled;
-                    height: effectView.height
-                    onClicked: {
-                        effectView.currentIndex = index;
-                    }
-                }
-                Column {
-                    id: col
-                    height: parent.height
-                    Text {
-                        text: model.NameRole
-                    }
-                    Text {
-                        id: desc
-                        function wrapDescription() {
-                            if (wrapMode == Text.NoWrap) {
-                                wrapMode = Text.WordWrap;
-                                elide = Text.ElideNone;
-                            } else {
-                                wrapMode = Text.NoWrap;
-                                elide = Text.ElideRight;
-                            }
-                        }
-                        text: model.DescriptionRole
-                        width: effectView.width - 100
-                        elide: Text.ElideRight
-                    }
-                    Item {
-                        id:aboutItem
-                        anchors.top: desc.bottom
-                        anchors.topMargin: 20
-                        visible: false
-
-                        Text {
-                            text: "Author: " + model.AuthorNameRole + "\n" + "License: " + model.LicenseRole
-                            font.bold: true
-                        }
-                        PropertyAnimation {id: animationAbout; target: aboutItem; property: "visible"; to: !aboutItem.visible}
-                        PropertyAnimation {id: animationAboutSpacing; target: item; property: "height"; to: item.height == 40 ?  item.height + 100 : 40}
-                    }
-                    Multimedia.Video {
-                        id: videoItem
-                        function showHide() {
-                            replayButton.visible = false;
-                            if (videoItem.visible === true) {
-                                videoItem.stop();
-                                videoItem.visible = false;
-                                // TODO: this should be done in a better way
-                                item.height = item.height - 400;
-                            } else {
-                                videoItem.visible = true;
-                                videoItem.play();
-                                item.height = item.height + 400;
-                            }
-                        }
-                        autoLoad: false
-                        source: model.VideoRole
-                        visible: false
-                        width: 400
-                        height: 400
-                        anchors.bottom: parent.bottom
-                        BusyIndicator {
-                            anchors.centerIn: parent
-                            visible: videoItem.status == Multimedia.MediaPlayer.Loading
-                            running: true
-                        }
-                        MouseArea {
-                            // it's a mouse area with icon inside to not have an ugly button background
-                            id: replayButton
-                            visible: false
-                            anchors.fill: parent
-                            onClicked: {
-                                replayButton.visible = false;
-                                videoItem.play();
-                            }
-                            QtExtra.QIconItem {
-                                id: replayIcon
-                                anchors.centerIn: parent
-                                width: 16
-                                height: 16
-                                icon: "media-playback-start"
-                            }
-                            Connections {
-                                target: videoItem
-                                onStopped: {
-                                    replayButton.visible = true
-                                }
-                            }
-                        }
-                    }
                 }
+                return windowManagementEnabled;
             }
 
-            RowLayout {
+            id: effectStatusCheckBox
-                anchors.right: parent.right
+            property bool windowManagementEnabled;
-                Button {
+            checked: model.EffectStatusRole
-                    id: videoButton
+            exclusiveGroup: isDesktopSwitching() ? desktopSwitching : null
-                    visible: model.VideoRole.toString() !== ""
-                    iconName: "video"
-                    onClicked: videoItem.showHide()
-                }
-                Button {
-                    id: configureButton
-                    visible: effectConfig.effectUiConfigExists(model.ServiceNameRole)
-                    enabled: effectStatusCheckBox.checked
-                    iconName: "configure"
-                    onClicked: {
-                        effectConfig.openConfig(model.NameRole);
-                    }
-                }
 
-                Button {
+            onCheckedChanged: item.changed()
-                    id: aboutButton
+            Connections {
-                    iconName: "dialog-information"
+                target: searchModel
-                    onClicked: {
+                onDataChanged: {
-                        animationAbout.running = true;
+                    effectStatusCheckBox.checked = model.EffectStatusRole;
-                        animationAboutSpacing.running = true;
-                        desc.wrapDescription();
-                    }
                 }
             }
+        }
 
-        } //end Row
+        ColumnLayout {
-    } //end Rectangle
+            id: effectItem
-} //end item
+            Text {
+                text: model.NameRole
+            }
+            Text {
+                id: desc
+                text: model.DescriptionRole
+            }
+            Text {
+                id:aboutItem
+                text: "Author: " + model.AuthorNameRole + "\n" + "License: " + model.LicenseRole
+                font.bold: true
+                visible: false
+            }
+            Video {
+                id: videoItem
+                visible: false
+            }
+        }
+        Item {
+            // spacer
+            Layout.fillWidth: true
+        }
 
+        Button {
+            id: videoButton
+            visible: model.VideoRole.toString() !== ""
+            iconName: "video"
+            onClicked: videoItem.showHide()
+        }
+        Button {
+            id: configureButton
+            visible: effectConfig.effectUiConfigExists(model.ServiceNameRole)
+            enabled: effectStatusCheckBox.checked
+            iconName: "configure"
+            onClicked: {
+                effectConfig.openConfig(model.NameRole);
+            }
+        }
+
+        Button {
+            id: aboutButton
+            iconName: "dialog-information"
+            onClicked: {
+                aboutItem.visible = !aboutItem.visible;
+            }
+        }
+    } //end Row
+} //end Rectangle

kcmkwin/kwincompositing/qml/EffectView.qml

@@ -93,20 +93,15 @@ Item {
 
         ScrollView {
             id: scroll
+            frameVisible: true
             highlightOnFocus: true
             Layout.fillWidth: true
             Layout.fillHeight: true
-            anchors {
-                top: searchField.bottom
-                left: parent.left
-                bottom: parent.bottom
-            }
             ListView {
                 id: effectView
                 property color backgroundActiveColor: searchModel.backgroundActiveColor
                 property color backgroundNormalColor: searchModel.backgroundNormalColor
                 property color backgroundAlternateColor: searchModel.backgroundAlternateColor
-                Layout.fillWidth: true
                 anchors.fill: parent
                 model: searchModel
                 delegate: Effect{

kcmkwin/kwincompositing/qml/Video.qml

@@ -0,0 +1,71 @@
+/**************************************************************************
+* KWin - the KDE window manager                                          *
+* This file is part of the KDE project.                                  *
+*                                                                        *
+* Copyright (C) 2013 Antonis Tsiapaliokas <kok3rs@gmail.com>             *
+* Copyright (C) 2014 Martin Gräßlin       <mgraesslin@kde.org>           *
+*                                                                        *
+* This program is free software; you can redistribute it and/or modify   *
+* it under the terms of the GNU General Public License as published by   *
+* the Free Software Foundation; either version 2 of the License, or      *
+* (at your option) any later version.                                    *
+*                                                                        *
+* This program is distributed in the hope that it will be useful,        *
+* but WITHOUT ANY WARRANTY; without even the implied warranty of         *
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the          *
+* GNU General Public License for more details.                           *
+*                                                                        *
+* You should have received a copy of the GNU General Public License      *
+* along with this program.  If not, see <http://www.gnu.org/licenses/>.  *
+**************************************************************************/
+import QtQuick 2.1
+import QtQuick.Controls 1.1
+import QtQuick.Layouts 1.0
+import QtMultimedia 5.0 as Multimedia
+import org.kde.qtextracomponents 2.0 as QtExtra
+
+Multimedia.Video {
+    id: videoItem
+    function showHide() {
+        replayButton.visible = false;
+        if (videoItem.visible === true) {
+            videoItem.stop();
+            videoItem.visible = false;
+        } else {
+            videoItem.visible = true;
+            videoItem.play();
+        }
+    }
+    autoLoad: false
+    source: model.VideoRole
+    width: 400
+    height: 400
+    BusyIndicator {
+        anchors.centerIn: parent
+        visible: videoItem.status == Multimedia.MediaPlayer.Loading
+        running: true
+    }
+    MouseArea {
+        // it's a mouse area with icon inside to not have an ugly button background
+        id: replayButton
+        visible: false
+        anchors.fill: parent
+        onClicked: {
+            replayButton.visible = false;
+            videoItem.play();
+        }
+        QtExtra.QIconItem {
+            id: replayIcon
+            anchors.centerIn: parent
+            width: 16
+            height: 16
+            icon: "media-playback-start"
+        }
+        Connections {
+            target: videoItem
+            onStopped: {
+                replayButton.visible = true
+            }
+        }
+    }
+}