Merge lp:~vthompson/ubuntu-weather-app/reboot-static-current-location into lp:ubuntu-weather-app

Proposed by Victor Thompson on 2015-06-10
Status: Merged
Approved by: Andrew Hayzen on 2015-06-14
Approved revision: 54
Merged at revision: 52
Proposed branch: lp:~vthompson/ubuntu-weather-app/reboot-static-current-location
Merge into: lp:ubuntu-weather-app
Diff against target: 210 lines (+113/-7)
2 files modified
app/ubuntu-weather-app.qml (+8/-2)
app/ui/LocationsPage.qml (+105/-5)
To merge this branch: bzr merge lp:~vthompson/ubuntu-weather-app/reboot-static-current-location
Reviewer Review Type Date Requested Status
Andrew Hayzen 2015-06-10 Approve on 2015-06-14
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve on 2015-06-14
Review via email: mp+261584@code.launchpad.net

Commit message

* Make current location static in LocationsPage.

Description of the change

* Make current location static in LocationsPage.

To post a comment you must log in.
51. By Victor Thompson on 2015-06-10

Always make current location's data visible when in multiselect mode.

52. By Victor Thompson on 2015-06-11

Cleanup

53. By Victor Thompson on 2015-06-11

Small update for divider

Andrew Hayzen (ahayzen) wrote :

Visual things
- If you tap the static list item the 'dim' part has margins can this be fixed so that it is full width?

Code
- I assume these errors
file:///opt/click.ubuntu.com/com.ubuntu.weather/3.0./share/qml/components/WeatherListItem.qml:42: TypeError: Cannot call method 'lastIndexOf' of undefined
file:///opt/click.ubuntu.com/com.ubuntu.weather/3.0./share/qml/components/WeatherListItem.qml:119:9: QML Connections: Cannot assign to non-existent property "onSelectAll"
file:///opt/click.ubuntu.com/com.ubuntu.weather/3.0./share/qml/components/WeatherListItem.qml:119:9: QML Connections: Cannot assign to non-existent property "onClearSelection"
file:///opt/click.ubuntu.com/com.ubuntu.weather/3.0./share/qml/components/WeatherListItem.qml:133: TypeError: Cannot call method 'indexOf' of undefined

Are due to the ListItem being in a non-multiselect ListView? This should probably be fixed by splitting the ListItem into two so you have a WeatherMultiSelectListItem or something?

review: Needs Fixing
54. By Victor Thompson on 2015-06-14

Update for item margins and undefined methods.

Victor Thompson (vthompson) wrote :

I resolved the margins issue and am now using a MultiSelectListView to fix the undefined methods issue.

Andrew Hayzen (ahayzen) wrote :

