Merge lp:~neokore/ubuntu-weather-app/CurrentWeatherComponent into lp:ubuntu-weather-app/obsolete.trunk

Proposed by Raúl Yeguas on 2013-03-04
Status: Merged
Approved by: Martin Borho on 2013-03-09
Approved revision: 5
Merged at revision: 3
Proposed branch: lp:~neokore/ubuntu-weather-app/CurrentWeatherComponent
Merge into: lp:ubuntu-weather-app/obsolete.trunk
Diff against target: 203 lines (+169/-2)
3 files modified
components/CurrentWeather.qml (+112/-0)
components/OpenWeatherMap.js (+49/-0)
weather.qml (+8/-2)
To merge this branch: bzr merge lp:~neokore/ubuntu-weather-app/CurrentWeatherComponent
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve on 2013-03-08
David Planella 2013-03-04 Needs Fixing on 2013-03-07
Review via email: mp+151606@code.launchpad.net

Commit message

Added the current weather component, a javascript function to get the condition icon and a dummy image from the Mika Meskanen's design.

Description of the change

Added the current weather component, a javascript function to get the condition icon and a dummy image from the Mika Meskanen's design.

To post a comment you must log in.

FAILED: Continuous integration, rev:3
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~neokore/ubuntu-weather-app/CurrentWeatherComponent/+merge/151606/+edit-commit-message

http://91.189.93.125:8080/job/ubuntu-weather-app-ci/2/
Executed test runs:
    SUCCESS: http://91.189.93.125:8080/job/ubuntu-weather-app-ci/./build=pbuilder,distribution=quantal,flavor=amd64/2/console

Click here to trigger a rebuild:
http://91.189.93.125:8080/job/ubuntu-weather-app-ci/2//rebuild/?

review: Needs Fixing (continuous-integration)
Martin Borho (martin-borho) wrote :

Hi Raul,

when running in qmlscene I get the following warnings:

file:///home/mborho/ubuntu/CurrentWeatherComponent/components/CurrentWeather.qml:29:5: QML Row: Cannot specify left, right, horizontalCenter, fill or centerIn anchors for items inside Row. Row will not function.
file:///home/mborho/ubuntu/CurrentWeatherComponent/components/CurrentWeather.qml:17:5: QML QQuickImage: Cannot open: file:///home/mborho/ubuntu/CurrentWeatherComponent/resources/images/suncloud.jpg

Cheers
Martin

4. By Raúl Yeguas on 2013-03-05

Fixed Row warning and added suncloud dummy image.

Raúl Yeguas (neokore) wrote :

I updated the branch.
Thinking in widgets I used a row to keep these 3 items inline and then I used alingment. Now I use a simple rectangle instead of the row.
I uploaded the dummy image (I forgot to add it to bzr).

Thank you Martin! Cheers!

FAILED: Continuous integration, rev:4
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~neokore/ubuntu-weather-app/CurrentWeatherComponent/+merge/151606/+edit-commit-message

http://91.189.93.125:8080/job/ubuntu-weather-app-ci/3/
Executed test runs:
    SUCCESS: http://91.189.93.125:8080/job/ubuntu-weather-app-ci/./build=pbuilder,distribution=quantal,flavor=amd64/3/console

Click here to trigger a rebuild:
http://91.189.93.125:8080/job/ubuntu-weather-app-ci/3//rebuild/?

review: Needs Fixing (continuous-integration)
David Planella (dpm) wrote :

Thanks Raúl for your branch!

While I won't be reviewing all of the branches, I thought I'd provide some feedback in the hope that it is useful for future merge proposals:

1) Remember to set the commit message, otherwise the branch cannot be autocommitted to trunk once accepted, and you'll get the warning messages from Ubuntu Phone Apps Jenkins Bot

2) On line 29 of your diff, you're committing code that's commented out. If it's not functional, it's better to remove it than to commit dead code

3) Around line 69 of your diff I see `text: currentTemp+"°"` which means you're hardcoding Centigrade degrees. I'd recommend against using hardcoded units, as they won't be useful for those using Fahrenheit in the US, for example. Is there no Qt library that handles localized units?

