ubuntu-touch-coreapps-reviewers team mailing list archive

[Nekhelesh Ramananthan <krnekhelesh@xxxxxxxxx>]

Review: Needs Fixing manual testing + preliminary code review

A couples of fixed needed,

1. In the Settings Page, the time & date are shown as undefined.

2. The alarm function breaks (like in trunk) when editing alarms. But I guess we can tackle this in more detail in the next MP. So that's fine for now.

3. I would like a dedicated localized datetime property to be used here instead of localTimeString + localDateString analogy. Can you introduce a property in the c++ class called localizedDateTimeString and use that instead.

-                subText: {
-                    /*
-                  FIXME: When the upstream QT bug at
-                  https://bugreports.qt-project.org/browse/QTBUG-40275 is fixed
-                  it will be possible to receive a datetime object directly
-                  instead of using this hack.
-                */
-                    var localTime = new Date
-                            (
-                                localTimeSource.localDateString.split(":")[0],
-                                localTimeSource.localDateString.split(":")[1]-1,
-                                localTimeSource.localDateString.split(":")[2],
-                                localTimeSource.localTimeString.split(":")[0],
-                                localTimeSource.localTimeString.split(":")[1],
-                                localTimeSource.localTimeString.split(":")[2],
-                                localTimeSource.localTimeString.split(":")[3]
-                                )
-                    return localTime.toLocaleString()
-                }
-
+ subText: localTimeSource.localTimeString + " " + localTimeSource.localDateString

4. This code has a logical flaw. For instance let's say the local user date time (in Poland) is 17th September 2015, 00:30. If I change to London, the local date time should be 16th September 2015, 23:30. Notice that the date has changed from 17th to 16th september. However in the code below, the date is obtained using the javascript date object which doesn't work well with timezones and may not be aware of this change leading to bugs in the date.

-        localTime: clockTime
+        localTime: {
+            var date = new Date();
+            return new Date
+                (
+                    date.getFullYear(),
+                    date.getMonth(),
+                    date.getDate(),
+                    notLocalizedTimeString.split(":")[0],
+                    notLocalizedTimeString.split(":")[1],
+                    notLocalizedTimeString.split(":")[2],
+                    0
+                    )
+ }

5. Same issue as issue #4.

+        var date = new Date();
+        var clockTime = new Date
+                (
+                    date.getFullYear(),
+                    date.getMonth(),
+                    date.getDate(),
+                    clockTimeString.split(":")[0],
+                    clockTimeString.split(":")[1],
+                    clockTimeString.split(":")[2],
+                    0
+ )

6. Same issue as issue #4

+                    var date = new Date();
                     return new Date
                             (
-                                currentTime.localDateString.split(":")[0],
-                                currentTime.localDateString.split(":")[1] - 1,
-                                currentTime.localDateString.split(":")[2],
-                                currentTime.localTimeString.split(":")[0],
-                                Math.ceil((parseInt(currentTime.localTimeString
+                                date.getFullYear(),
+                                date.getMonth(),
+                                date.getDate(),
+                                currentTime.notLocalizedCurrentTimeString.split(":")[0],
+                                Math.ceil((parseInt(currentTime.notLocalizedCurrentTimeString
                                                    .split(":")[1]) + 1) / 5) * 5,
                                 0,
0

7. Please move the following changes to another MP for easier testing and regression tracking,

+    function get_current_utc_time() {
+        var localDate = new Date()
+        // FIXME Date() is not working correctly in runtime, when timezone is changed.
+        // To avoid issues with Date(), clock app needs to be restarted every timezone is changed
+        return new Date(localDate.getUTCFullYear(),
+                        localDate.getUTCMonth(),
+                        localDate.getUTCDate(),
+                        localDate.getUTCHours(),
+                        localDate.getUTCMinutes(),
+                        localDate.getUTCSeconds(),
+                        localDate.getUTCMilliseconds())
+    }
+
     Component.onCompleted: {
         console.log("[LOG]: Clock Page loaded")
         otherElementsStartUpAnimation.start()
@@ -106,14 +125,17 @@
              If Clock App is brought from background after more than 30 mins,
              query the user location to ensure it is up to date.
             */
+            // FIXME Date() is not working correctly in runtime, when timezone is changed.
+            // To avoid issues with Date(), clock app needs to be restarted every timezone is changed
+            var currentUTCTime = get_current_utc_time()
             if(applicationState
-                    && Math.abs(clock.analogTime - geoposition.lastUpdate) > 1800000) {
-                if(!geoposition.active)
+                    && Math.abs(currentUTCTime - geoposition.lastUpdate) > 1800000) {
+                if(!geoposition.active) {
+                    console.log("[LOG]: Starting geolocation update service at UTC time: " + currentUTCTime)
                     geoposition.start()
-            }
-
-            else if (!applicationState) {
-                geoposition.lastUpdate = clock.analogTime
+                }
+            } else if (!applicationState) {
+ geoposition.lastUpdate = currentUTCTime
-- 
https://code.launchpad.net/~gang65/ubuntu-clock-app/ubuntu-clock-mainclock-runtime-timezone-fix/+merge/271033
Your team Ubuntu Clock Developers is subscribed to branch lp:ubuntu-clock-app.