Unity 8

Merge lp:~macslow/unity8/combo-button-support into lp:unity8

combo-button-support
Merge into trunk

Proposed by Mirco Müller on 2014-05-30

Status:	Merged
Approved by:	Michał Sawicz on 2014-07-11
Approved revision:	940
Merged at revision:	1042
Proposed branch:	lp:~macslow/unity8/combo-button-support
Merge into:	lp:unity8
Diff against target:	363 lines (+135/-118) 4 files modified debian/control (+1/-1) qml/Notifications/Notification.qml (+104/-79) tests/autopilot/unity8/shell/tests/test_notifications.py (+10/-13) tests/qmltests/Notifications/tst_Notifications.qml (+20/-25)
To merge this branch:	bzr merge lp:~macslow/unity8/combo-button-support
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Michał Sawicz		2014-05-30	Approve on 2014-07-11
PS Jenkins bot (community)	continuous-integration		Approve on 2014-07-11
Review via email: mp+221499@code.launchpad.net

Commit message

Added support for utilization of the ComboButton SDK-element for snap-decision notifications with many actions.

Description of the change

Added support for the ComboButton SDK-element to be utilized for snap-decision notifications with many actions (upto 7) to comply with design presented in the "compact widget patterns"-document from January 2014.

QML- and autopilot-tests were also updated to reflect the change in layout and behaviour.

Furthermore there'll be a newer version of lp:unity-notifications with some tweaks and updated example code of how to use the comboButton-support. This newer version of lp:unity-notifications is mandatory for this unity8 branch to work correctly.

Here's a short screencast: http://www.youtube.com/watch?v=4KMrsj_h0EM

- Are there any related MPs required for this MP to build/function as expected?
Yes. https://code.launchpad.net/~macslow/unity-notifications/combo-button-support

- Did you perform an exploratory manual test run of your code change and any related functionality?
Yes, but only on desktop not yet on the device/phone since cross-compiling currently doesn't work for me (Malta-sprint, unreliable wifi)

- Did you make sure that your branch does not contain spurious tags?
Yes

- If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
not applicable

- If you changed the UI, has there been a design review?
Not yet. This branch does implement a desired design though.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-05-30:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3072/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-06-06:

It feels to me like the pip should be colourized (grey?) instead of white, is the design to leave it white?

review: Needs Fixing

Revision history for this message

Mirco Müller (macslow) wrote on 2014-06-06:

Correct. That's a fix to the ComboButton SDK-element, which has not been done yet. I'm currently looking into this. Furthermore the "carret" of the drop-down button has the wrong color. It should be full white. This too needs to be fixed in the ComboButton SDK-element.

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-06-06:

Please bump the plugin dep after you bump it in the corresponding MP in lp:unity-notifications.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-07:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3113/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-14:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3141/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-06-14:

Test Result (1 failure / -7)
unity8.shell.tests.test_notifications.InteractiveNotificationBase.test_sd_incoming_call(Native Device)

lp:~macslow/unity8/combo-button-support updated on 2014-06-16

931. By Mirco Müller on 2014-06-16: Enforce dependency on 0.1.2 version of notification-plugin to reflect newly supported max. number of snap-decision actions.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-16:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3153/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-18:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3167/rebuild

review: Needs Fixing (continuous-integration)

lp:~macslow/unity8/combo-button-support updated on 2014-06-20

932. By Mirco Müller on 2014-06-20: The even newer Design wants flat colors. But the ubuntu-ui-toolkit does not offer those yet. Thus I'll stick with these hard-coded values for the time being. I'll change the notification-code once the newer themed flat colors become available in our SDK.
933. By Mirco Müller on 2014-06-20: Further fixes from MP-comments.
934. By Mirco Müller on 2014-06-20: More cleanup and fixes from MP-comments.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-20:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3187/rebuild

review: Needs Fixing (continuous-integration)

lp:~macslow/unity8/combo-button-support updated on 2014-06-23

