Merge lp:~laney/ubuntu-system-settings/as-background-fixes into lp:ubuntu-system-settings

[Iain Lane]

[background] Correctly set the background in accountsservice. Make it update in the UI. Fix the "same/different" toggle. Update the reset button to reset the greeter background too, and to set the toggle back to "same".

Fix setting the greeter background.

We didn't maintain this code once the UI was hidden and it bitrotted (incorrect changes were added).

Correctly set the background in AS. Make it update in the UI. Fix the "same/different" toggle. Update the reset button to reset the greeter background too, and to set the toggle back to "same".

I didn't unhide this UI yet as it should be tested by others a bit first. Also we should decide what to do about the buttons. Probably get rid of "Change" and move the reset one underneath the OptionSelector?

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:473
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/478/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-saucy-amd64-ci/478

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/ubuntu-system-settings-ci/478/rebuild

review: Approve (continuous-integration)

[Łukasz Zemczak (sil2100)]

Looking good, actually I'm rejecting my previous merge as this one has all the things in it as well. As mentioned on IRC, I'll check if now we can use the .property() thing.

[Łukasz Zemczak (sil2100)]

I checked if .property() works - and sadly not. It's still returning an empty string for me when doing return userInterface.property("BackgroundFile").toString();, so let's stick with what you have here.

But there are some things that need fixing:

1) In the welcomeImage, you need:

+ source: backgroundPanel.backgroundFile

2) The Change... button needs to have the callback as well, otherwise it doesn't change anything (and breaking v1), so:

- onClicked: startContentTransfer()
+ onClicked: startContentTransfer(function(url) {
+ if (systemSettingsSettings.backgroundDuplicate) {
+ updateBoth(url)
+ } else {
+ updateHome(url)
+ systemSettingsSettings.backgroundSetLast = "home"
+ }
+ })

I guess that's all for now what I saw.

review: Needs Fixing

[Iain Lane (laney)]

On Thu, Oct 17, 2013 at 11:49:17AM -0000, Łukasz Zemczak wrote:
> […]
> 1) In the welcomeImage, you need:
>
> + source: backgroundPanel.backgroundFile

No. This goes through the test image. Did you see something break?

> 2) The Change... button needs to have the callback as well, otherwise it doesn't change anything (and breaking v1), so:
>
> - onClicked: startContentTransfer()
> + onClicked: startContentTransfer(function(url) {
> + if (systemSettingsSettings.backgroundDuplicate) {
> + updateBoth(url)
> + } else {
> + updateHome(url)
> + systemSettingsSettings.backgroundSetLast = "home"
> + }
> + })

Okay. It should actually be connected to the click action of the image
instead of duplicating the code.

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

[lp:~laney/ubuntu-system-settings/as-background-fixes]

474. By Iain Lane on 2013-10-17

Make the "Change" button do the same as clicking on the home image

[Iain Lane (laney)]

On Thu, Oct 17, 2013 at 12:55:14PM +0100, Iain Lane wrote:
> […]
> Okay. It should actually be connected to the click action of the image
> instead of duplicating the code.

Fixed r474

--
Iain Lane [ <email address hidden> ]
Debian Developer [ <email address hidden> ]
Ubuntu Developer [ <email address hidden> ]

[lp:~laney/ubuntu-system-settings/as-background-fixes]

475. By Iain Lane on 2013-10-17

Remove {}

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:475
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/479/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-saucy-amd64-ci/479

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/ubuntu-system-settings-ci/479/rebuild

review: Approve (continuous-integration)

[Łukasz Zemczak (sil2100)]

Now it's awesome. Thanks!

review: Approve

=== modified file 'plugins/background/MainPage.qml'
--- plugins/background/MainPage.qml	2013-10-15 12:04:10 +0000
+++ plugins/background/MainPage.qml	2013-10-17 12:15:08 +0000
@@ -62,6 +62,19 @@
         }
     }
 
+    function updateWelcome(imageUrl) {
+        backgroundPanel.backgroundFile = imageUrl
+    }
+
+    function updateHome(imageUrl) {
+        background.pictureUri = imageUrl
+    }
+
+    function updateBoth(imageUrl) {
+        updateWelcome(imageUrl)
+        updateHome(imageUrl)
+    }
+
     /* TODO: We hide the welcome screen parts for v1 -
        there's a lot of elements to hide */
 
@@ -89,7 +102,14 @@
             left: parent.left
          }
 
-        onClicked: startContentTransfer()
+        onClicked: startContentTransfer(function(url) {
+            if (systemSettingsSettings.backgroundDuplicate) {
+                updateBoth(url)
+            } else {
+                updateWelcome(url)
+                systemSettingsSettings.backgroundSetLast = "welcome"
+            }
+        })
         Component.onCompleted: updateImage(testWelcomeImage,
                                            welcomeImage)
 
