Merge lp:~renatofilho/address-book-app/fix-1377334 into lp:~phablet-team/address-book-app/staging

[Renato Araujo Oliveira Filho]

Implement AvatarImport as a Page.

Does not use dialog on content hub component, to avoid application to get stuck.
Handle operation aborted in pick mode.

[Ken VanDine (ken-vandine)]

I haven't done a full review yet, but I would suggest perhaps it's worth talking about adding the title to ContentTransferHint rather than reimplementing it here. I worry about needing to maintain a copy of that code, and it seems like a nice feature for ContentTransferHint.

[Ken VanDine (ken-vandine)]

I haven't run it yet, I'll wait for a CI build for that. But from the code review it looks good, see a couple inline comments.

Also, unrelated to this MP, I see there is a copyImage function to copy the file. There is now an API for that in content-hub, ContentItem.move(). Now that content-hub provides an API for that, you could consider switching to that.

[lp:~renatofilho/address-book-app/fix-1377334]

315. By Renato Araujo Oliveira Filho on 2014-10-07

Fixed contact export.

[Renato Araujo Oliveira Filho (renatofilho)]

> I haven't run it yet, I'll wait for a CI build for that. But from the code
> review it looks good, see a couple inline comments.
>
> Also, unrelated to this MP, I see there is a copyImage function to copy the
> file. There is now an API for that in content-hub, ContentItem.move(). Now
> that content-hub provides an API for that, you could consider switching to
> that.

The copyImage not only copy the image it resize it to a small size to be used as avatar.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:314
http://jenkins.qa.ubuntu.com/job/phablet-team-address-book-app-staging-ci/352/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/5624/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3981
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-team-address-book-app-staging-utopic-i386-ci/352
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/5313/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6876
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6876/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/14295
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/3352
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4308
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4308/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-team-address-book-app-staging-ci/352/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:315
http://jenkins.qa.ubuntu.com/job/phablet-team-address-book-app-staging-ci/353/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/5629/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3985
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-team-address-book-app-staging-utopic-i386-ci/353
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/5317/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6881
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/6881/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/14299
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/3354
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4312
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/4312/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/phablet-team-address-book-app-staging-ci/353/rebuild

review: Needs Fixing (continuous-integration)

[Ken VanDine (ken-vandine)]

Works well on my mako, I tested the full path including adding the avatar from gallery as well as aborting the transfer. All worked well.

review: Approve

Unmerged revisions

315. By Renato Araujo Oliveira Filho on 2014-10-07

Fixed contact export.

314. By Renato Araujo Oliveira Filho on 2014-10-07

Implement AvatarImport as a Page.

Does not use dialogs on content hub component, to avoid application to get stuck.
Handle operation aborted in pick mode.

=== modified file 'src/imports/ContactEdit/AvatarImport.qml'
--- src/imports/ContactEdit/AvatarImport.qml	2014-08-13 20:52:57 +0000
+++ src/imports/ContactEdit/AvatarImport.qml	2014-10-07 20:52:25 +0000
@@ -18,103 +18,108 @@
 
 import Ubuntu.Components 1.1
 import Ubuntu.Components.Popups 1.0
-import Ubuntu.Content 0.1
+import Ubuntu.Content 1.1
 
