ubuntu-touch-coreapps-reviewers team mailing list archive

[Bartosz Kosiorek <gang65@xxxxxxxxxxxxxx>]

1. I added inline comments. Generally we shouldn't process alarm when (model.status !== Alarm.Ready)
In most cases changing model.status to Alarm.Ready is fast. But in rare cases, Alarm will be parsed faster when it is not ready. It is causing "Alarm disabled" issue.
2. Thanks done
3. I have added description to bug report

Diff comments:

> === modified file 'app/alarm/AlarmDelegate.qml'
> --- app/alarm/AlarmDelegate.qml	2015-12-11 02:13:16 +0000
> +++ app/alarm/AlarmDelegate.qml	2015-12-30 13:05:55 +0000
> @@ -56,7 +56,7 @@
>  
>              color: UbuntuColors.midAubergine
>              fontSize: "x-large"
> -            text: Qt.formatTime(date)
> +            text: Qt.formatTime(model.date)

Adding model. for constistency reason in the whole file.

>          }
>  
>          RowLayout {
> @@ -85,11 +85,10 @@
>  
>                  fontSize: "small"
>                  Layout.fillWidth: true
> -                visible: !(type === Alarm.OneTime && !model.enabled)
> +                visible: ((type === Alarm.Repeating) || model.enabled) && (model.status === Alarm.Ready)

We don't want to display anything if alarm is not ready, and the values in it is old.

>                  elide: Text.ElideRight
>                  text: type === Alarm.Repeating ? alarmUtils.format_day_string(daysOfWeek, type)
> -                                               : model.enabled ? alarmUtils.get_time_to_alarm(model.date, localTime)
> -                                                               : "Alarm Disabled"
> +                                               : alarmUtils.get_time_to_alarm(model.date, localTime)

We don't want to display any not localized string. If alarm will be disabled, then localized string from get_time_to_alarm will be used.

>  
>                  function animateTextChange() {
>                      textChangeAnimation.start()
> @@ -124,6 +123,7 @@
>      Switch {
>          id: alarmStatus
>          objectName: "listAlarmStatus" + index
> +        checked: model.enabled && (model.status === Alarm.Ready)

We would like to have reliable update Switch position.
With this single line we are covering cases when Alarm.OneTime was enabled, and caption is updated.

>  
>          anchors {
>              right: parent.right


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