935. By Mirco Müller on 2014-06-23: Merged with trunk.
936. By Mirco Müller on 2014-06-23: Use a regular expression instead of two custom JavaScript-functions for the icon-label split of the action-label.
937. By Mirco Müller on 2014-06-23: Tighten up anchors code a bit.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-23:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3199/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-23:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3200/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-26:

FAILED: Continuous integration, rev:937
http://jenkins.qa.ubuntu.com/job/unity8-ci/3235/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/1248/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/239/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/329/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/329/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/329/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2133/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3235/rebuild

review: Needs Fixing (continuous-integration)

lp:~macslow/unity8/combo-button-support updated on 2014-06-27

938. By Mirco Müller on 2014-06-27: Merged again with trunk and solved merge-conflicts.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-28:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3254/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-07-04:

See inline.

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-05:

FAILED: Continuous integration, rev:938
http://jenkins.qa.ubuntu.com/job/unity8-ci/3328/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/1577/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/332/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/422/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/422/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/422/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2574/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3328/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-07-05:

Text conflict in debian/control
1 conflicts encountered.

review: Needs Fixing

lp:~macslow/unity8/combo-button-support updated on 2014-07-11

939. By Mirco Müller on 2014-07-11: Merged with trunk and resolved conflicts.
940. By Mirco Müller on 2014-07-11: Adjust regular expression to allow a larger variety of icon-names.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3403/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-07-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/3405/rebuild

