Code review comment for lp:~nik90/ubuntu-clock-app/correct-time-locale

Revision history for this message
Victor Thompson (vthompson) wrote :

1. Currently the time does not show in the 12 hour locale. Please make the following change:

bzr diff
=== modified file 'app/clock/Clock.qml'
--- app/clock/Clock.qml 2014-06-30 13:39:47 +0000
+++ app/clock/Clock.qml 2014-06-30 22:42:42 +0000
@@ -95,11 +95,11 @@
                 }
                 else if(time.search(Qt.locale().amText) !== -1) {
                     // 12 hour format detected with the localised AM text
- return time.split(Qt.locale().amText)[0].slice(end-1)
+ return time.split(Qt.locale().amText)[0]
                 }
                 else {
                     // 12 hour format detected with the localised PM text
- return time.split(Qt.locale().pmText)[0].slice(end-1)
+ return time.split(Qt.locale().pmText)[0]
                 }
             }
         }

2. Each of places you check for amText/pmText would be simplified if you changed from doing this:

if (not AM and not PM)
else if (AM)
else

to be

if (AM)
else if (PM)
else // assumed to be 24 hour

3. Wouldn't it still be possible to parse the time separator and have it displayed in a different color?

review: Needs Fixing

« Back to merge proposal