Merge lp:~martin-borho/ubuntu-weather-app/settings-in-a-sheet into lp:ubuntu-weather-app/obsolete.trunk

[Martin Borho]

Moved settings dialog into a ComposerSheet.

First steps for solving Bug #1197364

* Settings dialog now moved in a ComposerSheet
* ValueSelector used for switching the temperature units
* updated tests accordingly
* removed SettingsPage, added SettingsSheet component

For resolving Bug #1197364 updated and/or new components are needed from the SDK, see bug report.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:59
http://91.189.93.70:8080/job/ubuntu-weather-app-ci/6/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-quantal-amd64-ci/6
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-raring-amd64-ci/6

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-weather-app-ci/6/rebuild

review: Approve (continuous-integration)

[lp:~martin-borho/ubuntu-weather-app/settings-in-a-sheet]

60. By Martin Borho on 2013-07-11

added settings icon

61. By Martin Borho on 2013-07-11

smaller modification in SettingsSheet

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:61
http://91.189.93.70:8080/job/ubuntu-weather-app-ci/8/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-precise-amd64-ci/1
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-quantal-amd64-ci/8
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-raring-amd64-ci/8
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-saucy-amd64-ci/1

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-weather-app-ci/8/rebuild

review: Approve (continuous-integration)

[Günter Schwann (schwann)]

Looks good, and works fine

review: Approve

[David Planella (dpm)]

Thanks Martin, nice work!

Just 3 small things: do you think you could address these?

- Make the text in the buttons
- Use Cancel and Done, as in the original wireframe [1]
- Make Cancel grey and Done Ubuntu orange

Once fixed, feel free to top-approve

[1] https://launchpadlibrarian.net/144073911/Weather%20app-settings.png

review: Needs Fixing

[Martin Borho (martin-borho)]

Hi David,

I'm still waiting for an updated Sheet component from the SDK, which should be the same as mentioned here:

https://bugs.launchpad.net/ubuntu-weather-app/+bug/1195243

Don't know how far that progressed ..

Cheers
Martin

[David Planella (dpm)]

Al 15/07/13 17:51, En/na David Planella ha escrit:
> Review: Needs Fixing
>
> Thanks Martin, nice work!
>
> Just 3 small things: do you think you could address these?
>
> - Make the text in the buttons

Sorry, that was supposed to mean "Make the text in the buttons start
with a capital letter"

Cheers,
David.

> - Use Cancel and Done, as in the original wireframe [1]
> - Make Cancel grey and Done Ubuntu orange
>
> Once fixed, feel free to top-approve
>
> [1] https://launchpadlibrarian.net/144073911/Weather%20app-settings.png
>

[Martin Borho (martin-borho)]

>
> Sorry, that was supposed to mean "Make the text in the buttons start
> with a capital letter"
>

I can't, the buttons are predefined fro the SDK

[David Planella (dpm)]

In that case, it looks good to me. The 3 issues mentioned should come from an update of the sheet component on the SDK, then.

review: Approve

[David Planella (dpm)]

And really nice work on updating the tests as well!

=== modified file 'components/LocationTab.qml'
--- components/LocationTab.qml	2013-07-10 13:46:10 +0000
+++ components/LocationTab.qml	2013-07-11 18:53:25 +0000
@@ -1,3 +1,21 @@
+/*
+ * Copyright (C) 2013 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * 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/>.
+ *
+ * Authored by: Raúl Yeguas <[e-mail]>
+ *              Martin Borho <[e-mail]>
+ */
 import QtQuick 2.0
 import Ubuntu.Components.Popups 0.1
 import Ubuntu.Components 0.1
@@ -87,11 +105,11 @@
                 objectName: "SettingsButton"
                 action:Action {
                     id: configAction
-                    iconSource: Qt.resolvedUrl("../resources/images/refresh_icon.png")
+                    iconSource: Qt.resolvedUrl("../resources/images/settings.png")
                     text: i18n.tr("Settings")
 
                     onTriggered: {
-                        mainView.showSettingsPage();
+                        mainView.showSettings();
                     }
                 }
             }