LGTM :-)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'app/ubuntu-weather-app.qml'
2--- app/ubuntu-weather-app.qml 2015-06-05 01:02:17 +0000
3+++ app/ubuntu-weather-app.qml 2015-06-14 21:17:34 +0000
4@@ -210,6 +210,10 @@
5 }
6
7 function moveLocation(from, to) {
8+ // Indexes are offset by 1 to account for current location
9+ from += 1
10+ to += 1
11+
12 // Update settings to respect new changes
13 if (from === settings.current) {
14 settings.current = to;
15@@ -226,6 +230,8 @@
16
17 // Remove a location from the list
18 function removeLocation(index) {
19+ // Indexes are offset by 1 to account for current location
20+ index += 1
21 if (settings.current >= index) { // Update settings to respect new changes
22 settings.current -= settings.current;
23 }
24@@ -242,7 +248,7 @@
25 indexes.sort(function(a,b) { return a - b })
26
27 for (i=0; i < indexes.length; i++) {
28- if (settings.current >= i) { // Update settings to respect new changes
29+ if (settings.current >= indexes[i] + 1) { // Update settings to respect new changes
30 settings.current -= settings.current;
31 }
32 }
33@@ -251,7 +257,7 @@
34 var locations = []
35
36 for (i=0; i < indexes.length; i++) {
37- locations.push(locationsList[indexes[i]].db.id)
38+ locations.push(locationsList[indexes[i] + 1].db.id)
39 }
40
41 storage.clearMultiLocation(locations);
42
43=== modified file 'app/ui/LocationsPage.qml'
44--- app/ui/LocationsPage.qml 2015-06-05 23:15:57 +0000
45+++ app/ui/LocationsPage.qml 2015-06-14 21:17:34 +0000
46@@ -54,6 +54,10 @@
47 }
48 ]
49
50+ ListModel {
51+ id: currentLocationModel
52+ }
53+
54 MultiSelectListView {
55 id: locationsListView
56 anchors {
57@@ -62,6 +66,90 @@
58 model: ListModel {
59 id: locationsModel
60 }
61+ header: MultiSelectListView {
62+ id: currentLocationListView
63+ anchors {
64+ left: parent.left
65+ right: parent.right
66+ }
67+ height: units.gu(8)
68+ interactive: false
69+ model: currentLocationModel
70+ delegate: WeatherListItem {
71+ id: currentLocationListItem
72+
73+ onItemClicked: {
74+ settings.current = index;
75+ pageStack.pop()
76+ }
77+
78+ Column {
79+ anchors {
80+ left: parent.left
81+ right: currentWeatherImage.left
82+ rightMargin: units.gu(1)
83+ verticalCenter: parent.verticalCenter
84+ }
85+
86+ Label {
87+ id: currentLocationName
88+
89+ anchors {
90+ left: parent.left
91+ leftMargin: units.gu(2)
92+ }
93+
94+ elide: Text.ElideRight
95+ fontSize: "medium"
96+ text: i18n.tr("Current Location")
97+ }
98+ Label {
99+ id: currentLocationName2
100+
101+ anchors {
102+ left: parent.left
103+ leftMargin: units.gu(2)
104+ }
105+
106+ color: UbuntuColors.lightGrey
107+ elide: Text.ElideRight
108+ fontSize: "small"
109+ font.weight: Font.Light
110+ text: name + ", " + (adminName1 == name ? countryName : adminName1)
111+ }
112+ }
113+
114+ Icon {
115+ id: currentWeatherImage
116+ anchors {
117+ right: parent.right
118+ rightMargin: units.gu(14)
119+ verticalCenter: parent.verticalCenter
120+ }
121+ name: icon
122+ height: units.gu(3)
123+ width: units.gu(3)
124+ }
125+
126+ Label {
127+ id: currentTemperatureLabel
128+ anchors {
129+ left: currentWeatherImage.right
130+ leftMargin: units.gu(1)
131+ right: parent.right
132+ rightMargin: units.gu(2)
133+ verticalCenter: parent.verticalCenter
134+ }
135+ color: UbuntuColors.orange
136+ elide: Text.ElideRight
137+ font.pixelSize: units.gu(4)
138+ font.weight: Font.Light
139+ horizontalAlignment: Text.AlignRight
140+ text: temp + settings.tempScale
141+ }
142+ }
143+ }
144+
145 delegate: WeatherListItem {
146 id: locationsListItem
147 leftSideAction: Remove {
148@@ -71,7 +159,7 @@
149 reorderable: true
150
151 onItemClicked: {
152- settings.current = index;
153+ settings.current = index + 1;
154 pageStack.pop()
155 }
156 onReorder: {
157@@ -80,6 +168,13 @@
158 storage.moveLocation(from, to);
159 }
160
161+ ListItem.ThinDivider {
162+ anchors {
163+ top: parent.top
164+ }
165+ visible: index == 0
166+ }
167+
168 Item {
169 anchors {
170 fill: parent
171@@ -99,7 +194,7 @@
172 id: locationName
173 elide: Text.ElideRight
174 fontSize: "medium"
175- text: index != 0 ? name : i18n.tr("Current Location")
176+ text: name
177 }
178 Label {
179 id: locationName2
180@@ -107,8 +202,7 @@
181 elide: Text.ElideRight
182 fontSize: "small"
183 font.weight: Font.Light
184- text: index != 0 ? (adminName1 == name ? countryName : adminName1)
185- : name + ", " + (adminName1 == name ? countryName : adminName1)
186+ text: adminName1 == name ? countryName : adminName1
187 }
188 }
189
190@@ -152,6 +246,7 @@
191 }
192
193 function populateLocationsModel() {
194+ currentLocationModel.clear()
195 locationsModel.clear()
196 var loc = {}, data = {},
197 tempUnits = settings.tempScale === "°C" ? "metric" : "imperial";
198@@ -166,7 +261,12 @@
199 "temp": Math.round(data.data[0].current[tempUnits].temp).toString(),
200 "icon": iconMap[data.data[0].current.icon]
201 }
202- locationsModel.append(loc)
203+
204+ if (i > 0) {
205+ locationsModel.append(loc)
206+ } else {
207+ currentLocationModel.append(loc)
208+ }
209 }
210 }
211

Subscribers

People subscribed via source and target branches

to all changes: