Merge lp:~mihirsoni/ubuntu-calendar-app/1280598 into lp:ubuntu-calendar-app

[Mihir Soni]

Display event date in event details view.

Display event date in event details view.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:262
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/432/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2415
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-raring-amd64-ci/432
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-saucy-amd64-ci/432
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-trusty-amd64-ci/266

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/432/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

This is only an opinion, but I think it would be better to leave the hh:mm start and end times, but add a date field above it (date string should be per the user's locale):

Date 05/14/2014
Start 12:30
End 13:30

In addition to the locale date format, it might be nice to add the day of the week.

[lp:~mihirsoni/ubuntu-calendar-app/1280598]

263. By Mihir Soni on 2014-05-05

Added date as separate field

[Victor Thompson (vthompson)]

This looks good to me, BUT I'm not sure 'dd-MMM-yyyy' is always the appropriate date string to use. I would assume that there's a way to default to the user's locale default. If there is not a way to do this, then I do think the format used here is fairly universal.

review: Approve

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:263
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/433/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2416
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-raring-amd64-ci/433
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-saucy-amd64-ci/433
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-trusty-amd64-ci/267

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/433/rebuild

review: Approve (continuous-integration)

[Kunal Parmar (pkunal-parmar)]

we also need to consider the case when start date and end date is different.

[Kunal Parmar (pkunal-parmar)]

 + eventDate.value = e.startDateTime.toLocaleString(Qt.locale(),'dd-MMM-yyyy');

Format 'dd-MMM-yyyy' needs to be localized too like being done for time string.

        var timeFormat = i18n.tr("hh:mm");
        var startTime = e.startDateTime.toLocaleTimeString(Qt.locale(), timeFormat);

review: Needs Fixing

[lp:~mihirsoni/ubuntu-calendar-app/1280598]

264. By Mihir Soni on 2014-05-06

Date formate localized

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:264
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/434/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2426
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-saucy-amd64-ci/434
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-trusty-amd64-ci/268

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/434/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

Mihir,

I think we need something like the following before the translation of the date string:

        // TRANSLATORS: this is a time formatting string,
        // see http://qt-project.org/doc/qt-5.0/qtqml/qml-qtquick2-date.html#details for valid expressions

review: Needs Fixing

[Mihir Soni (mihirsoni)]

> Mihir,
>
> I think we need something like the following before the translation of the
> date string:
>
> // TRANSLATORS: this is a time formatting string,
> // see http://qt-project.org/doc/qt-5.0/qtqml/qml-
> qtquick2-date.html#details for valid expressions

Hi Victor.

I have already followed that for expressions, the link which is there is dead , so here is new link for your reference :-
http://qt-project.org/doc/qt-5/qml-qtqml-date.html#toLocaleDateString-method

[Kunal Parmar (pkunal-parmar)]

> > Mihir,
> >
> > I think we need something like the following before the translation of the
> > date string:
> >
> > // TRANSLATORS: this is a time formatting string,
> > // see http://qt-project.org/doc/qt-5.0/qtqml/qml-
> > qtquick2-date.html#details for valid expressions
>
> Hi Victor.
>
> I have already followed that for expressions, the link which is there is dead
> , so here is new link for your reference :-
> http://qt-project.org/doc/qt-5/qml-qtqml-date.html#toLocaleDateString-method

Victor is asking you to put below comment

 // TRANSLATORS: this is a time formatting string,
 // see http://qt-project.org/doc/qt-5.0/qtqml/qml-qtquick2-date.html#details for valid expressions

before following statements

8 +
9 + var dateFormat = i18n.tr("dd-MMM-yyyy")
10 + eventDate.value = e.startDateTime.toLocaleString(Qt.locale(),dateFormat);

to help translators, where for look for format.

[Victor Thompson (vthompson)]

But certainly the link *is* broken. It would be nice to use a generic URL that is not tied to a revision of Qt. This could be fixed in this branch or at a later time. If Mihir used the currently broken URL, someone could fix it shortly so translators have a better description.

[lp:~mihirsoni/ubuntu-calendar-app/1280598]

265. By Mihir Soni on 2014-05-08

Added date & time formatting string reference

266. By Mihir Soni on 2014-05-08

merge with trunk

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:266
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/445/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2496
        deb: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2496/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/30
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/30/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-trusty-amd64-ci/279
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-utopic-amd64-ci/3

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/445/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

I'm not sure, but I assume the commentary needs to be on top of each instance of i18n.tr() in order for it to be picked up in the translation strings. You could build the .pot file under the 'po' directory to be certain.

[Victor Thompson (vthompson)]

I've verified my above comment. You do need the commentary above each instance.

Also, you've jumbled the comment a bit and should add " see " to the second line.

[lp:~mihirsoni/ubuntu-calendar-app/1280598]

267. By Mihir Soni on 2014-05-08

Added comment for translators

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:267
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/448/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2507
        deb: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2507/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/41
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/41/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-trusty-amd64-ci/282
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-utopic-amd64-ci/6

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/448/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

Could you also update EventBubble.qml L38/39 with the same commentary and then update the translations per README.translations? Currently the instructions for "hh:mm" get added twice in the pot file because the commentary differs.

review: Needs Fixing

[lp:~mihirsoni/ubuntu-calendar-app/1280598]

268. By Mihir Soni on 2014-05-09

Added translators

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:268
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/454/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2554
        deb: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2554/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/90
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/90/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-trusty-amd64-ci/288
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-utopic-amd64-ci/12

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/454/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

Mihir, thanks! This looks great to me!

review: Approve

[Victor Thompson (vthompson)]

One small thing that should be fixed is the following. 1) there are two "sees" in EventBubble, and 2) the intent is to make it match the commentary for the other "hh:ss" instance so the instructions are only output once in the pot file:

=== modified file 'EventBubble.qml'
--- EventBubble.qml 2014-05-09 06:23:01 +0000
+++ EventBubble.qml 2014-05-10 16:58:48 +0000
@@ -35,8 +35,8 @@
             return;
         }

- // TRANSLATORS: this is a time formatting string,
- // see see http://qt-project.org/doc/qt-5/qml-qtqml-date.html#details f
or valid expressions
+ // TRANSLATORS: this is a time & Date formatting string,
+ //see http://qt-project.org/doc/qt-5/qml-qtqml-date.html#details

review: Needs Fixing

[lp:~mihirsoni/ubuntu-calendar-app/1280598]

269. By Mihir Soni on 2014-05-13

Merge from commit

270. By Mihir Soni on 2014-05-13

Merge from trunk

271. By Mihir Soni on 2014-05-13

remove typo

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:271
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/458/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2616
        deb: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2616/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/155
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/155/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-trusty-amd64-ci/292
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-utopic-amd64-ci/16

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/458/rebuild

review: Approve (continuous-integration)

[lp:~mihirsoni/ubuntu-calendar-app/1280598]

272. By Mihir Soni on 2014-05-15

match translator comments

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:272
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/459/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2642
        deb: http://91.189.93.70:8080/job/generic-mediumtests-trusty/2642/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/179
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/179/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-trusty-amd64-ci/293
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-calendar-app-utopic-amd64-ci/17

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-calendar-app-ci/459/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

lgtm! Thanks for revising this!

review: Approve

=== modified file 'EventBubble.qml'
--- EventBubble.qml	2014-05-09 07:52:41 +0000
+++ EventBubble.qml	2014-05-15 07:01:30 +0000
@@ -36,7 +36,7 @@
         }
 
         // TRANSLATORS: this is a time formatting string,
-        // see http://qt-project.org/doc/qt-5.0/qtqml/qml-qtquick2-date.html#details for valid expressions
+        // see http://qt-project.org/doc/qt-5/qml-qtqml-date.html#details for valid expressions
         var timeFormat = i18n.tr("hh:mm");
         var startTime = event.startDateTime.toLocaleTimeString(Qt.locale(), timeFormat)
         var endTime = event.endDateTime.toLocaleTimeString(Qt.locale(), timeFormat)

=== modified file 'EventDetails.qml'
--- EventDetails.qml	2014-05-08 00:57:11 +0000
+++ EventDetails.qml	2014-05-15 07:01:30 +0000
@@ -93,8 +93,12 @@
 
     function showEvent(e) {
         // TRANSLATORS: this is a time formatting string,
-        // see http://qt-project.org/doc/qt-5.0/qtqml/qml-qtquick2-date.html#details for valid expressions
+        // see http://qt-project.org/doc/qt-5/qml-qtqml-date.html#details for valid expressions
         var timeFormat = i18n.tr("hh:mm");
+        // TRANSLATORS: this is a time & Date formatting string,
+        //see http://qt-project.org/doc/qt-5/qml-qtqml-date.html#details
+        var dateFormat = i18n.tr("dd-MMM-yyyy")
+        eventDate.value = e.startDateTime.toLocaleString(Qt.locale(),dateFormat);
         var startTime = e.startDateTime.toLocaleTimeString(Qt.locale(), timeFormat);
         var endTime = e.endDateTime.toLocaleTimeString(Qt.locale(), timeFormat);
 
@@ -216,7 +220,12 @@
                 left:parent.left
                 leftMargin: units.gu(2)
             }
-            property int timeLabelMaxLen: Math.max( startHeader.headerWidth, endHeader.headerWidth)// Dynamic Width
+            property int timeLabelMaxLen: Math.max( startHeader.headerWidth, endHeader.headerWidth,eventDate.headerWidth)// Dynamic Width
+            EventDetailsInfo{
+                id: eventDate
+                xMargin:column.timeLabelMaxLen
+                header: i18n.tr("Date")
+            }
             EventDetailsInfo{
                 id: startHeader
                 xMargin:column.timeLabelMaxLen