ubuntu-touch-coreapps-reviewers team mailing list archive

[Nekhelesh Ramananthan <krnekhelesh@xxxxxxxxx>]

Review: Needs Fixing performance regression

The following code changes are not necessary. Let me explain the reasoning why,

These code changes would add additional roles to *every* world city delegate in the c++ side and cost us performance. Instead, let's stick with the original code that returns the correct timezone info (hh:mm) as expected and just pass the current local time of the user as a property to the worldcitydelegate.qml. So we save one additional role in the QAbsractModel.

Also do note that you added the extra role in the TimeZone Model class which is the base class. This role will be inherited unnecessarily by StaticTimeZoneModel and JsonTimeZoneModel class where this role is useless as we just show the time at that city.

If you want, instead of the hard code "hh:mm" format, you could return the time in Qt::DefaultLocaleShortDate format. 

+        RoleTimezoneId,
+        RoleNotLocalizedTimeString,
+        RoleLocalizedTimeString,

-    case RoleTimeString:
+    case RoleNotLocalizedTimeString:
         /*
          FIXME: Until https://bugreports.qt-project.org/browse/QTBUG-40275
          is fixed, we will have to return a string.
         */
-        return worldCityTime.toString("hh:mm");
+        return worldCityTime.toString("hh:mm:ss");
+    case RoleLocalizedTimeString:
+ return worldCityTime.time().toString(Qt::DefaultLocaleShortDate);

-- 
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.