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

Proposed by Raúl Yeguas
Status: Merged
Approved by: Martin Borho
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
David Planella Needs Fixing
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.
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

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)
Revision history for this message
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

Fixed Row warning and added suncloud dummy image.

Revision history for this message
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!

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

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)
Revision history for this message
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

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

Revision history for this message
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?

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
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
=== added directory 'components'
=== added file 'components/CurrentWeather.qml'
--- components/CurrentWeather.qml 1970-01-01 00:00:00 +0000
+++ components/CurrentWeather.qml 2013-03-08 16:12:20 +0000
@@ -0,0 +1,112 @@
1import QtQuick 2.0
2import Ubuntu.Components 0.1
3import "OpenWeatherMap.js" as Weather
4
5Rectangle {
6 id: currentWeather
7
8 // For Status I take the same condition codes from OpenWeatherMap (http://openweathermap.org/wiki/API/Weather_Condition_Codes)
9 property int condition
10 property int currentTemp
11 property int minTemp
12 property int maxTemp
13 property bool metric
14
15 width: units.gu(50)
16 height: units.gu(60)
17
18 Component.onCompleted: {
19 if(currentWeather.metric){
20 tempCurrent.text = tempCurrent.text + ("C");
21 tempMin.text = tempMin.text + ("C");
22 tempMax.text = tempMax.text + ("C");
23 }else{
24 tempCurrent.text = tempCurrent.text + ("F");
25 tempMax.text = tempMax.text + ("F");
26 tempMin.text = tempMin.text + ("F");
27 }
28 }
29
30 Image {
31 id: image1
32 width: units.gu(30)
33 height: units.gu(30)
34 sourceSize.height: units.gu(30)
35 sourceSize.width: units.gu(30)
36 anchors.horizontalCenter: parent.horizontalCenter
37 source: "../resources/images/"+Weather.getConditionIcon(condition)+".jpg"
38 fillMode: Image.PreserveAspectFit
39 }
40
41 Rectangle {
42 id: rectangle1
43 width: units.gu(50)
44 height: units.gu(10)
45 anchors.top: image1.bottom
46 anchors.topMargin: 0
47 color: "#00000000"
48
49 Column {
50 id: column1
51 width: units.gu(5)
52 height: units.gu(5)
53 anchors.rightMargin: 0
54 anchors.verticalCenter: parent.verticalCenter
55 anchors.right: tempCurrent.left
56
57 Label {
58 text: i18n.tr("Max.")
59 anchors.horizontalCenter: parent.horizontalCenter
60 font.pixelSize: FontUtils.sizeToPixels("x-small")
61 horizontalAlignment: Text.AlignHCenter
62 }
63
64 Label {
65 id: tempMax
66 text: maxTemp+"°"
67 anchors.horizontalCenter: parent.horizontalCenter
68 font.pixelSize: FontUtils.sizeToPixels("medium")
69 horizontalAlignment: Text.AlignHCenter
70 }
71 }
72
73 Text {
74 width: units.gu(17)
75 height: units.gu(10)
76 id: tempCurrent
77 text: currentTemp+String("°")
78 style: Text.Normal
79 font.bold: false
80 font.family: "Ubuntu"
81 verticalAlignment: Text.AlignVCenter
82 anchors.horizontalCenter: parent.horizontalCenter
83 anchors.verticalCenter: parent.verticalCenter
84 font.pointSize: 52
85 horizontalAlignment: Text.AlignHCenter
86 }
87
88 Column {
89 id: column2
90 width: units.gu(5)
91 height: units.gu(5)
92 anchors.leftMargin: 0
93 anchors.verticalCenter: parent.verticalCenter
94 anchors.left: tempCurrent.right
95
96 Label {
97 text: i18n.tr("Min.")
98 horizontalAlignment: Text.AlignHCenter
99 anchors.horizontalCenter: parent.horizontalCenter
100 font.pixelSize: FontUtils.sizeToPixels("x-small")
101 }
102
103 Label {
104 id: tempMin
105 text: minTemp+"°"
106 horizontalAlignment: Text.AlignHCenter
107 anchors.horizontalCenter: parent.horizontalCenter
108 font.pixelSize: FontUtils.sizeToPixels("medium")
109 }
110 }
111 }
112}
0113
=== added file 'components/OpenWeatherMap.js'
--- components/OpenWeatherMap.js 1970-01-01 00:00:00 +0000
+++ components/OpenWeatherMap.js 2013-03-08 16:12:20 +0000
@@ -0,0 +1,49 @@
1.pragma library
2
3function getConditionIcon(condition){
4 var map = {
5 "200": "11",
6 "201": "11",
7 "202": "11",
8 "210": "11",
9 "211": "11",
10 "212": "11",
11 "221": "11",
12 "230": "11",
13 "231": "11",
14 "232": "11",
15 "300": "09",
16 "301": "09",
17 "302": "09",
18 "310": "09",
19 "311": "09",
20 "312": "09",
21 "321": "09",
22 "500": "10",
23 "501": "10",
24 "502": "10",
25 "503": "10",
26 "504": "10",
27 "511": "13",
28 "520": "09",
29 "521": "09",
30 "522": "09",
31 "600": "13",
32 "601": "13",
33 "602": "13",
34 "611": "13",
35 "621": "13",
36 "701": "50",
37 "711": "50",
38 "721": "50",
39 "731": "50",
40 "741": "50",
41 "800": "01",
42 "801": "02",
43 "802": "03",
44 "803": "04",
45 "804": "04"
46 }
47
48 return map[condition];
49}
050
=== added directory 'resources'
=== added directory 'resources/images'
=== added file 'resources/images/02.jpg'
1Binary files resources/images/02.jpg 1970-01-01 00:00:00 +0000 and resources/images/02.jpg 2013-03-08 16:12:20 +0000 differ51Binary 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
=== modified file 'weather.qml'
--- weather.qml 2013-02-12 16:27:32 +0000
+++ weather.qml 2013-03-08 16:12:20 +0000
@@ -1,5 +1,6 @@
1import QtQuick 2.01import QtQuick 2.0
2import Ubuntu.Components 0.12import Ubuntu.Components 0.1
3import "components" as Components
34
4/*!5/*!
5 \brief MainView with Tabs element.6 \brief MainView with Tabs element.
@@ -27,9 +28,14 @@
27 // Tab content begins here28 // Tab content begins here
28 page: Page {29 page: Page {
29 Column {30 Column {
31 id: column1
30 anchors.centerIn: parent32 anchors.centerIn: parent
31 Label {33 Components.CurrentWeather {
32 text: i18n.tr("Swipe from right to left to change tab.")34 condition: 801
35 currentTemp: 20
36 minTemp: 12
37 maxTemp: 25
38 metric: true
33 }39 }
34 }40 }
35 }41 }

Subscribers

People subscribed via source and target branches