Merge lp:~gerboland/unity-2d/dash-lenses into lp:unity-2d/3.0

[Gerry Boland]

[dash] Add Lens Bar to Dash. Remove it from Launcher and adjust sizes to fit.

[Florian Boucault (fboucault)]

Functionally, everything checks out!

review: Approve (functional)

[Florian Boucault (fboucault)]

places/LensBar.qml:

import Qt 4.7

should be:

import QtQuick 1.0

review: Needs Fixing (code)

[Florian Boucault (fboucault)]

In places/LensBar.qml, please remove the:

[...] /* required for drag’n’drop handling */

[Florian Boucault (fboucault)]

In places/LensBar.qml:

    /* Lens Bar rectangle */
    Rectangle {
        id: lensBar

[...]

Don't give it for id 'lensBar' as the whole component's class is LensBar which could lead to confusion. Use 'background' instead for example.
The comment above is not very helpful. Drop it if the id is explicit enough.

review: Needs Fixing

[Florian Boucault (fboucault)]

In places/LensBar.qml:

    Rectangle {
        id: lensBar
        anchors.top: parent.top
        anchors.bottom: parent.bottom
        anchors.left: parent.left
        width: parent.width
[...]

could be replaced by:

anchors.fill: parent

review: Needs Fixing

[Florian Boucault (fboucault)]

In places/dash.qml:

[...]
        LensBar {
[...]
            anchors.left: parent.left
            anchors.right: parent.right
            width: parent.width

either set the left and right anchors or the width, not both. Using anchors is more efficient.

review: Needs Fixing

[Florian Boucault (fboucault)]

General note, I like having the id of the component as the first line and then leaving a blank line afterwards.

[Florian Boucault (fboucault)]

In places/LensBar.qml:

Instead of using a ListView inside the Row, just use a Repeater. It will make things easier and simplify your keyboard navigation code that will then become generic.

review: Needs Fixing

[Florian Boucault (fboucault)]

Consistency in styling comments would be nice:

    /* BLA BLA */
    // BLA BLA
    //BLA BLA

I usually just use the first form.

[Florian Boucault (fboucault)]

In places/LensBar.qml:

        ListView {
            id: lensList
[...]
            orientation: "Horizontal"
[...]

That code is likely to go away but note that it's probably better to use the enum value ListView.Horizontal instead of a string "Horizontal".

[Florian Boucault (fboucault)]

In places/LensBar.qml:

Please move up the activateLens to dash.qml, rename it to activatePlaceEntry and factor the code between it and its sibling "function activatePlaceEntry(fileName, groupName, section)"

It would look like:
    function activatePlaceEntry(fileName, groupName, section) {
        var placeEntryModel = places.findPlaceEntry(fileName, groupName)
        if (placeEntryModel == null) {
            console.log("No match for place: %1 [Entry:%2]".arg(fileName).arg(groupName))
            return
        }
        activatePlaceEntry(placeEntryModel, section)
    }

    function activatePlaceEntry(placeEntry, section=0) {
         CODE OF THE FORMER activatePlaceEntry
    }

review: Needs Fixing

[Florian Boucault (fboucault)]

In places/LensButton.qml:

    property alias iconSourceSize: icon.sourceSize

is not used.

review: Needs Fixing

[Florian Boucault (fboucault)]

In places/LensButton.qml:

'iconWidth' is used in a couple of places but not defined in that file thus breaking encapsulation. Same for 'lensBar'.

Solution:
- for the root element: don't set the width and height for the root element in this file but in LensBar.qml
- for icon: use parent.height

review: Needs Fixing

[Florian Boucault (fboucault)]

In places/LensButton.qml:

    Rectangle {
        width: 32
        height: parent.height
[...]

hardcoded 32 should probably be parent.width, or better yet, replace the all thing with anchors.fill: parent

[Florian Boucault (fboucault)]

In places/LensButton.qml:

    Image {
        id: indicator
        width: 5
        height: 9

maybe use sourceSize.width and sourceSize.height instead if it works?

[...]

        y: 37
        anchors.horizontalCenter: parent.horizontalCenter

Setting it centered horizontally conflicts with hardcoding its y coordinate.

review: Needs Fixing

[Florian Boucault (fboucault)]

In places/LensButton.qml:

    // Lens icon supplied is of size 48x48. This is too much for the 44pixels high
    // lensBar. However much of the icon is transparent so we effectively crop it to 32x32.

I am not sure I am a big fan of this. Let's discuss it.

[lp:~gerboland/unity-2d/dash-lenses]

676. By Gerry Boland on 2011-08-10

Fix lensbar imports

677. By Gerry Boland on 2011-08-10

Fix layout issues and coment style

678. By Gerry Boland on 2011-08-10

Refactor activatePlaceEntry

679. By Gerry Boland on 2011-08-10

Another layout fix to LensBar in dash

680. By Gerry Boland on 2011-08-10

Fix keyboard navigation when Repeater used

681. By Gerry Boland on 2011-08-10

Revert rename of activePlaceEntryFromFile

682. By Gerry Boland on 2011-08-10

Fix iconWidth and layout

[Florian Boucault (fboucault)]

Everything checks out now. Thanks

review: Approve

Unmerged revisions

682. By Gerry Boland on 2011-08-10

Fix iconWidth and layout

681. By Gerry Boland on 2011-08-10

Revert rename of activePlaceEntryFromFile

680. By Gerry Boland on 2011-08-10

Fix keyboard navigation when Repeater used

679. By Gerry Boland on 2011-08-10

Another layout fix to LensBar in dash

678. By Gerry Boland on 2011-08-10

Refactor activatePlaceEntry

677. By Gerry Boland on 2011-08-10

Fix layout issues and coment style

676. By Gerry Boland on 2011-08-10

Fix lensbar imports

675. By Gerry Boland on 2011-08-10

[dash] Home Lens is special case, and for the moment requires own icon to be supplied.

674. By Gerry Boland on 2011-08-10

[launcher] Remove Lenses from Launcher, as they now exist in the Dash.

673. By Gerry Boland on 2011-08-10

[dash] With new LensBar in the dash, the Home Shortcuts layout needs to be adjusted to fit correctly.

=== modified file 'launcher/Launcher.qml'
--- launcher/Launcher.qml	2011-08-01 11:58:35 +0000
+++ launcher/Launcher.qml	2011-08-10 19:03:55 +0000
@@ -105,10 +105,6 @@
         id: applications
     }
 
-    LauncherPlacesList {
-        id: places
-    }
-
     LauncherDevicesList {
         id: devices
     }

=== modified file 'places/Home.qml'
--- places/Home.qml	2011-08-10 10:01:52 +0000
+++ places/Home.qml	2011-08-10 19:03:55 +0000
@@ -135,7 +135,7 @@
         anchors.verticalCenter: parent.verticalCenter
 
         width: 888
-        height: 466
+        height: 436
 
         Rectangle {
             anchors.fill: parent

=== modified file 'places/HomeShortcuts.qml'
--- places/HomeShortcuts.qml	2011-08-10 00:22:07 +0000
+++ places/HomeShortcuts.qml	2011-08-10 19:03:55 +0000
@@ -21,11 +21,11 @@
 
 Grid {
     anchors.fill: parent
-    anchors.topMargin: 26
+    anchors.topMargin: 21
     anchors.bottomMargin: 35
-    anchors.leftMargin: 32
+    anchors.leftMargin: 46
     anchors.rightMargin: 32
-    spacing: 61
+    spacing: 51
     columns: 4
     rows: 2
 
@@ -55,25 +55,25 @@
         focus: true
         label: u2d.tr("Media Apps")
         icon: "artwork/find_media_apps.png"
-        onClicked: activatePlaceEntry("/usr/share/unity/places/applications.place", "Files", 9)
+        onClicked: activatePlaceEntryFromFile("/usr/share/unity/places/applications.place", "Files", 9)
     }
 
     HomeButton {
         label: u2d.tr("Internet Apps")
         icon: "artwork/find_internet_apps.png"
-        onClicked: activatePlaceEntry("/usr/share/unity/places/applications.place", "Files", 8)
+        onClicked: activatePlaceEntryFromFile("/usr/share/unity/places/applications.place", "Files", 8)
     }
 
     HomeButton {
         label: u2d.tr("More Apps")
         icon: "artwork/find_more_apps.png"
-        onClicked: activatePlaceEntry("/usr/share/unity/places/applications.place", "Files", 0)
+        onClicked: activatePlaceEntryFromFile("/usr/share/unity/places/applications.place", "Files", 0)
     }
 
     HomeButton {
         label: u2d.tr("Find Files")
         icon: "artwork/find_files.png"
-        onClicked: activatePlaceEntry("/usr/share/unity/places/files.place", "Files", 0)
+        onClicked: activatePlaceEntryFromFile("/usr/share/unity/places/files.place", "Files", 0)
     }
 
     /* FIXME: use user's preferred applications instead of hardcoding them */

=== added file 'places/LensBar.qml'
--- places/LensBar.qml	1970-01-01 00:00:00 +0000
+++ places/LensBar.qml	2011-08-10 19:03:55 +0000
@@ -0,0 +1,114 @@
+/*
+ * This file is part of unity-2d
+ *
+ * Copyright 2011 Canonical Ltd.
+ *
+ * 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; version 3.
+ *
+ * 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 1.0
+import Unity2d 1.0
+
+FocusScope {
+    /* declare width & spacing of icons as required for layout calculations */
+    property int iconWidth: 32
+    property int iconSpacing: 28
+
+    property variant lenses: SortFilterProxyModel {
+        id: visiblePlaces
+        model: places
+        dynamicSortFilter: true
+
+        filterRole: Place.RoleShowEntry
+        filterRegExp: RegExp("^true$")
+    }
+
+    Rectangle {
+        id: background
+
+        anchors.fill: parent
+        color: "black"
+        opacity: 0.22
+    }
+
+    /* LensBar contains a row of LensButtons */
+    Row {
+        id: lensContainer
+
+        anchors.horizontalCenter: background.horizontalCenter
+        anchors.top: background.top
+        anchors.bottom: background.bottom
+        spacing: iconSpacing
+
+        Keys.onPressed: if (handleKeyPress(event.key)) event.accepted = true
+
+        /* The Home lens is unfortunately not supplied by the "lenses" list
+           This causes the keyboard navigation logic to be messy */
+        property int currentIndex: 0
+
+        function selectChild(index) {
+            var child = lensContainer.childFromIndex(index)
+            if (child != undefined) {
+                child.focus = true
+                currentIndex = index
+                return true
+            } else {
+                return false
+            }
+        }
+
+        function handleKeyPress(key) {
+            switch (key) {
+            case Qt.Key_Right:
+                return selectChild(currentIndex+1)
+            case Qt.Key_Left:
+                return selectChild(currentIndex-1)
+            }
+        }
+
+        function childFromIndex(index) {
+            var indexInChildren = 0
+            for(var i=0; i<children.length; i++) {
+                if (children[i] != repeater) {
+                    if (indexInChildren == index) return children[i]
+                    indexInChildren++
+                }
+            }
+            return undefined
+        }
+
+        /* Need to manually include the Home lens */
+        LensButton {
+            id: homeLens
+
+            focus: true
+            icon: "artwork/home.png"
+            onClicked: activateHome()
+            active: ( dashView.activePlaceEntry == "" )
+            width: iconWidth
+        }
+
+        /* Now fetch all other lenses and display */
+        Repeater{
+            id: repeater
+
+            model: lenses
+            delegate: LensButton {
+                icon: item.icon
+                active: item.active
+                onClicked: activatePlaceEntry(item)
+                width: iconWidth
+            }
+        }
+    }
+}

=== added file 'places/LensButton.qml'
--- places/LensButton.qml	1970-01-01 00:00:00 +0000
+++ places/LensButton.qml	2011-08-10 19:03:55 +0000
@@ -0,0 +1,83 @@
+/*
+ * This file is part of unity-2d
+ *
+ * Copyright 2010-2011 Canonical Ltd.
+ *
+ * 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; version 3.
+ *
+ * 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 1.0
+import Effects 1.0
+
+/* This component represents a single "lens" in the dash and an active
+ * indicator icon positioned above it if necessary.
+ */
+AbstractButton {
+    property alias icon: icon.source
+    property bool active: false
+
+    id: lensButton
+
+    anchors.top: parent.top
+    anchors.bottom: parent.bottom
+
+    effect: DropShadow {
+         blurRadius: 8
+         color: "white"
+         offset.x: 0
+         offset.y: 0
+         enabled: ( lensButton.state == "selected" || active )
+    }
+
+    Rectangle {
+        anchors.fill: parent
+        anchors.topMargin: 7
+        anchors.bottomMargin: 7
+
+        border.color: "white"
+        border.width: 1
+        color: "transparent"
+        radius: 2
+        visible: ( parent.state == "selected" )
+    }
+
+    /* Lens icon supplied is of size 48x48. This is too much for the 44pixels high
+       lensBar. However much of the icon is transparent so we effectively crop it to 32x32. */
+    Image {
+        id: icon
+
+        height: iconWidth
+        fillMode: Image.PreserveAspectCrop
+        clip: true
+
+        anchors.horizontalCenter: parent.horizontalCenter
+        anchors.centerIn: parent
+
+        opacity: ( parent.state == "mouseOver" || parent.state == "pressed" || active ) ? 1.0 : 0.57
+    }
+
+    /* Indicator arrow to show Lens active */
+    Image {
+        id: indicator
+        source: "artwork/arrow.png"
+
+        width: sourceSize.width
+        height: sourceSize.height
+        anchors.bottomMargin: (width-height)/2 /* Correct for rotation */
+        anchors.bottom: parent.bottom
+        anchors.horizontalCenter: parent.horizontalCenter
+
+        rotation: -90
+        visible: active
+    }
+}

=== modified file 'places/app/dashdeclarativeview.cpp'
--- places/app/dashdeclarativeview.cpp	2011-08-09 14:32:13 +0000
+++ places/app/dashdeclarativeview.cpp	2011-08-10 19:03:55 +0000
@@ -238,7 +238,7 @@
 DashDeclarativeView::activatePlaceEntry(const QString& file, const QString& entry, const int section)
 {
     QGraphicsObject* dash = rootObject();
-    QMetaObject::invokeMethod(dash, "activatePlaceEntry", Qt::AutoConnection,
+    QMetaObject::invokeMethod(dash, "activatePlaceEntryFromFile", Qt::AutoConnection,
                               Q_ARG(QVariant, QVariant::fromValue(file)),
                               Q_ARG(QVariant, QVariant::fromValue(entry)),
                               Q_ARG(QVariant, QVariant::fromValue(section)));

=== added file 'places/artwork/home.png'
Binary files places/artwork/home.png	1970-01-01 00:00:00 +0000 and places/artwork/home.png	2011-08-10 19:03:55 +0000 differ
=== modified file 'places/dash.qml'
--- places/dash.qml	2011-08-10 00:10:39 +0000
+++ places/dash.qml	2011-08-10 19:03:55 +0000
@@ -64,25 +64,28 @@
         currentPage.visible = true
     }
 
-    function activatePlaceEntry(fileName, groupName, section) {
+    function activatePlaceEntryFromFile(fileName, groupName, section) {
         var placeEntryModel = places.findPlaceEntry(fileName, groupName)
         if (placeEntryModel == null) {
             console.log("No match for place: %1 [Entry:%2]".arg(fileName).arg(groupName))
             return
         }
 
+        activatePlaceEntry( placeEntryModel, section)
+    }
+
+    function activatePlaceEntry( place, section ) {
         /* FIXME: PlaceEntry.SetActiveSection needs to be called after
            PlaceEntry.SetActive in order for it to have an effect.
-           This is likely a bug in the place daemons.
-        */
-        placeEntryModel.active = true
-        placeEntryModel.activeSection = section
+           This is likely a bug in the place daemons. */
+        place.active = true
+        place.activeSection = ( section != undefined ) ? section : 0
         pageLoader.source = "PlaceEntryView.qml"
         /* Take advantage of the fact that the loaded qml is local and setting
            the source loads it immediately making pageLoader.item valid */
-        pageLoader.item.model = placeEntryModel
+        pageLoader.item.model = place
         activatePage(pageLoader.item)
-        dashView.activePlaceEntry = placeEntryModel.dbusObjectPath
+        dashView.activePlaceEntry = place.dbusObjectPath
     }
 
     function activateHome() {
@@ -221,15 +224,28 @@
             */
             KeyNavigation.right: refine_search.visible && !refine_search.folded ? refine_search : pageLoader
             KeyNavigation.up: search_entry
+            KeyNavigation.down: lensBar
 
             anchors.top: search_entry.bottom
             anchors.topMargin: 2
-            anchors.bottom: parent.bottom
+            anchors.bottom: lensBar.top
             anchors.left: parent.left
             anchors.right: !refine_search.visible || refine_search.folded ? parent.right : refine_search.left
             anchors.rightMargin: !refine_search.visible || refine_search.folded ? 0 : 15
             onLoaded: item.focus = true
         }
+
+        LensBar {
+            id: lensBar
+
+            KeyNavigation.up: pageLoader
+
+            anchors.bottom: parent.bottom
+            anchors.left: parent.left
+            anchors.right: parent.right
+            height: 44
+            visible: dashView.expanded
+        }
     }
 
     Button {