@@ -108,7 +128,14 @@
             horizontalCenter: (showAllUI) ? undefined : parent.horizontalCenter
          }
 
-        onClicked: startContentTransfer()
+        onClicked: startContentTransfer(function(url) {
+            if (systemSettingsSettings.backgroundDuplicate) {
+                updateBoth(url)
+            } else {
+                updateHome(url)
+                systemSettingsSettings.backgroundSetLast = "home"
+            }
+        })
         Component.onCompleted: updateImage(testHomeImage,
                                            homeImage)
 
@@ -159,7 +186,8 @@
             text: i18n.tr("Change…")
             width: parent.width - units.gu(4)
             anchors.horizontalCenter: parent.horizontalCenter
-            onClicked: startContentTransfer()
+            Component.onCompleted:
+                clicked.connect(homeImage.clicked)
         }
 
         Button {
@@ -167,8 +195,13 @@
             width: parent.width - units.gu(4)
             anchors.horizontalCenter: parent.horizontalCenter
             onClicked: {
+                // Reset all of the settings
                 background.schema.reset('pictureUri')
-                setUpImages()
+                systemSettingsSettings.backgroundPreviouslySetValue =
+                        background.pictureUri
+                backgroundPanel.backgroundFile = background.pictureUri
+                systemSettingsSettings.backgroundSetLast = "home"
+                optionSelector.selectedIndex = 0 // Same
             }
         }
 
@@ -208,6 +241,12 @@
 
     Image {
         id: testWelcomeImage
+
+        function update(uri) {
+            // Will update source
+            updateWelcome(uri)
+        }
+
         property string fallback: defaultBackground
         visible: false
         onStatusChanged: updateImage(testWelcomeImage,
@@ -216,6 +255,12 @@
 
     Image {
         id: testHomeImage
+
+        function update(uri) {
+            // Will update source
+            updateHome(uri)
+        }
+
         property string fallback: defaultBackground
         source: background.pictureUri
         visible: false
@@ -234,11 +279,11 @@
             systemSettingsSettings.backgroundPreviouslySetValue =
                     leastRecent.source
             /* copy most recently changed to least recently changed */
-            leastRecent.source = mostRecent.source
+            leastRecent.update(mostRecent.source)
         } else { // different
             /* restore least recently changed to previous value */
-            leastRecent.source =
-                    systemSettingsSettings.backgroundPreviouslySetValue
+            leastRecent.update(
+                    systemSettingsSettings.backgroundPreviouslySetValue)
         }
     }
 
@@ -256,21 +301,25 @@
     property var activeTransfer
 
     Connections {
+        id: contentHubConnection
+        property var imageCallback
         target: activeTransfer ? activeTransfer : null
         onStateChanged: {
             if (activeTransfer.state === ContentTransfer.Charged) {
                 if (activeTransfer.items.length > 0) {
                     var imageUrl = activeTransfer.items[0].url;
-                    background.pictureUri = imageUrl;
-                    setUpImages();
+                    imageCallback(imageUrl);
                 }
             }
         }
     }
 
-    function startContentTransfer() {
-        var transfer = ContentHub.importContent(ContentType.Pictures,
-                                                ContentHub.defaultSourceForType(ContentType.Pictures));
+    function startContentTransfer(callback) {
+        if (callback)
+            contentHubConnection.imageCallback = callback
+        var transfer = ContentHub.importContent(
+                    ContentType.Pictures,
+                    ContentHub.defaultSourceForType(ContentType.Pictures));
         if (transfer != null)
         {
             transfer.selectionType = ContentTransfer.Single;

=== modified file 'plugins/background/background.cpp'
--- plugins/background/background.cpp	2013-07-21 19:25:47 +0000
+++ plugins/background/background.cpp	2013-10-17 12:15:08 +0000
@@ -55,12 +55,18 @@
     QDBusInterface userInterface (
                 "org.freedesktop.Accounts",
                 m_objectPath,
-                "org.freedesktop.Accounts.User",
+                "org.freedesktop.DBus.Properties",
                 m_systemBusConnection,
                 this);
 
     if (userInterface.isValid()) {
-        return userInterface.property("BackgroundFile").toString();
+        QDBusReply<QDBusVariant> answer = userInterface.call (
+                    "Get",
+                    "org.freedesktop.Accounts.User",
+                    "BackgroundFile");
+
+        if (answer.isValid())
+            return answer.value().variant().toString();
     }
 
     return QString();
@@ -84,6 +90,7 @@
     QString backgroundFileSave = backgroundFile.path();
     m_backgroundFile = backgroundFileSave;
     userInterface.call("SetBackgroundFile", backgroundFileSave);
+    Q_EMIT backgroundFileChanged();
 }
 
 void Background::slotChanged()