Merge lp:~andrewsomething/ubuntu-weather-app/use_system_locales into lp:ubuntu-weather-app/obsolete.trunk

Proposed by Andrew Starr-Bochicchio
Status: Merged
Approved by: David Planella
Approved revision: 125
Merged at revision: 122
Proposed branch: lp:~andrewsomething/ubuntu-weather-app/use_system_locales
Merge into: lp:ubuntu-weather-app/obsolete.trunk
Diff against target: 70 lines (+15/-3)
5 files modified
components/CurrentWeather.qml (+2/-1)
components/LastUpdatedComponent.qml (+1/-1)
tests/autopilot/ubuntu_weather_app/tests/__init__.py (+7/-0)
tests/autopilot/ubuntu_weather_app/tests/test_settings.py (+1/-0)
ubuntu-weather-app.qml (+4/-1)
To merge this branch: bzr merge lp:~andrewsomething/ubuntu-weather-app/use_system_locales
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
David Planella Approve
Andrew Starr-Bochicchio (community) Approve
Martin Borho Approve
Review via email: mp+187301@code.launchpad.net

This proposal supersedes a proposal from 2013-09-20.

Commit message

Use the system locale to determine both the default units and the display of time.

Description of the change

Use the system locale to determine both the default units and the display of time.

To post a comment you must log in.
Revision history for this message
Martin Borho (martin-borho) wrote : Posted in a previous version of this proposal

Important fix, thanks a lot!

Unfortunately it fails to merge now with 2 conflicts, since last merge in trunk brought updates to the used dates and their internal representation. Sorry for that.

But should be easy to fix, though.

Cheers
Martin

review: Needs Fixing
Revision history for this message
Andrew Starr-Bochicchio (andrewsomething) wrote : Posted in a previous version of this proposal

Hmm... The autopilot tests really don't like that the default settings are dynamic.

Is it possible to import PyQt in the autopilot test? If so we could check Qt.QLocale.MetricSystem from the tests.

Revision history for this message
David Planella (dpm) wrote : Posted in a previous version of this proposal

9 + var dateTimeString = Qt.formatDateTime(dateTime, 'dddd, dd MMMM - ') + Qt.formatTime(dateTime);

While we are at it, it might be worth wrapping the date in i18n(), and adding a comment on top, so that translators can rearrange it if needed. E.g.

# TRANSLATORS: this is a date to which a time of the day will be appended
var dateTimeString = Qt.formatDateTime(dateTime, i18n.tr('dddd, dd MMMM - ')) + Qt.formatTime(dateTime);

Not sure about the question on PyQt, it might be worth asking Nick Skaggs (balloons on IRC) about it.

review: Needs Fixing
Revision history for this message
David Planella (dpm) wrote : Posted in a previous version of this proposal

Even better if we can make it:

# TRANSLATORS: this is a date to which a time of the day will be appended
var dateTimeString = Qt.formatDateTime(dateTime, i18n.tr('dddd, dd MMMM')) + ' - ' + Qt.formatTime(dateTime);