=== removed file 'components/SettingsPage.qml'
--- components/SettingsPage.qml	2013-07-03 19:39:29 +0000
+++ components/SettingsPage.qml	1970-01-01 00:00:00 +0000
@@ -1,80 +0,0 @@
-import QtQuick 2.0
-import Ubuntu.Components 0.1
-import Ubuntu.Components.Popups 0.1
-
-// Page for settings (units by now)
-Page {
-    id: settingsPage
-    objectName: "SettingsPage"
-    title: i18n.tr("Settings")
-    visible:false
-
-    property bool settingsChanged: false
-
-    tools: ToolbarItems {
-        locked: true
-        opened: true
-        back: ToolbarButton {
-            objectName: "BackButton"
-            text: i18n.tr("Back")
-            iconSource: Qt.resolvedUrl("../resources/images/[e-mail]")
-            onTriggered: {
-                if(settingsChanged) {
-                    if(imperialSwitch.checked){
-                      storage.saveSetting("units","imperial");
-                    }else{
-                      storage.saveSetting("units","metric");
-                    }
-                    storage.getSettings(function(storedSettings) {
-                        for(var settingName in storedSettings) {
-                            settings[settingName] = storedSettings[settingName];
-                        }
-                        refreshData(true);
-                    });
-                    settingsChanged = false;
-                }
-                if(pageStack.depth === 2) pageStack.pop()
-            }
-        }
-    }
-
-    Rectangle {
-        anchors {
-            right: parent.right
-            rightMargin: units.gu(1)
-            left: parent.left
-            leftMargin: units.gu(1)
-            top: parent.top
-            topMargin: units.gu(3)
-        }
-        Label {
-            id: imperialString
-            width: units.gu(21)
-            anchors {
-                verticalCenter: parent.verticalCenter
-                left: parent.left
-            }
-
-            text: i18n.tr("Use imperial units")
-        }
-        Switch {
-            id: imperialSwitch
-            objectName: "ImperialSwitch"
-            anchors {
-                verticalCenter: parent.verticalCenter
-                right: parent.right
-            }
-            Component.onCompleted: {
-                storage.getSettings(function(storedSettings) {
-                    if (storedSettings['units'] === 'imperial')
-                        imperialSwitch.checked = true;
-                    else
-                        imperialSwitch.checked = false;
-                });
-            }
-            onCheckedChanged: {
-                settingsChanged = true;
-            }
-        }
-    }
-}

=== added file 'components/SettingsSheet.qml'
--- components/SettingsSheet.qml	1970-01-01 00:00:00 +0000
+++ components/SettingsSheet.qml	2013-07-11 18:53:25 +0000
@@ -0,0 +1,63 @@
+/*
+ * Copyright (C) 2013 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * 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/>.
+ *
+ * Authored by: Raúl Yeguas <[e-mail]>
+ *              Martin Borho <[e-mail]>
+ */
+import QtQuick 2.0
+import Ubuntu.Components 0.1
+import Ubuntu.Components.Popups 0.1
+import Ubuntu.Components.ListItems 0.1 as ListItem
+
+Component {
+
+    ComposerSheet {
+        id: sheet
+        objectName: "SettingsSheet"
+        title: i18n.tr("Settings")
+        contentsHeight: parent.height
+
+        container:     ListItem.ValueSelector {
+            id: unitsSelector
+            objectName: "UnitsSelector"
+            text: i18n.tr("Temperature unit")
+            values: [i18n.tr("°C"), i18n.tr("°F")]
+            selectedIndex: (settings["units"] === "imperial") ? 1 : 0;
+        }
+
+        onConfirmClicked: {
+            var refresh = false,
+                selectedUnit = (unitsSelector.selectedIndex === 0) ? "metric" : "imperial";
+            // check if temperaure scale was changed
+            if(settings["units"] !== selectedUnit) {
+                storage.saveSetting("units", selectedUnit);
+                refresh = true;
+            }
+            // handling of other settings here
+            // ....
+
+            // a setting was changed, reload settings and refresh the location tabs
+            if(refresh === true) {
+                storage.getSettings(function(storedSettings) {
+                    for(var settingName in storedSettings) {
+                        settings[settingName] = storedSettings[settingName];
+                    }
+                    refreshData(true);
+                });
+            }
+            PopupUtils.close(sheet)
+        }
+    }
+}