review: Approve (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-07-11:

* Did you perform an exploratory manual test run of the code change and any related functionality?
Yes. Works nicely!

* Did CI run pass? If not, please explain why.
Yes!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Mirco Müller

Mohammad Rakibul Hasan

Unity Team

 === modified file 'debian/control'
 --- debian/control	2014-07-09 19:44:14 +0000
 +++ debian/control	2014-07-11 08:50:45 +0000
@@ -103,7 +103,7 @@
  Architecture: all
  Depends: qtdeclarative5-ubuntu-thumbnailer0.1 | ubuntu-thumbnailer-impl,
           qtdeclarative5-ubuntu-ui-toolkit-plugin (>= 0.1.49) | qtdeclarative5-ubuntu-ui-toolkit-plugin-gles (>= 0.1.49),
--         qtdeclarative5-unity-notifications-plugin | unity-notifications-impl,
++         qtdeclarative5-unity-notifications-plugin (>= 0.1.2) | unity-notifications-impl,
           ubuntu-thumbnailer-impl-0,
           unity-application-impl-2,
           unity-notifications-impl-3,
 === modified file 'qml/Notifications/Notification.qml'
 --- qml/Notifications/Notification.qml	2014-06-18 09:45:37 +0000
 +++ qml/Notifications/Notification.qml	2014-07-11 08:50:45 +0000
@@ -16,11 +16,13 @@
  import QtQuick 2.0
  import QtMultimedia 5.0
--import Ubuntu.Components 0.1
++import Ubuntu.Components 1.1
  import Unity.Notifications 1.0
  import QMenuModel 0.1
  import Utils 0.1
++import Ubuntu.Components.ListItems 0.1 as ListItem
++
  Item {
      id: notification
@@ -37,6 +39,14 @@
      property bool fullscreen
      property int maxHeight
      property int margins
++    property Gradient greenGradient : Gradient {
++        GradientStop { position: 0.0; color: "#3fb24f" }
++        GradientStop { position: 1.0; color: "#3fb24f" }
++    }
++    property Gradient darkgreyGradient: Gradient {
++        GradientStop { position: 0.0; color: "#4d4745" }
++        GradientStop { position: 1.0; color: "#4d4745" }
++    }
      fullscreen: false
      objectName: "background"
@@ -291,7 +301,7 @@
+                 }
+             }
--            Item {
++            Row {
                  id: buttonRow
                  objectName: "buttonRow"
@@ -300,70 +310,8 @@
                      right: parent.right
+                 }
                  visible: notification.type == Notification.SnapDecision && actionRepeater.count > 0
--                height: units.gu(5)
--
--                property real buttonWidth: (width - contentColumn.spacing) / 2
--                property bool expanded
--
--                Button {
--                    id: leftButton
--
--                    objectName: "button1"
--                    width: parent.expanded ? parent.width : parent.buttonWidth
--                    anchors {
--                        top: parent.top
--                        bottom: parent.bottom
--                    }
--                    text: notification.type == Notification.SnapDecision && actionRepeater.count >= 2 ? actionRepeater.itemAt(1).actionLabel : ""
--                    gradient: UbuntuColors.greyGradient
--                    onClicked: {
--                        if (actionRepeater.count > 2) {
--                            buttonRow.expanded = !buttonRow.expanded
--                        } else {
--                            notification.notification.invokeAction(actionRepeater.itemAt(1).actionId)
--                        }
--                    }
--
--                    Behavior on width {
--                        UbuntuNumberAnimation {
--                            duration: UbuntuAnimation.SnapDuration
--                        }
--                    }
--                }
--
--                Button {
--                    id: rightButton
--
--                    objectName: "button0"
--                    anchors {
--                        left: leftButton.right
--                        leftMargin: contentColumn.spacing
--                        right: parent.right
--                    }
--                    text: notification.type == Notification.SnapDecision && actionRepeater.count >= 1 ? actionRepeater.itemAt(0).actionLabel : ""
--                    anchors {
--                        top: parent.top
--                        bottom: parent.bottom
--                    }
--                    gradient: notification.hints["x-canonical-private-button-tint"] == "true" ? UbuntuColors.orangeGradient : UbuntuColors.greyGradient
--                    visible: width > 0
--                    onClicked: notification.notification.invokeAction(actionRepeater.itemAt(0).actionId)
--                }
--            }
--
--            Column {
--                objectName: "buttonColumn"
--                spacing: contentColumn.spacing
--                anchors {
--                    left: parent.left
--                    right: parent.right
--                }
--
--                // calculate initial position before Column takes over
--                y: buttonRow.y + buttonRow.height + contentColumn.spacing
--
--                visible: notification.type == Notification.SnapDecision && buttonRow.expanded
--                height: buttonRow.expanded ? implicitHeight : 0
++                spacing: units.gu(1)
++                layoutDirection: Qt.RightToLeft
                  Repeater {
                      id: actionRepeater
@@ -375,28 +323,105 @@
                          property string actionId: id
                          property string actionLabel: label
--                        anchors {
--                            left: parent.left
--                            right: parent.right
--                        }
--
                          Component {
                              id: actionButton
                              Button {
                                  objectName: "button" + index
--                                anchors {
--                                    left: parent.left
--                                    right: parent.right
--                                }
--
++                                width: buttonRow.width / 2 - spacing
                                  text: loader.actionLabel
--                                height: units.gu(5)
--                                gradient: UbuntuColors.greyGradient
++                                gradient: notification.hints["x-canonical-private-button-tint"] == "true" && index == 0 ? greenGradient : darkgreyGradient
                                  onClicked: notification.notification.invokeAction(loader.actionId)
+                             }
+                         }
--                        sourceComponent: (index == 0 || index == 1) ? undefined : actionButton
++                        sourceComponent: (index == 0 || index == 1) ? actionButton : undefined
++                    }
++                }
++            }
++
++            ComboButton {
++                id: comboButton
++
++                objectName: "button2"
++                width: parent.width
++                visible: notification.type == Notification.SnapDecision && actionRepeater.count > 3
++                gradient: darkgreyGradient
++                onClicked: notification.notification.invokeAction(comboRepeater.itemAt(2).actionId)
++                expanded: false
++                expandedHeight: (comboRepeater.count - 2) * units.gu(4) + units.gu(.5)
++                comboList: Flickable {
++                    // this has to be wrapped inside a flickable
++                    // to work around a feature/bug? of the
++                    // ComboButton SDK-element, making a regular
++                    // unwrapped Column item flickable
++                    // see LP: #1332590
++                    interactive: false
++                    Column {
++                        Repeater {
++                            id: comboRepeater
++
++                            onVisibleChanged: {
++                                comboButton.text = comboRepeater.itemAt(2).actionLabel
++                            }
++
++                            model: notification.actions
++                            delegate: Loader {
++                                id: comboLoader
++
++                                asynchronous: true
++                                visible: status == Loader.Ready
++                                property string actionId: id
++                                property string actionLabel: label
++                                readonly property var splitLabel: actionLabel.match(/(^([-a-z0-9]+):)?(.*)$/)
++                                Component {
++                                    id: comboEntry
++
++                                    MouseArea {
++                                        id: comboInputArea
++
++                                        objectName: "button" + index
++                                        width: comboButton.width
++                                        height: comboIcon.height + units.gu(2)
++
++                                        onClicked: {
++                                            notification.notification.invokeAction(actionId)
++                                        }
++
++                                        ListItem.ThinDivider {
++                                            visible: index > 3
++                                        }
++
++                                        Icon {
++                                            id: comboIcon
++
++                                            anchors {
++                                                left: parent.left
++                                                leftMargin: units.gu(.5)
++                                                verticalCenter: parent.verticalCenter
++                                            }
++                                            width: units.gu(2)
++                                            height: units.gu(2)
++                                            color: "white"
++                                            name: splitLabel[2]
++                                        }
++
++                                        Label {
++                                            id: comboLabel
++
++                                            anchors {
++                                                left: comboIcon.right
++                                                leftMargin: units.gu(1)
++                                                verticalCenter: comboIcon.verticalCenter
++                                            }
++                                            fontSize: "small"
++                                            color: "white"
++                                            text: splitLabel[3]
++                                        }
++                                    }
++                                }
++                                sourceComponent: (index > 2) ? comboEntry : undefined
++                            }
++                        }
+                     }
+                 }
+             }
 === modified file 'tests/autopilot/unity8/shell/tests/test_notifications.py'
 --- tests/autopilot/unity8/shell/tests/test_notifications.py	2014-07-07 08:51:33 +0000
 +++ tests/autopilot/unity8/shell/tests/test_notifications.py	2014-07-11 08:50:45 +0000