Or use a standard time format, so that it can be used directly without the need for translation (but I'm not sure if there is one for this combination of date and time).

Revision history for this message
Martin Borho (martin-borho) wrote : Posted in a previous version of this proposal

Hi Andrew,

Locale.MetricSystem and Locale.ImperialSystem are only constants and the Locale object may only be created via the Qt.locale() function, so the right conditions should be :

(Qt.locale().measurementSystem === Locale.MetricSystem)

But this leads to the same problem....

Perhaps, we can save the settings beforehand to the storage, like the test locations? See tests/autopilot/ubuntu_weather_app/tests/__init.py at the bottom.

Cheers
Martin

Docs:
http://qt-project.org/doc/qt-5.0/qtqml/qml-qtquick2-locale.html#measurementSystem-prop

Revision history for this message
Andrew Starr-Bochicchio (andrewsomething) wrote : Posted in a previous version of this proposal

Alright, I think this is ready to land now.

review: Needs Resubmitting
Revision history for this message
Martin Borho (martin-borho) wrote :

Great, good stuff! Thanks!

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

FAILED: Autolanding.
No commit message was specified in the merge proposal. Hit 'Add commit message' on the merge proposal web page or follow the link below. You can approve the merge proposal yourself to rerun.
https://code.launchpad.net/~andrewsomething/ubuntu-weather-app/use_system_locales/+merge/187301/+edit-commit-message

review: Needs Fixing (continuous-integration)
Revision history for this message
Andrew Starr-Bochicchio (andrewsomething) :
review: Approve
Revision history for this message
David Planella (dpm) wrote :

Looks good to me now, thanks!

review: Approve
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'components/CurrentWeather.qml'
2--- components/CurrentWeather.qml 2013-09-20 16:48:23 +0000
3+++ components/CurrentWeather.qml 2013-09-24 17:29:48 +0000
4@@ -88,7 +88,8 @@
5 adjustBackground(Math.round(temp));
6 var dateData = currentWeather.hourly.get(hourIndex).date;
7 var dateTime = new Date(dateData.year, dateData.month, dateData.date, dateData.hours, dateData.minutes)
8- var dateTimeString = Qt.formatDateTime(dateTime, 'dddd, dd MMMM - HH:mm');
9+ // TRANSLATORS: this is a date to which a time of the day will be appended
10+ var dateTimeString = Qt.formatDateTime(dateTime, i18n.tr('dddd, dd MMMM')) + ' - ' + Qt.formatTime(dateTime);
11 dateComponent.dateString = dateTimeString;
12 dateComponent.renderText();
13 }
14
15=== modified file 'components/LastUpdatedComponent.qml'
16--- components/LastUpdatedComponent.qml 2013-09-18 17:36:20 +0000
17+++ components/LastUpdatedComponent.qml 2013-09-24 17:29:48 +0000
18@@ -31,7 +31,7 @@
19 onUpdateDateChanged: {
20 var updatedLabel = i18n.tr("Updated: "),
21 updated = new Date(parseInt(updateDate)),
22- updateTime = Qt.formatTime(updated, 'HH:mm');
23+ updateTime = Qt.formatTime(updated);
24 if(Qt.formatDateTime(updated, "yyyy-MM-dd") === Qt.formatDateTime(new Date(), "yyyy-MM-dd")) {
25 updatedLabel += i18n.tr("Today, ")+updateTime;
26 } else {
27
28=== modified file 'tests/autopilot/ubuntu_weather_app/tests/__init__.py'
29--- tests/autopilot/ubuntu_weather_app/tests/__init__.py 2013-09-21 17:38:36 +0000
30+++ tests/autopilot/ubuntu_weather_app/tests/__init__.py 2013-09-24 17:29:48 +0000
31@@ -141,3 +141,10 @@
32 conn.commit()
33 conn.close()
34
35+ def save_units_to_storage(self):
36+ path = self.find_db()
37+ conn = sqlite3.connect(path)
38+ cursor = conn.cursor()
39+ cursor.execute("INSERT INTO settings(key, value) VALUES('units', 'metric'), ('wind_units', 'kmh')")
40+ conn.commit()
41+ conn.close()
42\ No newline at end of file
43
44=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_settings.py'
45--- tests/autopilot/ubuntu_weather_app/tests/test_settings.py 2013-09-16 13:15:48 +0000
46+++ tests/autopilot/ubuntu_weather_app/tests/test_settings.py 2013-09-24 17:29:48 +0000
47@@ -20,6 +20,7 @@
48 self.clean_db()
49 self.launch_and_quit_app()
50 self.save_locations_to_storage(locations_data)
51+ self.save_units_to_storage()
52 super(TestSettings, self).setUp()
53 self.assertThat(
54 self.get_qml_view().visible, Eventually(Equals(True)))
55
56=== modified file 'ubuntu-weather-app.qml'
57--- ubuntu-weather-app.qml 2013-09-21 17:38:36 +0000
58+++ ubuntu-weather-app.qml 2013-09-24 17:29:48 +0000
59@@ -43,7 +43,10 @@
60 property var locationsList: []
61 property var tabsObject: null
62 // set default values for settings here
63- property var settings: {"units":"metric", "wind_units": "kmh"}
64+ property var settings: {
65+ "units": Qt.locale().measurementSystem === Locale.MetricSystem ? "metric" : "imperial",
66+ "wind_units": Qt.locale().measurementSystem === Locale.MetricSystem ? "kmh" : "mph"
67+ }
68
69 WorkerScript {
70 id: locationDataWorker

Subscribers

People subscribed via source and target branches