ubuntu-touch-coreapps-reviewers team mailing list archive

[Victor Thompson <victor.thompson@xxxxxxxxx>]

Review: Needs Information

2 inline comments.

Diff comments:

> === modified file 'app/components/DigitalMode.qml'
> --- app/components/DigitalMode.qml	2015-09-16 15:13:36 +0000
> +++ app/components/DigitalMode.qml	2015-10-12 20:43:31 +0000
> @@ -81,17 +84,20 @@
>          font.pixelSize: units.dp(1)
>          visible: text !== ""
>          text: {
> -            if (localizedTimeString.search(Qt.locale().amText) !== -1) {
> -                // 12 hour format detected with the localised AM text
> -                return Qt.locale().amText
> -            }
> -            else if (localizedTimeString.search(Qt.locale().pmText) !== -1) {
> -                // 12 hour format detected with the localised PM text
> -                return Qt.locale().pmText
> -            }
> -            else {
> -                // 24-hour format detected
> -                return ""
> +            // Fix for lp:1384739

Rather than commenting what the bug is, wouldn't it be better to say something like this: "Use the pmText/amText embedded in the time string". Is the amText/pmText available in this format? This bug is probably a locale issue, should the bug be assigned upstream?

> +            if (localizedTimeString.split(" ").length > 1) {

Have you checked other locales? While it may be unlikely, there might be other locales that behave well with a space in the timestring. I would suggest checking for "AM" or "PM" specifically.

> +                localizedTimeString.split(" ")[1] 
> +            } else {
> +                if (localizedTimeString.search(Qt.locale().amText) !== -1) {
> +                    // 12 hour format detected with the localised AM text
> +                    return Qt.locale().amText
> +                } else if (localizedTimeString.search(Qt.locale().pmText) !== -1) {
> +                    // 12 hour format detected with the localised PM text
> +                    return Qt.locale().pmText
> +                } else {
> +                    // 24-hour format detected
> +                    return ""
> +                }
>              }
>          }
>      }


-- 
https://code.launchpad.net/~gang65/ubuntu-clock-app/ubuntu-clock-app-australia-display-fix/+merge/274183
Your team Ubuntu Clock Developers is subscribed to branch lp:ubuntu-clock-app.