-Item {
+Page {
     id: root
 
-    property var importDialog: null
+    property var avatarListener: null
+    property alias activeTransfer: signalConnections.target
+    property alias isTransferRunning: activityIndicator.running
 
     signal avatarReceived(string avatarUrl)
 
-    function requestNewAvatar()
-    {
-        if (!root.importDialog) {
-            root.importDialog = PopupUtils.open(contentHubDialog, null)
-        } else {
-            console.warn("Import dialog already running")
-        }
-    }
-
-    Component {
-        id: contentHubDialog
-
-        PopupBase {
-            id: dialogue
-
-            property alias activeTransfer: signalConnections.target
-
-            parent: QuickUtils.rootItem(this)
-            focus: true
-
-            Rectangle {
-                anchors.fill: parent
-
-                ContentTransferHint {
-                    anchors.fill: parent
-                    activeTransfer: dialogue.activeTransfer
-                }
-
-                ContentPeerPicker {
-                    id: peerPicker
-
-                    anchors.fill: parent
-                    visible: dialogue.done
-                    contentType: ContentType.Pictures
-                    handler: ContentHandler.Source
-
-                    onPeerSelected: {
-                        peer.selectionType = ContentTransfer.Single
-                        dialogue.activeTransfer = peer.request()
-                    }
-
-                    onCancelPressed: {
-                        PopupUtils.close(root.importDialog)
-                    }
-                }
-            }
-
-            Connections {
-                id: signalConnections
-
-                target: dialogue.activeTransfer
-                onStateChanged: {
-                    var done = ((dialogue.activeTransfer.state === ContentTransfer.Charged) ||
-                                (dialogue.activeTransfer.state === ContentTransfer.Aborted))
-
-                    if (dialogue.activeTransfer.state === ContentTransfer.Charged) {
-                        dialogue.hide()
-                        if (dialogue.activeTransfer.items.length > 0) {
-                            root.avatarReceived(dialogue.activeTransfer.items[0].url)
-                        }
-                    }
-
-                    if (done) {
-                        acceptTimer.restart()
-                    }
-                }
-            }
-
-            // WORKAROUND: Work around for application becoming insensitive to touch events
-            // if the dialog is dismissed while the application is inactive.
-            // Just listening for changes to Qt.application.active doesn't appear
-            // to be enough to resolve this, so it seems that something else needs
-            // to be happening first. As such there's a potential for a race
-            // condition here, although as yet no problem has been encountered.
-            Timer {
-                id: acceptTimer
-
-                interval: 100
-                repeat: true
-                running: false
-                onTriggered: {
-                   if(Qt.application.active) {
-                       PopupUtils.close(root.importDialog)
-                   }
-                }
-            }
-
-            Component.onDestruction: root.importDialog = null
+    title: i18n.tr("Import avatar from...")
+
+    Item {
+        id: transferHint
+
+        anchors.fill: parent
+        visible: root.isTransferRunning
+
+        ActivityIndicator {
+            id: activityIndicator
+
+            anchors {
+                horizontalCenter: parent.horizontalCenter
+                verticalCenter: parent.verticalCenter
+                verticalCenterOffset: units.gu(-4)
+            }
+            running: root.activeTransfer
+        }
+
+        Label {
+            anchors {
+                top: activityIndicator.bottom
+                topMargin: units.gu(2)
+                horizontalCenter: parent.horizontalCenter
+            }
+            // TRANSLATORS: This message appear while waiting for a external application content,
+            // %1 refers to the the name of source application
+            text: i18n.tr("Waiting for content from %1...").arg(peerPicker.peer.name)
+        }
+
+        Button {
+            id: cancelButton
+
+            enabled: root.activeTransfer && (root.activeTransfer.state === ContentTransfer.InProgress)
+            anchors {
+                bottom: parent.bottom
+                left: parent.left
+                right: parent.right
+                margins: units.gu(2)
+            }
+            text: i18n.tr("Cancel")
+            onClicked: root.activeTransfer.state = ContentTransfer.Aborted
+        }
+    }
+
+    ContentPeerPicker {
+        id: peerPicker
+
+        anchors.fill: parent
+        contentType: ContentType.Pictures
+        handler: ContentHandler.Source
+        showTitle: false
+        visible: !root.isTransferRunning
+
+        onPeerSelected: {
+            peer.selectionType = ContentTransfer.Single
+            root.activeTransfer = peer.request()
+        }
+
+        onCancelPressed: pageStack.pop()
+    }
+
+    Connections {
+        id: signalConnections
+
+        target: null
+        onStateChanged: {
+            var done = ((root.activeTransfer.state === ContentTransfer.Charged) ||
+                        (root.activeTransfer.state === ContentTransfer.Aborted))
+            transferHint.enabled = !done
+            if (root.activeTransfer.state === ContentTransfer.Charged) {
+                if (( root.activeTransfer.items.length > 0) && root.avatarListener) {
+                    root.avatarListener.avatarReceived( root.activeTransfer.items[0].url)
+                }
+            }
+
+            if (done) {
+                pageStack.pop()
+            }
+        }
+    }
+
+    Component.onDestruction: {
+        if (root.avatarListener) {
+            root.avatarListener.busy = false
+        }
+    }
+
+    Component.onCompleted: {
+        if (root.avatarListener) {
+            root.avatarListener.busy = true
         }
     }
 }

=== modified file 'src/imports/ContactEdit/ContactDetailAvatarEditor.qml'
--- src/imports/ContactEdit/ContactDetailAvatarEditor.qml	2014-09-29 13:20:08 +0000
+++ src/imports/ContactEdit/ContactDetailAvatarEditor.qml	2014-10-07 20:52:25 +0000
@@ -23,7 +23,7 @@
 ContactDetailBase {
     id: root
 
-    readonly property alias busy: activityIndicator.running
+    property alias busy: activityIndicator.running
     readonly property string defaultAvatar: "image://theme/add"
 
     function isEmpty() {
@@ -61,6 +61,16 @@
         return avatarUrl
     }
 
+    function avatarReceived(avatarUrl)
+    {
+        // remove the previous image, this is nessary to make sure that the new image
+        // get updated otherwise if the new image has the same name the image will not
+        // be updated
+        avatarImage.source = ""
+        // Update with the new value
+        avatarImage.source = application.copyImage(root.contact, avatarUrl);
+    }
+
     detail: contact ? contact.detail(ContactDetail.Avatar) : null
     implicitHeight: units.gu(8)
     implicitWidth: units.gu(8)
@@ -81,7 +91,7 @@
             anchors.centerIn: visible ? avatar : undefined
             height: units.gu(3)
             width: units.gu(3)
-            visible: source == defaultAvatar
+            visible: source == defaultAvatar && !root.busy
 
             // When updating the avatar using the content picker the temporary file returned
             // can contain the same name as the previous one and if the cache is enabled this
@@ -94,29 +104,16 @@
         id: activityIndicator
 
         anchors.centerIn: avatar
-        running: (avatarImport.importDialog != null)
         visible: running
     }
 
-    AvatarImport {
-        id: avatarImport
-
-        onAvatarReceived: {
-            // remove the previous image, this is nessary to make sure that the new image
-            // get updated otherwise if the new image has the same name the image will not
-            // be updated
-            avatarImage.source = ""
-            // Update with the new value
-            avatarImage.source = application.copyImage(root.contact, avatarUrl);
-        }
-    }
-
     MouseArea {
         anchors.fill: parent
         onClicked: {
             // make sure the OSK disappear
             root.forceActiveFocus()
-            avatarImport.requestNewAvatar()
+            root.busy = true
+            pageStack.push(Qt.resolvedUrl("AvatarImport.qml"), {avatarListener: root})
         }
     }
 }

=== modified file 'src/imports/ContactList/ContactExporter.qml'
--- src/imports/ContactList/ContactExporter.qml	2014-09-30 21:17:25 +0000
+++ src/imports/ContactList/ContactExporter.qml	2014-10-07 20:52:25 +0000
@@ -16,15 +16,17 @@
 
 import QtQuick 2.2
 import QtContacts 5.0
+import Ubuntu.Content 1.1
 
 Item {
     id: root
 
     property var contactModel
     property var outputFile
+    property var activeTransfer: null
 
-    signal contactExported(int error, string url)
     signal contactsFetched(var contacts)
+    signal done()
 
     function start(contacts) {
         if (!contactModel) {
@@ -59,7 +61,20 @@
 
             onExportCompleted: {
                 priv.currentQueryId = -1
-                root.contactExported(error, root.outputFile)
+
+                // send contacts back to source app (pick mode)
+                if (error === ContactModel.ExportNoError) {
+                    var obj = Qt.createQmlObject("import Ubuntu.Content 1.1;  ContentItem { url: '" + url + "' }", root)
+                    if (root.activeTransfer) {
+                        root.activeTransfer.items = [obj]
+                        root.activeTransfer.state = ContentTransfer.Charged
+                    } else {
+                        console.error("No active transfer")
+                    }
+                } else {
+                    console.error("Fail to export contacts:" + error)
+                }
+                root.done()
             }
 
             onContactsFetched: {
@@ -74,5 +89,16 @@
                 }
             }
         }
+
+        Connections {
+            target: root.activeTransfer
+
+            onStateChanged: {
+                if (root.activeTransfer.state === ContentTransfer.Aborted) {
+                    root.activeTransfer = null
+                    root.done()
+                }
+            }
+        }
     }
 }

=== modified file 'src/imports/ContactList/ContactListPage.qml'
--- src/imports/ContactList/ContactListPage.qml	2014-10-03 21:45:23 +0000
+++ src/imports/ContactList/ContactListPage.qml	2014-10-07 20:52:25 +0000
@@ -21,7 +21,7 @@
 import Ubuntu.Components.ListItems 1.0 as ListItem
 import Ubuntu.Components.Popups 1.0 as Popups
 import Ubuntu.Contacts 0.1 as ContactsUI
-import Ubuntu.Content 0.1 as ContentHub
+import Ubuntu.Content 1.1 as ContentHub
 
 import "../Common"
 
@@ -622,27 +622,13 @@
     ContactExporter {
         id: contactExporter
 
-        property var activeTransfer: null
-
         contactModel: contactList.listModel
         outputFile: mainPage.pickMode ? "file:///tmp/address_book_app_export.vcf" : ""
-        onContactExported: {
-            // send contacts back to source app (pick mode)
-            if (error === ContactModel.ExportNoError) {
-                var obj = Qt.createQmlObject("import Ubuntu.Content 0.1;  ContentItem { url: '" + url + "' }", contactExporter)
-                if (activeTransfer) {
-                    activeTransfer.items = [obj]
-                    activeTransfer.state = ContentHub.ContentTransfer.Charged
-                }  else {
-                    console.error("No active transfer")
-                }
-            } else {
-                console.error("Fail to export contacts:" + error)
-            }
-            activeTransfer = null
+
+        onDone: {
             mainPage.pickMode = false
             mainPage.state = "default"
-            application.returnVcard(url)
+            application.returnVcard(contactExporter.outputFile)
         }
 
         onContactsFetched: {