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

Proposed by Martin Borho on 2013-07-06
Status: Merged
Approved by: David Planella on 2013-07-15
Approved revision: 61
Merged at revision: 63
Proposed branch: lp:~martin-borho/ubuntu-weather-app/settings-in-a-sheet
Merge into: lp:ubuntu-weather-app/obsolete.trunk
Diff against target: 378 lines (+182/-115)
5 files modified
components/LocationTab.qml (+20/-2)
components/SettingsPage.qml (+0/-80)
components/SettingsSheet.qml (+63/-0)
tests/autopilot/ubuntu_weather_app/tests/test_settings.py (+77/-28)
ubuntu-weather-app.qml (+22/-5)
To merge this branch: bzr merge lp:~martin-borho/ubuntu-weather-app/settings-in-a-sheet
Reviewer Review Type Date Requested Status
David Planella Approve on 2013-07-15
Günter Schwann (community) Approve on 2013-07-15
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve on 2013-07-11
Raúl Yeguas 2013-07-06 Pending
Review via email: mp+173312@code.launchpad.net

Commit message

Moved settings dialog into a ComposerSheet.

Description of the change

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.

To post a comment you must log in.
60. By Martin Borho on 2013-07-11

added settings icon

61. By Martin Borho on 2013-07-11

smaller modification in SettingsSheet

Günter Schwann (schwann) wrote :

Looks good, and works fine

review: Approve
David Planella (dpm) wrote :

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) wrote :

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) wrote :

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) wrote :

>
> 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) wrote :

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) wrote :