=== added file 'resources/images/settings.png'
Binary files resources/images/settings.png	1970-01-01 00:00:00 +0000 and resources/images/settings.png	2013-07-11 18:53:25 +0000 differ
=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_settings.py'
--- tests/autopilot/ubuntu_weather_app/tests/test_settings.py	2013-07-03 19:41:03 +0000
+++ tests/autopilot/ubuntu_weather_app/tests/test_settings.py	2013-07-11 18:53:25 +0000
@@ -24,47 +24,96 @@
         self.assertThat(
             self.main_window.get_qml_view().visible, Eventually(Equals(True)))
 
-    def _open_settings_page(self):
-        """Opens the settings page"""
+    def _open_settings_sheet(self):
+        """Opens the settings sheet"""
         self.main_window.open_toolbar()
         self.assertThat(self.main_window.get_toolbar().opened, Eventually(Equals(True)))
         self.main_window.click_toolbar_button("SettingsButton")
 
+    def _move_pointer_around(self):
+        """Helper method to move the pointer around, to assure selector is opened"""
+        sheet = self.main_window.get_object('ComposerSheet', 'SettingsSheet')
+        self.pointing_device.move_to_object(sheet)
+
+    def _click_confirm(self):
+        """Clicks the confirm button"""
+        self.pointing_device.click_object(self.main_window.app.select_many('Button')[2])
+
+    def _click_cancel(self):
+        """Clicks the cancel button"""
+        self.pointing_device.click_object(self.main_window.app.select_many('Button')[1])
+
+    def _check_units(self, units):
+        """Checks selected units by values from the first location tab"""
+        current_temps = self.main_window.get_objects('QQuickText', 'CurrentTempText')
+        if units == "imperial":
+            self.assertThat(current_temps[0].text, Eventually(Equals(u'69°F')))
+            self.assertThat(current_temps[1].text, Eventually(Equals(u'78°F')))
+        else:
+            self.assertThat(current_temps[0].text, Eventually(Equals(u'21°C')))
+            self.assertThat(current_temps[1].text, Eventually(Equals(u'26°C')))
+
     def test_switch_scale(self):
         """Tests switching the scale in the settings"""
-        # first check metric values
-        current_temps = self.main_window.get_objects('QQuickText', 'CurrentTempText')
-        self.assertThat(current_temps[0].text, Eventually(Equals(u'21°C')))
-        self.assertThat(current_temps[1].text, Eventually(Equals(u'26°C')))
-
-        # open the settings and switch to imperial
-        self._open_settings_page()
-        switch = self.main_window.get_object('CheckBox', 'ImperialSwitch')
-        self.assertThat(switch.visible, Eventually(Equals(True)))
-        self.assertThat(switch.checked, Eventually(Equals(False)))
-        self.pointing_device.click_object(switch)
-        self.assertThat(switch.checked, Eventually(Equals(True)))
-        self.main_window.click_toolbar_button("BackButton")
+        # first check metric values and open the settings sheet
+        self._check_units('metric')
+        self._open_settings_sheet()
+
+        # open the value selector
+        units_selector = self.main_window.get_object('ValueSelector', "UnitsSelector")
+        self.assertThat(units_selector.selectedIndex, Eventually(Equals(0)))
+        self.pointing_device.click_object(units_selector)
+
+        # choose second option, fahrenheit
+        self._move_pointer_around()
+        imperial_option = units_selector.get_children()[3]
+        self.pointing_device.click_object(imperial_option)
+        self.assertThat(units_selector.selectedIndex, Eventually(Equals(1)))
+        self._click_confirm()
 
         # wait for reload and check the imperial values
         load_indicator = self.main_window.get_object('ActivityIndicator', 'LoadingSpinner')
         self.assertThat(load_indicator.running, Eventually(Equals(False)))
