Merge lp:~vthompson/ubuntu-clock-app/sound-and-repeat-on-click into lp:ubuntu-clock-app

[Victor Thompson]

Allow tapping repeat and sound ListItems to trigger action.

Allow tapping repeat and sound ListItems to trigger action. Currently the user has to tap the CheckBox in order to toggle to the selection. It'd be preferable to be able to tap the ListItem to toggle the action.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:202
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/616/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2078
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2078/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/616/rebuild

review: Approve (continuous-integration)

[Nekhelesh Ramananthan (nik90)]

> Allow tapping repeat and sound ListItems to trigger action. Currently the user has to tap the CheckBox in order to toggle to the selection. It'd be preferable to be able to tap the ListItem to toggle the action.

I noticed this "regression" a couple of weeks ago which caused the clock app tests to fail. After some investigation, I noticed that

- In Ubuntu-RTM the listitems have the behaviour where the user can click anywhere and to check/uncheck the checkbox.

- In Ubuntu-Vivid, the user has to explicitly click on the checkbox to check/uncheck it.

On talking to zsombor, he said the ubuntu-rtm behaviour was a SDK bug and that it was fixed in vivid. So naturally I just fixed the clock app tests with the new behavior although I prefer the ubuntu-rtm solution.

Can you check what the behavior is for listitems shown in the indicator panels on vivid? I would prefer we stick to their solution to maintain consistency with the platform. And if this usability issue is present throughout the platform we can take it up on the mailing list to fix it in the SDK itself.

review: Needs Information

[Victor Thompson (vthompson)]

I was almost 100% sure it's always been like this. I might flash back to RTM to verify, though.

On Vivid, the indicator values are toggled and any corresponding action is executed by tapping the ListItem--the icon/switch does not need to be explicitly tapped.

[Victor Thompson (vthompson)]

After flashing RTM, I agree that this seems like a "regression" since it behaved as expected in RTM. However, it makes sense that the SDK shouldn't automatically assume a certain action is taken when a ListItem is clicked. So it does seem like something that should be application specific.

[Nekhelesh Ramananthan (nik90)]

I believe the following code,

51 + onClicked: {
52 + if (!_soundStatus.checked) {
53 + // Ensures only one alarm sound is selected
54 + for(var i=0; i<soundModel.count; i++) {
55 + if(_alarmSounds.itemAt(i).isChecked &&
56 + i !== index) {
57 + _alarmSounds.itemAt(i).isChecked = false
58 + }
59 + }
60 + _soundStatus.checked = true
61 }
62 }

can be refactored into,

onClicked: {
     if (!_soundStatus.checked) {
          _soundStatus.checked = true
     }
}

We don't need to do the "ensure only one alarm" loop again since it will be done automatically inside the onCheckedChanged signal. Do you agree?

Also please update the debian changelog with your change. Please add to the existing version 3.3 by nano editing it directly.

Everything else looks good.

review: Needs Fixing

[Victor Thompson (vthompson)]

I agree, ensuring that the other CheckBox components are not checked is no longer necessary.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:203
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/623/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2264
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2264/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/623/rebuild

review: Approve (continuous-integration)

[Nekhelesh Ramananthan (nik90)]

LGTM! Thnx for the fix Victor.

review: Approve

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Autolanding.
More details in the following jenkins job:
http://91.189.93.70:8080/job/ubuntu-clock-app-autolanding/226/
Executed test runs:
    FAILURE: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2270/console

review: Needs Fixing (continuous-integration)

[Nekhelesh Ramananthan (nik90)]

Looks like there were 3 QML test failures. It looks random since the code wasn't changed there. Reapproving.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

review: Approve (continuous-integration)

=== modified file 'app/alarm/AlarmRepeat.qml'
--- app/alarm/AlarmRepeat.qml	2015-01-22 00:26:16 +0000
+++ app/alarm/AlarmRepeat.qml	2015-03-06 01:09:34 +0000
@@ -154,6 +154,10 @@
                         }
                     }
                 }
+
+                onClicked: {
+                    daySwitch.checked = !daySwitch.checked
+                }
             }
         }
     }

=== modified file 'app/alarm/AlarmSound.qml'
--- app/alarm/AlarmSound.qml	2014-10-16 19:03:51 +0000
+++ app/alarm/AlarmSound.qml	2015-03-06 01:09:34 +0000
@@ -95,11 +95,10 @@
 
                         checked: alarmSound.subText === _soundName.text ? true
                                                                         : false
-                        onClicked: {
-                            previewAlarmSound.source = fileURL
-                            previewAlarmSound.play()
-
+                        onCheckedChanged: {
                             if (checked) {
+                                previewAlarmSound.source = fileURL
+                                previewAlarmSound.play()
                                 alarmSound.subText = _soundName.text
                                 alarm.sound = fileURL
 
@@ -111,12 +110,20 @@
                                     }
                                 }
                             }
+                        }
 
-                            else {
-                                checked = !checked
+                        onClicked: {
+                            if (!checked) {
+                                checked = true
                             }
                         }
                     }
+
+                    onClicked: {
+                        if (!_soundStatus.checked) {
+                            _soundStatus.checked = true
+                        }
+                    }
                 }
             }
         }

=== modified file 'debian/changelog'
--- debian/changelog	2015-03-05 12:57:41 +0000
+++ debian/changelog	2015-03-06 01:09:34 +0000
@@ -19,6 +19,10 @@
   * Fixed one-time alarms not being able to be re-enabled using the alarm switch
     after they have gone off once (LP: #1413027)
 
+  [Victor Thompson]
+  * Fixed a regression in vivid where clicking an alarm sound ListItem was not
+    causing the CheckBox to be checked.
+
  -- Nekhelesh Ramananthan <[e-mail]>  Wed, 21 Jan 2015 21:05:24 +0100
 
 ubuntu-clock-app (3.2) utopic; urgency=medium