@@ -178,11 +178,13 @@
+         ]
          actions = [
--            ('action_accept', 'Accept'),
--            ('action_decline_1', 'Decline'),
--            ('action_decline_2', '"Can\'t talk now, what\'s up?"'),
--            ('action_decline_3', '"I call you back."'),
--            ('action_decline_4', 'Send custom message...'),
++            ('action_accept', 'Hold + Answer'),
++            ('action_decline_1', 'End + Answer'),
++            ('action_decline_2', 'Decline'),
++            ('action_decline_3', 'messages:I missed your call - can you call me now?'),
++            ('action_decline_4', 'messages:I\'m running late. I\'m on my way.'),
++            ('action_decline_5', 'messages:I\'m busy at the moment. I\'ll call later.'),
++            ('action_decline_6', 'edit:Custom'),
+         ]
          self._create_interactive_notification(
@@ -200,14 +202,9 @@
          notification = get_notification()
          self._assert_notification(notification, summary, body, True, True, 1.0)
          initial_height = notification.height
--        self.touch.tap_object(notification.select_single(objectName="button1"))
--        self.assertThat(
--            notification.height,
--            Eventually(Equals(initial_height +
--                              3 * notification.select_single(
--                                  objectName="buttonColumn").spacing +
--                              3 * notification.select_single(
--                                  objectName="button4").height)))
++        self.touch.tap_object(notification.select_single(objectName="combobutton_dropdown"))
++        self.assertThat(notification.select_single(objectName="button2").expanded, Eventually(Equals(True)))
++        time.sleep(2)
          self.touch.tap_object(notification.select_single(objectName="button4"))
          self.assert_notification_action_id_was_called("action_decline_4")
 === modified file 'tests/qmltests/Notifications/tst_Notifications.qml'
 --- tests/qmltests/Notifications/tst_Notifications.qml	2014-06-16 11:07:18 +0000
 +++ tests/qmltests/Notifications/tst_Notifications.qml	2014-07-11 08:50:45 +0000
@@ -72,8 +72,8 @@
              actions: [{ id: "ok_id", label: "Ok"},
                        { id: "cancel_id", label: "Cancel"},
                        { id: "notreally_id", label: "Not really"},
--                      { id: "noway_id", label: "No way"},
--                      { id: "nada_id", label: "Nada"}]
++                      { id: "noway_id", label: "messages:No way"},
++                      { id: "nada_id", label: "messages:Nada"}]
+         }
          mockModel.append(n)