-        current_temps = self.main_window.get_objects('QQuickText', 'CurrentTempText')
-        self.assertThat(current_temps[0].text, Eventually(Equals(u'69°F')))
-        self.assertThat(current_temps[1].text, Eventually(Equals(u'78°F')))
+        self._check_units('imperial')
 
         # switch back to metric values again
-        self._open_settings_page()
-        switch = self.main_window.get_object('CheckBox', 'ImperialSwitch')
-        self.assertThat(switch.visible, Eventually(Equals(True)))
-        self.assertThat(switch.checked, Eventually(Equals(True)))
-        self.pointing_device.click_object(switch)
-        self.assertThat(switch.checked, Eventually(Equals(False)))
-        self.main_window.click_toolbar_button("BackButton")
+        self._open_settings_sheet()
+        units_selector = self.main_window.get_object('ValueSelector', "UnitsSelector")
+        self.assertThat(units_selector.selectedIndex, Eventually(Equals(1)))
+        self.pointing_device.click_object(units_selector)
+        self.assertThat(units_selector.expanded, Eventually(Equals(True)))
+
+        # click celsius option
+        self._move_pointer_around()
+        metric_option = units_selector.get_children()[4]
+        self.pointing_device.click_object(metric_option)
+
+        # confirm
+        self.assertThat(units_selector.selectedIndex, Eventually(Equals(0)))
+        self._click_confirm()
 
         # wait for reload and check the metric values again
         load_indicator = self.main_window.get_object('ActivityIndicator', 'LoadingSpinner')
         self.assertThat(load_indicator.running, Eventually(Equals(False)))
-        current_temps = self.main_window.get_objects('QQuickText', 'CurrentTempText')
-        self.assertThat(current_temps[0].text, Eventually(Equals(u'21°C')))
-        self.assertThat(current_temps[1].text, Eventually(Equals(u'26°C')))
\ No newline at end of file
+        self._check_units('metric')
+
+    def test_switch_scale_cancel(self):
+        """Test canceling switching scale"""
+        # first check metric values and open the settings sheet
+        self._check_units('metric')
+        self._open_settings_sheet()
+
+        units_selector = self.main_window.get_object('ValueSelector', "UnitsSelector")
+        self.assertThat(units_selector.selectedIndex, Eventually(Equals(0)))
+        self.pointing_device.click_object(units_selector)
+
+        # choose second option, fahrenheit
+        self._move_pointer_around()
+        imperial_option = units_selector.get_children()[3]
+        self.pointing_device.click_object(imperial_option)
+        self.assertThat(units_selector.selectedIndex, Eventually(Equals(1)))
+
+        # cancel and check
+        self._click_cancel()
+        self._check_units('metric')
+

=== modified file 'ubuntu-weather-app.qml'
--- ubuntu-weather-app.qml	2013-06-26 10:43:49 +0000
+++ ubuntu-weather-app.qml	2013-07-11 18:53:25 +0000
@@ -1,3 +1,21 @@
+/*
+ * Copyright (C) 2013 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * 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/>.
+ *
+ * Authored by: Raúl Yeguas <[e-mail]>
+ *              Martin Borho <[e-mail]>
+ */
 import QtQuick 2.0
 import Ubuntu.Components 0.1
 import "components" as Components
@@ -93,14 +111,13 @@
         locationManager.loadData()
     }
 
-    function showSettingsPage() {
-        pageStack.push(settingsPage)
+    function showSettings() {
+        PopupUtils.open(settingsSheet)
     }
 
     Component.onCompleted: {
         //storage.clearDB();
         //storage.clearSetting('units');
-        //storage.saveSetting("units", "imperial")
         storage.getSettings(function(storedSettings) {
             for(var settingName in storedSettings) {
                 settings[settingName] = storedSettings[settingName];
@@ -109,8 +126,8 @@
         })
     }
 
-    Components.SettingsPage {
-        id: settingsPage
+    Components.SettingsSheet {
+        id: settingsSheet
     }
     
     Components.Storage{