And really nice work on updating the tests as well!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'components/LocationTab.qml'
2--- components/LocationTab.qml 2013-07-10 13:46:10 +0000
3+++ components/LocationTab.qml 2013-07-11 18:53:25 +0000
4@@ -1,3 +1,21 @@
5+/*
6+ * Copyright (C) 2013 Canonical Ltd
7+ *
8+ * This program is free software: you can redistribute it and/or modify
9+ * it under the terms of the GNU General Public License version 3 as
10+ * published by the Free Software Foundation.
11+ *
12+ * This program is distributed in the hope that it will be useful,
13+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
14+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+ * GNU General Public License for more details.
16+ *
17+ * You should have received a copy of the GNU General Public License
18+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
19+ *
20+ * Authored by: Raúl Yeguas <neokore@gmail.com>
21+ * Martin Borho <martin@borho.net>
22+ */
23 import QtQuick 2.0
24 import Ubuntu.Components.Popups 0.1
25 import Ubuntu.Components 0.1
26@@ -87,11 +105,11 @@
27 objectName: "SettingsButton"
28 action:Action {
29 id: configAction
30- iconSource: Qt.resolvedUrl("../resources/images/refresh_icon.png")
31+ iconSource: Qt.resolvedUrl("../resources/images/settings.png")
32 text: i18n.tr("Settings")
33
34 onTriggered: {
35- mainView.showSettingsPage();
36+ mainView.showSettings();
37 }
38 }
39 }
40
41=== removed file 'components/SettingsPage.qml'
42--- components/SettingsPage.qml 2013-07-03 19:39:29 +0000
43+++ components/SettingsPage.qml 1970-01-01 00:00:00 +0000
44@@ -1,80 +0,0 @@
45-import QtQuick 2.0
46-import Ubuntu.Components 0.1
47-import Ubuntu.Components.Popups 0.1
48-
49-// Page for settings (units by now)
50-Page {
51- id: settingsPage
52- objectName: "SettingsPage"
53- title: i18n.tr("Settings")
54- visible:false
55-
56- property bool settingsChanged: false
57-
58- tools: ToolbarItems {
59- locked: true
60- opened: true
61- back: ToolbarButton {
62- objectName: "BackButton"
63- text: i18n.tr("Back")
64- iconSource: Qt.resolvedUrl("../resources/images/back@18.png")
65- onTriggered: {
66- if(settingsChanged) {
67- if(imperialSwitch.checked){
68- storage.saveSetting("units","imperial");
69- }else{
70- storage.saveSetting("units","metric");
71- }
72- storage.getSettings(function(storedSettings) {
73- for(var settingName in storedSettings) {
74- settings[settingName] = storedSettings[settingName];
75- }
76- refreshData(true);
77- });
78- settingsChanged = false;
79- }
80- if(pageStack.depth === 2) pageStack.pop()
81- }
82- }
83- }
84-
85- Rectangle {
86- anchors {
87- right: parent.right
88- rightMargin: units.gu(1)
89- left: parent.left
90- leftMargin: units.gu(1)
91- top: parent.top
92- topMargin: units.gu(3)
93- }
94- Label {
95- id: imperialString
96- width: units.gu(21)
97- anchors {
98- verticalCenter: parent.verticalCenter
99- left: parent.left
100- }
101-
102- text: i18n.tr("Use imperial units")
103- }
104- Switch {
105- id: imperialSwitch
106- objectName: "ImperialSwitch"
107- anchors {
108- verticalCenter: parent.verticalCenter
109- right: parent.right
110- }
111- Component.onCompleted: {
112- storage.getSettings(function(storedSettings) {
113- if (storedSettings['units'] === 'imperial')
114- imperialSwitch.checked = true;
115- else
116- imperialSwitch.checked = false;
117- });
118- }
119- onCheckedChanged: {
120- settingsChanged = true;
121- }
122- }
123- }
124-}
125
126=== added file 'components/SettingsSheet.qml'
127--- components/SettingsSheet.qml 1970-01-01 00:00:00 +0000
128+++ components/SettingsSheet.qml 2013-07-11 18:53:25 +0000
129@@ -0,0 +1,63 @@
130+/*
131+ * Copyright (C) 2013 Canonical Ltd
132+ *
133+ * This program is free software: you can redistribute it and/or modify
134+ * it under the terms of the GNU General Public License version 3 as
135+ * published by the Free Software Foundation.
136+ *
137+ * This program is distributed in the hope that it will be useful,
138+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
139+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
140+ * GNU General Public License for more details.
141+ *
142+ * You should have received a copy of the GNU General Public License
143+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
144+ *
145+ * Authored by: Raúl Yeguas <neokore@gmail.com>
146+ * Martin Borho <martin@borho.net>
147+ */
148+import QtQuick 2.0
149+import Ubuntu.Components 0.1
150+import Ubuntu.Components.Popups 0.1
151+import Ubuntu.Components.ListItems 0.1 as ListItem
152+
153+Component {
154+
155+ ComposerSheet {
156+ id: sheet
157+ objectName: "SettingsSheet"
158+ title: i18n.tr("Settings")
159+ contentsHeight: parent.height
160+
161+ container: ListItem.ValueSelector {
162+ id: unitsSelector
163+ objectName: "UnitsSelector"
164+ text: i18n.tr("Temperature unit")
165+ values: [i18n.tr("°C"), i18n.tr("°F")]
166+ selectedIndex: (settings["units"] === "imperial") ? 1 : 0;
167+ }
168+
169+ onConfirmClicked: {
170+ var refresh = false,
171+ selectedUnit = (unitsSelector.selectedIndex === 0) ? "metric" : "imperial";
172+ // check if temperaure scale was changed
173+ if(settings["units"] !== selectedUnit) {
174+ storage.saveSetting("units", selectedUnit);
175+ refresh = true;
176+ }
177+ // handling of other settings here
178+ // ....
179+
180+ // a setting was changed, reload settings and refresh the location tabs
181+ if(refresh === true) {
182+ storage.getSettings(function(storedSettings) {
183+ for(var settingName in storedSettings) {
184+ settings[settingName] = storedSettings[settingName];
185+ }
186+ refreshData(true);
187+ });
188+ }
189+ PopupUtils.close(sheet)
190+ }
191+ }
192+}
193
194=== added file 'resources/images/settings.png'
195Binary 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
196=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_settings.py'
197--- tests/autopilot/ubuntu_weather_app/tests/test_settings.py 2013-07-03 19:41:03 +0000
198+++ tests/autopilot/ubuntu_weather_app/tests/test_settings.py 2013-07-11 18:53:25 +0000
199@@ -24,47 +24,96 @@
200 self.assertThat(
201 self.main_window.get_qml_view().visible, Eventually(Equals(True)))
202
203- def _open_settings_page(self):
204- """Opens the settings page"""
205+ def _open_settings_sheet(self):
206+ """Opens the settings sheet"""
207 self.main_window.open_toolbar()
208 self.assertThat(self.main_window.get_toolbar().opened, Eventually(Equals(True)))
209 self.main_window.click_toolbar_button("SettingsButton")
210
211+ def _move_pointer_around(self):
212+ """Helper method to move the pointer around, to assure selector is opened"""
213+ sheet = self.main_window.get_object('ComposerSheet', 'SettingsSheet')
214+ self.pointing_device.move_to_object(sheet)
215+
216+ def _click_confirm(self):
217+ """Clicks the confirm button"""
218+ self.pointing_device.click_object(self.main_window.app.select_many('Button')[2])
219+
220+ def _click_cancel(self):
221+ """Clicks the cancel button"""
222+ self.pointing_device.click_object(self.main_window.app.select_many('Button')[1])
223+
224+ def _check_units(self, units):
225+ """Checks selected units by values from the first location tab"""
226+ current_temps = self.main_window.get_objects('QQuickText', 'CurrentTempText')
227+ if units == "imperial":
228+ self.assertThat(current_temps[0].text, Eventually(Equals(u'69°F')))
229+ self.assertThat(current_temps[1].text, Eventually(Equals(u'78°F')))
230+ else:
231+ self.assertThat(current_temps[0].text, Eventually(Equals(u'21°C')))
232+ self.assertThat(current_temps[1].text, Eventually(Equals(u'26°C')))
233+
234 def test_switch_scale(self):
235 """Tests switching the scale in the settings"""
236- # first check metric values
237- current_temps = self.main_window.get_objects('QQuickText', 'CurrentTempText')
238- self.assertThat(current_temps[0].text, Eventually(Equals(u'21°C')))
239- self.assertThat(current_temps[1].text, Eventually(Equals(u'26°C')))
240-
241- # open the settings and switch to imperial
242- self._open_settings_page()
243- switch = self.main_window.get_object('CheckBox', 'ImperialSwitch')
244- self.assertThat(switch.visible, Eventually(Equals(True)))
245- self.assertThat(switch.checked, Eventually(Equals(False)))
246- self.pointing_device.click_object(switch)
247- self.assertThat(switch.checked, Eventually(Equals(True)))
248- self.main_window.click_toolbar_button("BackButton")
249+ # first check metric values and open the settings sheet
250+ self._check_units('metric')
251+ self._open_settings_sheet()
252+
253+ # open the value selector
254+ units_selector = self.main_window.get_object('ValueSelector', "UnitsSelector")
255+ self.assertThat(units_selector.selectedIndex, Eventually(Equals(0)))
256+ self.pointing_device.click_object(units_selector)
257+
258+ # choose second option, fahrenheit
259+ self._move_pointer_around()
260+ imperial_option = units_selector.get_children()[3]
261+ self.pointing_device.click_object(imperial_option)
262+ self.assertThat(units_selector.selectedIndex, Eventually(Equals(1)))
263+ self._click_confirm()
264
265 # wait for reload and check the imperial values
266 load_indicator = self.main_window.get_object('ActivityIndicator', 'LoadingSpinner')
267 self.assertThat(load_indicator.running, Eventually(Equals(False)))
268- current_temps = self.main_window.get_objects('QQuickText', 'CurrentTempText')
269- self.assertThat(current_temps[0].text, Eventually(Equals(u'69°F')))
270- self.assertThat(current_temps[1].text, Eventually(Equals(u'78°F')))
271+ self._check_units('imperial')
272
273 # switch back to metric values again
274- self._open_settings_page()
275- switch = self.main_window.get_object('CheckBox', 'ImperialSwitch')
276- self.assertThat(switch.visible, Eventually(Equals(True)))
277- self.assertThat(switch.checked, Eventually(Equals(True)))
278- self.pointing_device.click_object(switch)
279- self.assertThat(switch.checked, Eventually(Equals(False)))
280- self.main_window.click_toolbar_button("BackButton")
281+ self._open_settings_sheet()
282+ units_selector = self.main_window.get_object('ValueSelector', "UnitsSelector")
283+ self.assertThat(units_selector.selectedIndex, Eventually(Equals(1)))
284+ self.pointing_device.click_object(units_selector)
285+ self.assertThat(units_selector.expanded, Eventually(Equals(True)))
286+
287+ # click celsius option
288+ self._move_pointer_around()
289+ metric_option = units_selector.get_children()[4]
290+ self.pointing_device.click_object(metric_option)
291+
292+ # confirm
293+ self.assertThat(units_selector.selectedIndex, Eventually(Equals(0)))
294+ self._click_confirm()
295
296 # wait for reload and check the metric values again
297 load_indicator = self.main_window.get_object('ActivityIndicator', 'LoadingSpinner')
298 self.assertThat(load_indicator.running, Eventually(Equals(False)))
299- current_temps = self.main_window.get_objects('QQuickText', 'CurrentTempText')
300- self.assertThat(current_temps[0].text, Eventually(Equals(u'21°C')))
301- self.assertThat(current_temps[1].text, Eventually(Equals(u'26°C')))
302\ No newline at end of file
303+ self._check_units('metric')
304+
305+ def test_switch_scale_cancel(self):
306+ """Test canceling switching scale"""
307+ # first check metric values and open the settings sheet
308+ self._check_units('metric')
309+ self._open_settings_sheet()
310+
311+ units_selector = self.main_window.get_object('ValueSelector', "UnitsSelector")
312+ self.assertThat(units_selector.selectedIndex, Eventually(Equals(0)))
313+ self.pointing_device.click_object(units_selector)
314+
315+ # choose second option, fahrenheit
316+ self._move_pointer_around()
317+ imperial_option = units_selector.get_children()[3]
318+ self.pointing_device.click_object(imperial_option)
319+ self.assertThat(units_selector.selectedIndex, Eventually(Equals(1)))
320+
321+ # cancel and check
322+ self._click_cancel()
323+ self._check_units('metric')
324+
325
326=== modified file 'ubuntu-weather-app.qml'
327--- ubuntu-weather-app.qml 2013-06-26 10:43:49 +0000
328+++ ubuntu-weather-app.qml 2013-07-11 18:53:25 +0000
329@@ -1,3 +1,21 @@
330+/*
331+ * Copyright (C) 2013 Canonical Ltd
332+ *
333+ * This program is free software: you can redistribute it and/or modify
334+ * it under the terms of the GNU General Public License version 3 as
335+ * published by the Free Software Foundation.
336+ *
337+ * This program is distributed in the hope that it will be useful,
338+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
339+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
340+ * GNU General Public License for more details.
341+ *
342+ * You should have received a copy of the GNU General Public License
343+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
344+ *
345+ * Authored by: Raúl Yeguas <neokore@gmail.com>
346+ * Martin Borho <martin@borho.net>
347+ */
348 import QtQuick 2.0
349 import Ubuntu.Components 0.1
350 import "components" as Components
351@@ -93,14 +111,13 @@
352 locationManager.loadData()
353 }
354
355- function showSettingsPage() {
356- pageStack.push(settingsPage)
357+ function showSettings() {
358+ PopupUtils.open(settingsSheet)
359 }
360
361 Component.onCompleted: {
362 //storage.clearDB();
363 //storage.clearSetting('units');
364- //storage.saveSetting("units", "imperial")
365 storage.getSettings(function(storedSettings) {
366 for(var settingName in storedSettings) {
367 settings[settingName] = storedSettings[settingName];
368@@ -109,8 +126,8 @@
369 })
370 }
371
372- Components.SettingsPage {
373- id: settingsPage
374+ Components.SettingsSheet {
375+ id: settingsSheet
376 }
377
378 Components.Storage{

Subscribers

People subscribed via source and target branches