ubuntu-touch-coreapps-reviewers team mailing list archive

[Nekhelesh Ramananthan <krnekhelesh@xxxxxxxxx>]

Nekhelesh Ramananthan has proposed merging lp:~nik90/ubuntu-clock-app/improve-header-confirmation-behavior into lp:ubuntu-clock-app.

Commit message:
Fixes the confusing back & save button behaviour in the alarm pages.

Requested reviews:
  Ubuntu Clock Developers (ubuntu-clock-dev)
Related bugs:
  Bug #1408015 in Ubuntu Clock App: "[sdk][UX] Confirmation in the header bar confusing"
  https://bugs.launchpad.net/ubuntu-clock-app/+bug/1408015

For more details, see:
https://code.launchpad.net/~nik90/ubuntu-clock-app/improve-header-confirmation-behavior/+merge/267839

This MP fixes the confusing back & save button behaviour in the alarm pages.

Original issue was that the back button was sometimes used for "Saving" & "Going Back". This alternating behaviour was confusing to users as seen in the bug report attached. So according to UX Suggestion, the back button will now behave as "Go Back" or "Cancel" action. While the save button will be the save to confirm your changes.
-- 
Your team Ubuntu Clock Developers is requested to review the proposed merge of lp:~nik90/ubuntu-clock-app/improve-header-confirmation-behavior into lp:ubuntu-clock-app.

=== modified file 'app/alarm/AlarmLabel.qml'
--- app/alarm/AlarmLabel.qml	2015-05-27 16:03:23 +0000
+++ app/alarm/AlarmLabel.qml	2015-08-12 15:55:06 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014 Canonical Ltd
+ * Copyright (C) 2014-2015 Canonical Ltd
  *
  * This file is part of Ubuntu Clock App
  *
@@ -26,6 +26,9 @@
     // Property to set the alarm label in the edit alarm page
     property var alarm
 
+    // Property to store the old alarm label to detect if user changed it or not
+    property string oldAlarmLabel: alarm.message
+
     visible: false
     title: i18n.tr("Label")
 
@@ -33,7 +36,18 @@
         id: backAction
         iconName: "back"
         onTriggered: {
-            alarm.message = _labelEntry.text
+            // Restore old alarm label if user presses the back button
+            alarm.message = oldAlarmLabel
+            pop()
+        }
+    }
+
+    head.actions: Action {
+        id: saveAction
+        iconName: "tick"
+        enabled: oldAlarmLabel !== _labelEntry.text.trim() && !!_labelEntry.text.trim()
+        onTriggered: {
+            alarm.message = _labelEntry.text.trim()
             pop()
         }
     }
@@ -63,10 +77,6 @@
             text: alarm.message
             width: parent.width
             inputMethodHints: Qt.ImhNoPredictiveText
-
-            onTextChanged: {
-                backAction.enabled = !!text.trim()
-            }
         }
     }
 }

=== modified file 'app/alarm/AlarmRepeat.qml'
--- app/alarm/AlarmRepeat.qml	2015-06-03 22:49:29 +0000
+++ app/alarm/AlarmRepeat.qml	2015-08-12 15:55:06 +0000
@@ -29,9 +29,31 @@
     // Property to hold the alarm utils functions passed from edit alarm page
     property var alarmUtils
 
+    // Property to store the previously set alarm frequency to detect user changes
+    property int oldAlarmDaysOfWeek
+
     visible: false
     title: i18n.tr("Repeat")
 
+    // Function to detect if alarm is OneTime or Repeating
+    function detectAlarmType() {
+        if (alarm.daysOfWeek > 0) {
+            alarm.type = Alarm.Repeating
+        } else {
+            alarm.type = Alarm.OneTime
+        }
+    }
+
+    head.backAction: Action {
+        iconName: "back"
+        onTriggered: {
+            // Restore alarm frequency and type if user presses the back button
+            alarm.daysOfWeek = oldAlarmDaysOfWeek
+            detectAlarmType()
+            pop()
+        }
+    }
+
     head.actions: [
         Action {
             text: i18n.tr("Select All")
@@ -56,6 +78,14 @@
                     }
                 }
             }
+        },
+
+        Action {
+            iconName: "tick"
+            enabled: oldAlarmDaysOfWeek !== alarm.daysOfWeek
+            onTriggered: {
+                pop()
+            }
         }
     ]
 
@@ -69,6 +99,9 @@
     Component.onCompleted: {
         if (alarm.type === Alarm.OneTime)
             alarm.daysOfWeek = 0
+
+        // Record the current alarm repeat values (frequency)
+        oldAlarmDaysOfWeek = alarm.daysOfWeek
     }
 
     Component.onDestruction: {
@@ -147,11 +180,7 @@
                             alarm.daysOfWeek &= ~flag
                         }
 
-                        if (alarm.daysOfWeek > 0) {
-                            alarm.type = Alarm.Repeating
-                        } else {
-                            alarm.type = Alarm.OneTime
-                        }
+                        detectAlarmType()
                     }
                 }
 

=== modified file 'app/alarm/AlarmSound.qml'
--- app/alarm/AlarmSound.qml	2015-06-03 22:49:29 +0000
+++ app/alarm/AlarmSound.qml	2015-08-12 15:55:06 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014 Canonical Ltd
+ * Copyright (C) 2014-15 Canonical Ltd
  *
  * This file is part of Ubuntu Clock App
  *
@@ -37,9 +37,33 @@
     // Property to set the alarm sound model in the edit alarm page
     property var soundModel
 
+    /*
+     Properties to store the previously set alarm sound values to detect
+     if the user changed the alarm sound or not
+    */
+    property url oldAlarmSoundUrl
+    property string oldAlarmSoundName
+
+    Component.onCompleted: {
+        // Record the current alarm sound values (url, name)
+        oldAlarmSoundUrl = alarm.sound
+        oldAlarmSoundName = alarmSound.subText
+    }
+
     head.backAction: Action {
         iconName: "back"
         onTriggered: {
+            // Restore alarm sound to old values (url, name) if the back button is pressed
+            alarm.sound = oldAlarmSoundUrl
+            alarmSound.subText = oldAlarmSoundName
+            pop()
+        }
+    }
+
+    head.actions: Action {
+        iconName: "tick"
+        enabled: oldAlarmSoundUrl !== alarm.sound
+        onTriggered: {
             pop()
         }
     }

=== modified file 'debian/changelog'
--- debian/changelog	2015-08-11 20:40:33 +0000
+++ debian/changelog	2015-08-12 15:55:06 +0000
@@ -6,6 +6,7 @@
   
   [Nekhelesh Ramananthan]
   * Increase the height of times in the alarm screen (LP: #1365428)
+  * Fixed the confusing behavior of the confirmation button (LP: #1408015)
 
   [Victor Thompson]
   * Show all README files in QtCreator