Merge lp:~gang65/ubuntu-clock-app/ubuntu-clock-app-slider-fix into lp:ubuntu-clock-app

[Bartosz Kosiorek]

Fix continously move the alarm volume slider to the desired value (LP: #1492584)

Fix continously move the alarm volume slider to the desired value (LP: #1492584)

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

FAILED: Continuous integration, rev:381
https://core-apps-jenkins.ubuntu.com/job/clock-app-ci/19/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/245/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/clock-app-ci/19/rebuild

review: Needs Fixing (continuous-integration)

[Victor Thompson (vthompson)]

Bartosz, you'll need to update the changelog.

review: Needs Fixing

[Victor Thompson (vthompson)]

Also, this doesn't appear to fix the actual issue.

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

PASSED: Continuous integration, rev:383
https://core-apps-jenkins.ubuntu.com/job/clock-app-ci/22/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/255/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/clock-app-ci/22/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

Can we not use the existing SDK Slider? Right now we get the following in the logs: file:///usr/lib/arm-linux-gnueabihf/qt5/qml/Ubuntu/Settings/Menus/SliderMenu.qml:145:13: QML Slider: Mixing of Ubuntu.Components module versions 1.3 and 1.2 detected!

Also, when I launch the application after changing the volume, occasionally the app will crash. Is it possible this is due to this change? I don't see anything in the logs.

phablet@ubuntu-phablet:~/.cache/upstart$ system-image-cli -i
current build number: 186
device name: mako
channel: ubuntu-touch/rc-proposed/bq-aquaris.en
last update: 2015-12-14 18:07:29
version version: 186
version ubuntu: 20151214
version device: 20150911
version custom: mako-1.1

One last minor comment, there's an extraneous newline on L53 of the diff. Could you please remove it?

review: Needs Information

[lp:~gang65/ubuntu-clock-app/ubuntu-clock-app-slider-fix]

384. By Bartosz Kosiorek on 2015-12-16

Fix continously move the alarm volume slider to the desired value (LP: #1492584)

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

PASSED: Continuous integration, rev:384
https://core-apps-jenkins.ubuntu.com/job/clock-app-ci/23/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/262/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/clock-app-ci/23/rebuild

review: Approve (continuous-integration)

[lp:~gang65/ubuntu-clock-app/ubuntu-clock-app-slider-fix]

385. By Bartosz Kosiorek on 2015-12-16

Fixes after review

[Bartosz Kosiorek (gang65)]

@Victor All review remarks are applied. Could you please test it again and check if crash is still existing?

review: Needs Information

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

PASSED: Continuous integration, rev:385
https://core-apps-jenkins.ubuntu.com/job/clock-app-ci/24/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/263/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/clock-app-ci/24/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

I have two comments, the first should be fixed, the second can wait as I assume the slider popover will be removed by the fix for bug #1362078.

1. There should be a divider under the alarm volume slider
2. Could we temporarily fix the top margin so the Slider popover doesn't get obscured by the header?

[Victor Thompson (vthompson)]

If you choose to fix the first issue by using the other slider component that is fine.

review: Needs Fixing

[Victor Thompson (vthompson)]

I believe the issue with the app crashing is bug #1527737, and is not related to your fix here.

[lp:~gang65/ubuntu-clock-app/ubuntu-clock-app-slider-fix]

386. By Bartosz Kosiorek on 2015-12-20

Use Menu.SliderMenu, as it is better common with other Ubuntu Phone components: see change volume in System Settings or Status bar

[Jenkins Bot (ubuntu-core-apps-jenkins-bot)]

PASSED: Continuous integration, rev:386
https://core-apps-jenkins.ubuntu.com/job/clock-app-ci/26/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/267/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/clock-app-ci/26/rebuild

review: Approve (continuous-integration)

[Bartosz Kosiorek (gang65)]

@Victor Do you think crash in bug #1475223 is the same issue?

review: Needs Information

[Victor Thompson (vthompson)]

I don't believe so. The crash I was observing (bug #1527737) seems to be something introduced very recently and only occurs/is applicable to the device (not Unity 7).

[Victor Thompson (vthompson)]

This lgtm and behaves as expected. Thanks for the additional fixes Bartosz!

review: Approve

=== modified file 'app/alarm/AlarmSettingsPage.qml'
--- app/alarm/AlarmSettingsPage.qml	2015-10-22 16:49:23 +0000
+++ app/alarm/AlarmSettingsPage.qml	2015-12-20 00:04:12 +0000
@@ -20,6 +20,7 @@
 import QtQuick.Layouts 1.1
 import WorldClock 1.0
 import Alarm 1.0
+import Ubuntu.Settings.Menus 0.1 as Menus
 import Ubuntu.Components 1.3
 import "../components"
 
@@ -84,40 +85,33 @@
                 top: parent.top
                 left: parent.left
                 right: parent.right
-            }
-
-            ListItem {
-                height: 2 * implicitHeight
-
-                Label {
-                    color: UbuntuColors.midAubergine
-                    text: i18n.tr("Alarm volume")
-                    anchors {
-                        left: parent.left
-                        leftMargin: units.gu(2)
-                        top: parent.top
-                        topMargin: units.gu(1)
-                    }
-                }
-
-                Slider {
-                    anchors {
-                        left: parent.left
-                        right: parent.right
-                        margins: units.gu(2)
-                        verticalCenter: parent.verticalCenter
-                    }
-
-                    minimumValue: 1
-                    maximumValue: 100
-                    value: alarmSettings.volume
-
-                    onValueChanged: {
-                        alarmSettings.volume = formatValue(value)
-                    }
-                }
-            }
-
+                topMargin: units.gu(2)
+            }
+
+            Label {
+                color: UbuntuColors.midAubergine
+                text: i18n.tr("Alarm volume")
+                anchors {
+                    left: parent.left
+                    leftMargin: units.gu(2)
+                }
+            }
+
+            Menus.SliderMenu {
+                anchors {
+                    left: parent.left
+                    right: parent.right
+                }
+                minimumValue: 1
+                maximumValue: 100
+                live: true
+                value: alarmSettings.volume
+
+                onValueChanged: {
+                    alarmSettings.volume = value
+                }
+            }       
+   
             ExpandableListItem {
                 id: _alarmDuration
 

=== modified file 'debian/changelog'
--- debian/changelog	2015-12-11 02:13:16 +0000
+++ debian/changelog	2015-12-20 00:04:12 +0000
@@ -3,8 +3,9 @@
   [ Bartosz Kosiorek ]
   * Fix alarm difference time description, during DST change (LP: #1510694)
   * Move to use the new SDK components v1.3 (LP: #1508363)
+  * Fix continously move the alarm volume slider to the desired value (LP: #1492584)  
 
- -- Bartosz Kosiorek <[e-mail]>  Tue, 27 Oct 2015 23:49:19 +0100
+ -- Bartosz Kosiorek <[e-mail]>  Wed, 16 Dec 2015 22:44:49 +0100
 
 ubuntu-clock-app (3.6)  vivid; urgency=medium
 
@@ -381,7 +382,7 @@
 
 ubuntu-clock-app (2.2) utopic; urgency=medium
 
-  [Nekhelesh Ramananthan]
+  [ Nekhelesh Ramananthan ]
   * Split labels to allow for a different color time divider.
   * Added support for 12-hour time display.
   * Fixed CMakeList.txt for apparmor file
@@ -392,7 +393,7 @@
 
 ubuntu-clock-app (2.1) utopic; urgency=medium
 
-  [Nekhelesh Ramananthan]
+  [ Nekhelesh Ramananthan ]
   * Added support for creating new alarms
   * Added bottom edge for alarms
   * Added tug down add city animation
@@ -402,7 +403,7 @@
 
 ubuntu-clock-app (2.0) utopic; urgency=medium
 
-  [Nekhelesh Ramananthan]
+  [ Nekhelesh Ramananthan ]
   * First release of the clock app reboot
   * Implemented Digital Mode (LP: #1267146)
   * Fixed scrollable lists on scrollable pages (LP: #1227418)
@@ -420,24 +421,24 @@
 
 ubuntu-clock-app (1.0) saucy; urgency=low
 
-  [Nekhelesh Ramananthan]
+  [ Nekhelesh Ramananthan ]
   * World Clock list needs to inform user when results cannot be retrieved due to network error (LP: #1231106)
   * Load premade presets for new user so user doesn't end up in blank app (LP: #1226131)
   * Return more fine-grained territorial divisions for city search results (LP: #1230153)
   * Improve the visual appearance of the clock, timer, stopwatch and alarm
   * Removed hour support from timer
 
-  [Sergio Schvezov]
+  [ Sergio Schvezov ]
   * Translation for desktop and debian package
   * Fix click package confinement issues
 
-  [Riccardo Padovani]
+  [ Riccardo Padovani ]
   * Alarm notifications do not appear when an alarm is triggered (LP: #1233176)
 
-  [Nicholas Skaggs]
+  [ Nicholas Skaggs ]
   * Several apps have failing tests with 20131003 ui-toolkit (LP: #1234544)
 
-  [Andrea Cerisara]
+  [ Andrea Cerisara ]
   * Autopilot Testcase Needed: Test Add World Clock (LP: #1210196)
   * Autopilot Testcase Needed: Test Delete World Clock (LP: #1210197)