Other than that, looks good, thanks!

[1] Either use the "Set commit message" link near the top of the page, or the direct link at
 https://code.launchpad.net/~neokore/ubuntu-weather-app/CurrentWeatherComponent/+merge/151606/+edit-commit-message

review: Needs Fixing
5. By Raúl Yeguas on 2013-03-08

Added 'metric' var to set Metric or Imperial units, deleted dead code to
use 'getConditionIcon' script and renamed dummy icon.

Raúl Yeguas (neokore) wrote :

Thanks for the advices.

I just set the commit message (my fault!) with the same text of description (I promise I'll make it better next time).

I changed the dead code and made a little change to hardcoded degrees symbol to add a 'C' or a 'F' to text depending on a variable, because "°" represent only degree, it does not especify Celsius or Farenheit (my fault again).

Unfortunately, I couldn't find any QML library to handle it automatically and as we should convert Kelvin degrees into Farenheit or Celsius, maybe we can use a "metric" variable in app settings. Of course, it could be better if we get it from general device settings.

Any idea about that?

Martin Borho (martin-borho) wrote :

> Any idea about that?

Openweathermap.org (and all other weather services) allows to define the metric (F or C) via a request param. So if we store an app setting for the preferred metric, we will get the right values from the api, without further calculations on our side.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added directory 'components'
2=== added file 'components/CurrentWeather.qml'
3--- components/CurrentWeather.qml 1970-01-01 00:00:00 +0000
4+++ components/CurrentWeather.qml 2013-03-08 16:12:20 +0000
5@@ -0,0 +1,112 @@
6+import QtQuick 2.0
7+import Ubuntu.Components 0.1
8+import "OpenWeatherMap.js" as Weather
9+
10+Rectangle {
11+ id: currentWeather
12+
13+ // For Status I take the same condition codes from OpenWeatherMap (http://openweathermap.org/wiki/API/Weather_Condition_Codes)
14+ property int condition
15+ property int currentTemp
16+ property int minTemp
17+ property int maxTemp
18+ property bool metric
19+
20+ width: units.gu(50)
21+ height: units.gu(60)
22+
23+ Component.onCompleted: {
24+ if(currentWeather.metric){
25+ tempCurrent.text = tempCurrent.text + ("C");
26+ tempMin.text = tempMin.text + ("C");
27+ tempMax.text = tempMax.text + ("C");
28+ }else{
29+ tempCurrent.text = tempCurrent.text + ("F");
30+ tempMax.text = tempMax.text + ("F");
31+ tempMin.text = tempMin.text + ("F");
32+ }
33+ }
34+
35+ Image {
36+ id: image1
37+ width: units.gu(30)
38+ height: units.gu(30)
39+ sourceSize.height: units.gu(30)
40+ sourceSize.width: units.gu(30)
41+ anchors.horizontalCenter: parent.horizontalCenter
42+ source: "../resources/images/"+Weather.getConditionIcon(condition)+".jpg"
43+ fillMode: Image.PreserveAspectFit
44+ }
45+
46+ Rectangle {
47+ id: rectangle1
48+ width: units.gu(50)
49+ height: units.gu(10)
50+ anchors.top: image1.bottom
51+ anchors.topMargin: 0
52+ color: "#00000000"
53+
54+ Column {
55+ id: column1
56+ width: units.gu(5)
57+ height: units.gu(5)
58+ anchors.rightMargin: 0
59+ anchors.verticalCenter: parent.verticalCenter
60+ anchors.right: tempCurrent.left
61+
62+ Label {
63+ text: i18n.tr("Max.")
64+ anchors.horizontalCenter: parent.horizontalCenter
65+ font.pixelSize: FontUtils.sizeToPixels("x-small")
66+ horizontalAlignment: Text.AlignHCenter
67+ }
68+
69+ Label {
70+ id: tempMax
71+ text: maxTemp+"°"
72+ anchors.horizontalCenter: parent.horizontalCenter
73+ font.pixelSize: FontUtils.sizeToPixels("medium")
74+ horizontalAlignment: Text.AlignHCenter
75+ }
76+ }
77+
78+ Text {
79+ width: units.gu(17)
80+ height: units.gu(10)
81+ id: tempCurrent
82+ text: currentTemp+String("°")
83+ style: Text.Normal
84+ font.bold: false
85+ font.family: "Ubuntu"
86+ verticalAlignment: Text.AlignVCenter
87+ anchors.horizontalCenter: parent.horizontalCenter
88+ anchors.verticalCenter: parent.verticalCenter
89+ font.pointSize: 52
90+ horizontalAlignment: Text.AlignHCenter
91+ }
92+
93+ Column {
94+ id: column2
95+ width: units.gu(5)
96+ height: units.gu(5)
97+ anchors.leftMargin: 0
98+ anchors.verticalCenter: parent.verticalCenter
99+ anchors.left: tempCurrent.right
100+
101+ Label {
102+ text: i18n.tr("Min.")
103+ horizontalAlignment: Text.AlignHCenter
104+ anchors.horizontalCenter: parent.horizontalCenter
105+ font.pixelSize: FontUtils.sizeToPixels("x-small")
106+ }
107+
108+ Label {
109+ id: tempMin
110+ text: minTemp+"°"
111+ horizontalAlignment: Text.AlignHCenter
112+ anchors.horizontalCenter: parent.horizontalCenter
113+ font.pixelSize: FontUtils.sizeToPixels("medium")
114+ }
115+ }
116+ }
117+}
118
119=== added file 'components/OpenWeatherMap.js'
120--- components/OpenWeatherMap.js 1970-01-01 00:00:00 +0000
121+++ components/OpenWeatherMap.js 2013-03-08 16:12:20 +0000
122@@ -0,0 +1,49 @@
123+.pragma library
124+
125+function getConditionIcon(condition){
126+ var map = {
127+ "200": "11",
128+ "201": "11",
129+ "202": "11",
130+ "210": "11",
131+ "211": "11",
132+ "212": "11",
133+ "221": "11",
134+ "230": "11",
135+ "231": "11",
136+ "232": "11",
137+ "300": "09",
138+ "301": "09",
139+ "302": "09",
140+ "310": "09",
141+ "311": "09",
142+ "312": "09",
143+ "321": "09",
144+ "500": "10",
145+ "501": "10",
146+ "502": "10",
147+ "503": "10",
148+ "504": "10",
149+ "511": "13",
150+ "520": "09",
151+ "521": "09",
152+ "522": "09",
153+ "600": "13",
154+ "601": "13",
155+ "602": "13",
156+ "611": "13",
157+ "621": "13",
158+ "701": "50",
159+ "711": "50",
160+ "721": "50",
161+ "731": "50",
162+ "741": "50",
163+ "800": "01",
164+ "801": "02",
165+ "802": "03",
166+ "803": "04",
167+ "804": "04"
168+ }
169+
170+ return map[condition];
171+}
172
173=== added directory 'resources'
174=== added directory 'resources/images'
175=== added file 'resources/images/02.jpg'
176Binary files resources/images/02.jpg 1970-01-01 00:00:00 +0000 and resources/images/02.jpg 2013-03-08 16:12:20 +0000 differ
177=== modified file 'weather.qml'
178--- weather.qml 2013-02-12 16:27:32 +0000
179+++ weather.qml 2013-03-08 16:12:20 +0000
180@@ -1,5 +1,6 @@
181 import QtQuick 2.0
182 import Ubuntu.Components 0.1
183+import "components" as Components
184
185 /*!
186 \brief MainView with Tabs element.
187@@ -27,9 +28,14 @@
188 // Tab content begins here
189 page: Page {
190 Column {
191+ id: column1
192 anchors.centerIn: parent
193- Label {
194- text: i18n.tr("Swipe from right to left to change tab.")
195+ Components.CurrentWeather {
196+ condition: 801
197+ currentTemp: 20
198+ minTemp: 12
199+ maxTemp: 25
200+ metric: true
201 }
202 }
203 }

Subscribers

People subscribed via source and target branches