@@ -237,8 +237,8 @@
                  actions: [{ id: "ok_id", label: "Ok"},
                            { id: "cancel_id", label: "Cancel"},
                            { id: "notreally_id", label: "Not really"},
--                          { id: "noway_id", label: "No way"},
--                          { id: "nada_id", label: "Nada"}],
++                          { id: "noway_id", label: "messages:No way"},
++                          { id: "nada_id", label: "messages:Nada"}],
                  summaryVisible: true,
                  bodyVisible: true,
                  iconVisible: true,
@@ -456,32 +456,27 @@
                  actionSpy.clear()
                  waitForRendering(notification)
--                // check if there's more than one negative choice
++                // check if there's a ComboButton created due to more actions being passed
                  if (data.actions.length > 2) {
--                    var initialHeight = notification.implicitHeight
++                    var comboButton = findChild(notification, "button2")
++                    tryCompareFunction(function() { return comboButton.expanded == false; }, true);
                      // click to expand
--                    mouseClick(buttonCancel, buttonCancel.width / 2, buttonCancel.height / 2)
--                    var contentColumn = findChild(notification, "contentColumn")
--                    var collapsedContentColumnHeight = contentColumn.height;
--                    // Waiting for the inner column to change height because buttons appear
--                    tryCompareFunction(function() { return collapsedContentColumnHeight != contentColumn.height; }, true);
--                    // Waiting for notification to reach its target size
--                    tryCompare(notification, "height", contentColumn.height + contentColumn.spacing * 2)
--                    actionSpy.clear()
--
--                    // test the additional buttons
--                    for (var i = 2; i < data.actions.length; i++) {
--                        var buttonColumn = findChild(notification, "buttonColumn")
--                        var button = findChild(buttonColumn, "button" + i)
--                        mouseClick(button, button.width / 2, button.height / 2)
--                        compare(actionSpy.signalArguments[0][0], data.actions[i]["id"], "got wrong id for additional negative action")
--                        actionSpy.clear()
--                    }
++                    tryCompareFunction(function() { mouseClick(comboButton, comboButton.width - comboButton.__styleInstance.dropDownWidth / 2, comboButton.height / 2); return comboButton.expanded == true; }, true);
++
++                    // try clicking on choices in expanded comboList
++                    var choiceButton1 = findChild(notification, "button3")
++                    tryCompareFunction(function() { mouseClick(choiceButton1, choiceButton1.width / 2, choiceButton1.height / 2); return actionSpy.signalArguments.length > 0; }, true);
++                    compare(actionSpy.signalArguments[0][0], data.actions[3]["id"], "got wrong id choice action 1")
++                    actionSpy.clear()
++
++                    var choiceButton2 = findChild(notification, "button4")
++                    tryCompareFunction(function() { mouseClick(choiceButton2, choiceButton2.width / 2, choiceButton2.height / 2); return actionSpy.signalArguments.length > 0; }, true);
++                    compare(actionSpy.signalArguments[0][0], data.actions[4]["id"], "got wrong id choice action 2")
++                    actionSpy.clear()
                      // click to collapse
--                    mouseClick(buttonCancel, buttonCancel.width / 2, buttonCancel.height / 2)
--                    tryCompare(notification, "height", initialHeight)
++                    //tryCompareFunction(function() { mouseClick(comboButton, comboButton.width - comboButton.__styleInstance.dropDownWidth / 2, comboButton.height / 2); return comboButton.expanded == false; }, true);
                  } else {
                      mouseClick(buttonCancel, buttonCancel.width / 2, buttonCancel.height / 2)
                      compare(actionSpy.signalArguments[0][0], data.actions[1]["id"], "got wrong id for negative action")