Unity

Merge lp:~macslow/unity/phablet-notification-renderer into lp:unity/phablet

  1. phablet-notification-renderer
  2. Merge into phablet
Proposed by Mirco Müller
Status: Merged
Approved by: Mirco Müller
Approved revision: no longer in the source branch.
Merged at revision: 675
Proposed branch: lp:~macslow/unity/phablet-notification-renderer
Merge into: lp:unity/phablet
Diff against target: 645 lines (+591/-0)
7 files modified
CMakeLists.txt (+1/-0)
Notifications/Notification.qml (+153/-0)
Notifications/Notifications.qml (+77/-0)
Notifications/timings.js (+23/-0)
debian/qml-phone-shell.install (+1/-0)
tests/qmltests/CMakeLists.txt (+1/-0)
tests/qmltests/Notifications/tst_Notifications.qml (+335/-0)
To merge this branch: bzr merge lp:~macslow/unity/phablet-notification-renderer
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Michał Sawicz Approve
Michael Zanetti (community) Approve
Review via email: mp+155512@code.launchpad.net

Commit message

First basic NotificationRenderer (Frontend) for NotifyOSD NG. Ephemeral and 2-button snap-decision layout are support for a start. Using mocked up notification-data for the moment.

Description of the change

First basic NotificationRenderer (Frontend) for NotifyOSD NG. Ephemeral and 2-button snap-decision layout are support for a start. Using mocked up notification-data for the moment.

To post a comment you must log in.
Revision history for this message
Michał Sawicz (saviq) wrote :

* the change to .bzrignore seems unrelated, please don't submit it
* I'd rename Bubble to Notification
* I don't see a reason to separate Bubble.qml into a separate dir - unless really difficult, there should only be a single Notification component that lays itself out according to the properties set
* "property bool snapDecision: actions != null" - this should depend on the type of the notification, not the presence of actions
* the whole bubble is 0.85 opaque, is that per-design? shouldn't just the background be transparent and the text fully opaque?
* please separate the components with newlines
* "visible: secondaryIconSource ? true : false" you can use just "source" here, secondaryIconSource is an alias anyway
* "left: parent.left; leftMargin: units.gu(8)" you should anchor to icon.right instead
* "fillMode: Image.PreserveAspectCrop" for secondaryIcon seems unnecessary, the icon should always be 2x2 GUs
* "leftMargin: secondaryIcon.visible ? units.gu(11) : units.gu(8)" anchor to icon or secondaryIcon instead
* "width: secondaryIcon.visible ? units.gu (25) : units.gu (28)" anchor to parent.right instead
* "font.family: "Ubuntu"" is default
* "opacity: 1.0" is default, and does _not_ mean that the text itself will be fully opaque, because the parent item is not opaque [1]
* "maximumLineCount: 1" is unnecessary when wrapping is disabled
* the above applies to bodyLabel, too, except that bodyLabel should anchor to summaryLabel's left and bottom
* you should elide the body, too, in case it overflows
* "width: units.gu (17.5)" is arbitrary, calculate the width based on parent.width
* "text: actions.get(1).action_label" you should almost never need Model.get() - use Views or Positioners with Repeaters [2] [3]
* the Loader is unnecessary, if you use a Repeater, it will not create anything if the model is empty
* use add/move/delete transitions

* in this case I'd rather see the layout to use Positioners [2] extensively for better readability and flexibility, like so:

Column {
    Row {
        UbuntuShape { id: icon }
        Image { id: secondaryIcon}
        Column {
            Label { id: summary }
            Label { id: body }
        }
    }
    Flow {
        Repeater { id: buttons }
    }
}

[1] http://qt-project.org/doc/qt-5.0/qtquick/qml-qtquick2-item.html#opacity-prop
[2] http://qt-project.org/doc/qt-5.0/qtquick/qtquick-qmltypereference.html#positioning
[3] http://qt-project.org/doc/qt-5.0/qtquick/qtquick-qmltypereference.html#model-view-types-and-data-storage-and-access

review: Needs Fixing
Revision history for this message
Michał Sawicz (saviq) wrote :

The Columns might be overkill, but you need to wrap the Labels in an Item anyway for the Row to not try and position them next to each other.

Revision history for this message
Mirco Müller (macslow) wrote :

Ah, ok. I'll fix all those issues.

Revision history for this message
kevin gunn (kgunn72) wrote :

wrt >> I'd rename Bubble to Notification
I worry a little that we are overloading "notification"
e.g. if back end is also called notification it might get confusing
am i being a nut? ultimately i'll defer to Saviq/Mirco

Revision history for this message
Michał Sawicz (saviq) wrote :

Sure, that's a valid concern. QML Notification component would end up as a QQmlNotification in C++ world, and anyway namespacing is helping here - it's going to be Unity::Notifications::Notification in C++ world. And even in QML it's recommended to used named imports (import Unity.Notifications 0.1 as Notifications) and then the components exposed from that plugin are available as Notifications.Notification, for example.

What's more, we'll never use (title case) Notification as exposed from the QML Unity.Notifications plugin, at least not in the shell. That object will only be created C++ side.

So, all in all I don't foresee naming conflicts, but to avoid confusion, I could agree to a different name ;). I'd like to avoid "Bubble" nevertheless ;D

Revision history for this message
Mirco Müller (macslow) wrote :

All mentioned suggestions were addressed, with two variations:

* I left the explicit sizes for icon and secondary-icon in place as a "safty-net", for those cases when rogue applications pass in too small or too large icons, thus the layout of the notification does not break away from the design-guideline.

* With the way anchors are used in summayLabel and bodyLabel now, there's no danger of them being accidentally placed next to each other.

Good to merge with trunk?

Revision history for this message
Michał Sawicz (saviq) wrote :

Yeah, explicit sizes of icons / assets is correct, of course.

Revision history for this message
Michał Sawicz (saviq) wrote :

* please order the properties on the component in a more structured way (at least public properties first, and secondaryIcon source just after iconSource)
* components should not have their dimensions predefined, unless they depend (as in this case the height) on their child objects
    width: parent.width
    height: childrenRect.height + units.gu (1)
* don't expose private properties on root objects of components - in this case I don't see the need for that separate property, you can simply use the condition itself
    property bool snapDecision: type == "Notifications.Type.SnapDecision"
* components should not define their overall animations - and you're not changing the opacity of it anyway - you're going to that in the transitions
    Behavior on opacity {NumberAnimation{}}
* I would prefer using a Row for the icons and summary+body, all the margin: units.gu(1) make it hard to read, and it would handle the invisible secondaryIcon automatically
* explicitly check for source !== undefined && source != "", you could also check for status == Image.Ready [1], but that might result in some movement while the image is being loaded
    visible: source
* set sourceSize [2] on Images that come from outside to reduce memory
* this is not correct, use bodyLabel.y + bodyLabel.height - height of items does not include anchor margins - and if you use a Row above, you will just anchor to the row's bottom anyway
  top: (icon.height >= summaryLabel.height + bodyLabel.height) ? icon.bottom : bodyLabel.bottom
* why this??
    parent: notification
* set visibility instead
    topMargin: snapDecision ? units.gu (1) : 0
* there's a binding loop - should go away when you move to using a Row, too
    Notifications.qml:48:19: QML Notification: Binding loop detected for property "height"
* don't use space between function name and parenthesis [units.gu(1), not units.gu (1)]
* use camelCase instead of underscores (action_label)
* no need to prefix action's values with action_
* Notifications should not have its own queue or the displayNotification method, it should be a ListView{}, not a Showable{ListView{}}
* Notifications - see above about predefined dimensions
    anchors.fill: parent
* don't add Notifications to Shell, yet, since there's no model to supply to it
* the tests are not really tests - there's no way they would fail, is there?

[1] http://qt-project.org/doc/qt-5.0/qtquick/qml-qtquick2-image.html#status-prop
[2] http://qt-project.org/doc/qt-5.0/qtquick/qml-qtquick2-image.html#sourceSize-prop

review: Needs Fixing
Revision history for this message
Michał Sawicz (saviq) wrote :

=========

48 + color: Qt.rgba(0, 0, 0, 0.85)
with that, shouldn't you be able to just drop the Item?

=========

54 + top: parent.top
unnecessary

=========

56 + height: childrenRect.height + contentColumn.anchors.margins * 2
since spacing / margins are units.gu(1) everywhere, let's just use contentColumn.spacing everywhere?

=========

81 + width: (body == "") ? units.gu(2) : units.gu(6)
82 + height: (body == "") ? units.gu(2) : units.gu(6)
we need to be able to collapse items that _do_ have body text, no? actually, it's the secondaryIcon that should stay on screen in collapsed state, even if there is a primary icon, no? IMO it's a perfect place to use States and Transitions [1]

expanded => collapsed transition as follows:
SequentialAnimation {
  ParallelAnimation {
    icon shrinks
    icon fades out
    labelColumn.height shrinks to summaryLabel.height
    labelColumn fades out
  }
  PropertyAction {
    icon becomes invisible
    bodyLabel becomes invisible
  }
}

collapsed => expanded transition as follows:
SequentialAnimation {
  PropertyAction {
    icon becomes visible
    bodyLabel becomes visible
  }
  ParallelAnimation {
    icon grows
    icon fades in
    labelColumn.height grows to labelColumn.implicitHeight
    labelColumn fades in
  }
}

you also need to bind labelColumn.clip to Transition.running [2] of the two above.

=========

107 + height: childrenRect.height
this shouldn't be needed in Positioners - they get sized by their children implicitly

=========

113 + width: parent.width - x
this should only be like that in labelColumn, the Labels themselves should anchor to parent.left/right

=========

124 + visible: body != ""
as mentioned above, use States and Transitions for that

=========

141 + visible: type == "Notifications.Type.SnapDecision"
145 + model: actions
reference by object - notification.type, notification.actions

=========

74 + width: parent.width
142 + width: childrenRect.width
you should anchor the Rows to parent.left/right instead

=========

because of the quality issues, let's drop sourceSize for now (sorry)

=========

46 + width: parent.width
191 + anchors.fill: parent
these need to go away (component dimensions should generally be handled in their parents, unless they depend on content dimensions):

=========

225 +
rogue newline

=========

237 +<<<<<<< TREE
238 +=======
239 +add_qml_test(DashPreview)
240 +add_qml_test(NotificationRenderer)
241 +>>>>>>> MERGE-SOURCE
conflict (DashPreview shouldn't be touched)

=========

add some controls next to the view so that you can manually drive the test (see other tests in qmluitests for examples)

=========

[1] http://qt-project.org/doc/qt-5.0/qtquick/qtquick-statesanimations-states.html
[2] http://qt-project.org/doc/qt-5.0/qtquick/qml-qtquick2-transition.html#running-prop

review: Needs Fixing
Revision history for this message
Mirco Müller (macslow) wrote :

> 81 + width: (body == "") ? units.gu(2) : units.gu(6)
> 82 + height: (body == "") ? units.gu(2) : units.gu(6)
> we need to be able to collapse items that _do_ have body text, no? actually,
> it's the secondaryIcon that should stay on screen in collapsed state, even if
> there is a primary icon, no?

No, this here is meant for the special "icon+summary" layout case Design wants. This is different from a collapsing snap-decision, which is only meant to be used on large screens.

> 141 + visible: type == "Notifications.Type.SnapDecision"
> 145 + model: actions
> reference by object - notification.type, notification.actions

That does not work. How can I make the object known? Do I need to change the way data is passed in the test?

> 46 + width: parent.width
> 191 + anchors.fill: parent
> these need to go away (component dimensions should generally be handled in
> their parents, unless they depend on content dimensions):

I could get rid of "anchors.fill: parent", but I have to keep "width: parent.width". That way the width of the ListView holding the notifications follows any embedding parent-item. Otherwise there's no way to tell the ListView what horizontal space it may occupy.

Revision history for this message
Michał Sawicz (saviq) wrote :

W dniu 09.04.2013 17:07, Mirco Müller pisze:
>> 81 + width: (body == "") ? units.gu(2) : units.gu(6)
>> > 82 + height: (body == "") ? units.gu(2) : units.gu(6)
>> > we need to be able to collapse items that _do_ have body text, no? actually,
>> > it's the secondaryIcon that should stay on screen in collapsed state, even if
>> > there is a primary icon, no?
> No, this here is meant for the special "icon+summary" layout case Design wants. This is different from a collapsing snap-decision, which is only meant to be used on large screens.

Right, but since the icon is that small, do we not want to get rid of
the UbuntuShape for it? We could easily switch the source of the icons
instead of the dimensions of them? Is the lack of a body the only
determinant of whether it's a icon+summary layout? Does that mean body
is required for the generic case? It doesn't make sense for the icon to
be an avatar, for example, when it's just 2x2 GUs?

>> > 141 + visible: type == "Notifications.Type.SnapDecision"
>> > 145 + model: actions
>> > reference by object - notification.type, notification.actions
> That does not work. How can I make the object known? Do I need to change the way data is passed in the test?

Hum? The top level UbuntuShape has «id: notification» and type and
actions properties, can you not access those for some reason?

>> > 46 + width: parent.width
>> > 191 + anchors.fill: parent
>> > these need to go away (component dimensions should generally be handled in
>> > their parents, unless they depend on content dimensions):
> I could get rid of "anchors.fill: parent", but I have to keep "width: parent.width". That way the width of the ListView holding the notifications follows any embedding parent-item. Otherwise there's no way to tell the ListView what horizontal space it may occupy.

That's the «width: parent.width» in Notification.qml:29, it should be in
Notifications.qml:29 - it's in the delegate where you should define the
dimensions (again, unless they depend on their content). I think the
rule of thumb is: top-level component in a file should not have its
dimensions defined, unless they depend on the contents.

Case in point: it's Shell.qml that will decide on the dimensions of
Notifications {}, it's Notifications.qml that will decide on the
dimensions of Notification {} - it's delegate. And you should use
anchors there, too.

Cheers,
--
Michał Sawicz <email address hidden>
Canonical Services Ltd.

Revision history for this message
Michał Sawicz (saviq) wrote :

Hey, make sure to merge trunk, soon, there's conflicts.

review: Needs Fixing
Revision history for this message
Michael Zanetti (mzanetti) wrote :

if possible remove wait()s from the tests. They take ~8 secs now while I think they could run in < 0.5 secs. (Makes a huge difference in Jenkins where all 300 tests we have are summed up)

review: Needs Fixing
Revision history for this message
Michael Zanetti (mzanetti) wrote :

I'd like to see tests for the different mouse interactions:

- notifcation is click-through or not (interactive/not interactive?)
- click buttons in notifications

review: Needs Fixing
Revision history for this message
Michael Zanetti (mzanetti) wrote :

604 + wait(100)

really required? I know only because view.count == model.count it doesn't mean that the delegates are created already. I think still wait(0) should be enough. (Fine as long it is set to WIP - just wanted to mention it for completemess)

* Regarding the if() around the signal spys: I think it would be better to add the conditions to the test data. Because if you decide on interactiveArea.visible, your test wouldn't catch a bug where interactiveArea is already visible by mistake. Better extending testdata with data like "isInteractive: true; hasButtons: false"

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:575
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~macslow/unity/phablet-notification-renderer/+merge/155512/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/709/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-phablet-qmluitests/527/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/711/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/709/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/585/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/589/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/709/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

debian/qml-phone-shell.install needs to be fixed:

dh_install: usr/share/qml-phone-shell/Notifications/Notification.qml exists in debian/tmp but is not installed to anywhere
dh_install: usr/share/qml-phone-shell/Notifications/Notifications.qml exists in debian/tmp but is not installed to anywhere
dh_install: usr/share/qml-phone-shell/Notifications/timings.js exists in debian/tmp but is not installed to anywhere
dh_install: usr/share/qml-phone-shell/Notifications/graphics/tile.png exists in debian/tmp but is not installed to anywhere
dh_install: usr/share/qml-phone-shell/Notifications/graphics/avatar3.jpg exists in debian/tmp but is not installed to anywhere
dh_install: usr/share/qml-phone-shell/Notifications/graphics/avatar1.jpg exists in debian/tmp but is not installed to anywhere
dh_install: usr/share/qml-phone-shell/Notifications/graphics/icon_phone.png exists in debian/tmp but is not installed to anywhere
dh_install: usr/share/qml-phone-shell/Notifications/graphics/avatar2.jpg exists in debian/tmp but is not installed to anywhere
dh_install: missing files, aborting

review: Needs Fixing
Revision history for this message
Michael Zanetti (mzanetti) wrote :

I don't think those images should be installed with the shell. They should rather be moved into tests/mocks or the like

review: Needs Fixing
Revision history for this message
Michał Sawicz (saviq) wrote :

Hey, no need to put it as Work In Progress when you go and fix review comments. Leaving it at Needs Review is fine.

Revision history for this message
Michael Zanetti (mzanetti) wrote :

224 + easing.type: Timings.easing
288 +var easing = "OutQuint";

Interesting... I just thought easings.type is a enum instead of a string. Looking it up in the docs it seems using strings is possible. Easing.OutQuint the preferred one though (I guess because it gives us code completion in QtCreator while strings don't):

<snip>

QML Basic Type: enumeration
An enumeration type consists of a set of named values.
An enumeration value may be specifed as either a string:

 Text { horizontalAlignment: "AlignRight" }

or as <Element>.<value>:

 Text { horizontalAlignment: Text.AlignRight }

The second form is preferred.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:578
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/714/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/533
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/716
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/716/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/714
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/590
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/590/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/594
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1248

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/714/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:578
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/724/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/549
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/726
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/726/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/724
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/600
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/600/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/604
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1266

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/724/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :
Download full text (3.2 KiB)

Tests
========================

627 + wait(0)

Looks unnecessary?

========

643 + interactiveAreaSpy.clear()
644 + clickThroughSpy.clear()

651 + clickThroughSpy.clear()

You should do that in a « function cleanup() » instead - it's called after every test case.

========

646 + interactiveAreaSpy.wait()
647 + verify(interactiveAreaSpy.count == 1)

653 + clickThroughSpy.wait()
654 + verify(clickThroughSpy.count == 1)

SignalSpy.wait() already verifies the count is 1.

========

648 + verify(clickThroughSpy.count == 0)

Should use compare().

========

649 + }
650 + else {

Please keep in one line.

========

638 + compare(icon.visible, data.iconVisible, "check for icon of notification" + (mockModel.count - 1))
639 + compare(interactiveArea.visible, data.interactiveAreaVisible, "check for interactive notification" + (mockModel.count - 1))

657 + compare(secondaryIcon.visible, data.secondaryIconVisible, "check for secondary icon" + (mockModel.count - 1))
658 + compare(summaryLabel.visible, data.summaryVisible, "check for summary-label" + (mockModel.count - 1))
659 + compare(bodyLabel.visible, data.bodyVisible, "check for body-label" + (mockModel.count - 1))
660 + compare(buttonRow.visible, data.buttonRowVisible, "check for buttons" + (mockModel.count - 1))

The messages could be improved, and also "notification1", "notification2" are not really useful - the tag: from _data() shows which test is being run.

========

662 + if(data.buttonRowVisible) {
663 + var buttonCancel = findChild(buttonRow, "button0")
664 + var buttonAccept = findChild(buttonRow, "button1")
665 +
666 + buttonSpy.target = buttonCancel
667 + buttonSpy.clear()
668 + mouseClick(buttonCancel, buttonCancel.width / 2, buttonCancel.height / 2)
669 + buttonSpy.wait()
670 + verify(buttonSpy.count == 1)
671 +
672 + buttonSpy.target = buttonAccept
673 + buttonSpy.clear()
674 + mouseClick(buttonAccept, buttonAccept.width / 2, buttonAccept.height / 2)
675 + buttonSpy.wait()
676 + verify(buttonSpy.count == 1)
677 + }

This only tests that Button.clicked signal is working. You should instead create an actionInvoked(string) signal on Notification and spy on this signal, making sure that SignalSpy.signalArguments are as expected.

Another run through the code itself
===================================

 46 + width: parent.width
 47 + height: childrenRect.height

Again, top-level components in a file should not have width or height defined. Use implicitHeight and get rid of width altogether - it's determined by the delegate definition in Notifications.qml.

========

 57 + visible: type == "Notifications.Type.Interactive"

MouseAreas are not graphical, use "enabled" instead.

========

 58 + onClicked: print("clicked " + actions.get(0).id)

Forward through a...

Read more...

review: Needs Fixing
Revision history for this message
Michał Sawicz (saviq) wrote :
Download full text (3.3 KiB)

There's still some questions from https://code.launchpad.net/~macslow/unity/phablet-notification-renderer/+merge/155512/comments/346145 unanswered.

=========

 50 + visible: opacity > 0

Is that needed?

=========

 57 + visible: type == "Notifications.Type.Interactive"

Where is type defined?

=========

 68 + margins: units.gu(1)
 69 + }
 70 + height: childrenRect.height + contentColumn.anchors.margins
 71 + spacing: units.gu(1)

 76 + spacing: units.gu(1)

147 + spacing: units.gu(1)

163 + width: (topRow.width - units.gu(1)) / 2

I mentioned earlier a common spacing could be useful, go with the below instead:

 68 + margins: spacing
 69 + }
 70 + height: childrenRect.height + spacing
 71 + spacing: units.gu(1)

 76 + spacing: contentColumn.spacing

147 + spacing: contentColumn.spacing

163 + width: (topRow.width - buttonRow.spacing) / 2

=========

 81 + height: childrenRect.height

That's unnecessary - default in Positioners.

=========

 83 + UbuntuShape {
 84 + id: icon
 85 +
 86 + objectName: "icon"
 87 + width: (body == "") ? units.gu(2) : units.gu(6)
 88 + height: (body == "") ? units.gu(2) : units.gu(6)
 89 + image: Image {
 90 + id: avatarIcon
 91 +
 92 + fillMode: Image.PreserveAspectCrop
 93 + }
 94 + }
 95 +
 96 + Image {
 97 + id: secondaryIcon
 98 +
 99 + objectName: "secondaryIcon"
100 + width: units.gu(2)
101 + height: units.gu(2)
102 + visible: source !== undefined && source != "" && bodyLabel.visible
103 + fillMode: Image.PreserveAspectCrop
104 + }

I'm still not sure we want a 2x2 GU UbuntuShape, see the comment mentioned at the top of this one.

=========

149 + visible: type == "Notifications.Type.SnapDecision"

156 + model: actions

See the comment at the top.

=========

159 + id: button
160 +

Looks unused.

=========

163 + width: (topRow.width - units.gu(1)) / 2

Why topRow? Shouldn't this be buttonRow.width?

=========

166 + onClicked: print("clicked " + id)

Same as before - forward via a actionInvoked(string) signal.

=========

208 + delegate: Notification {
209 + objectName: "notification" + index
210 + iconSource: model.icon
211 + secondaryIconSource: model.secondaryIcon
212 + summary: model.summary
213 + body: model.body
214 + actions: model.actions
215 + }

Use anchors { left: parent.left; right: parent.right } to make the delegates fill Notifications horizontally. Also, "model" here is unnecessary - you can access the roles directly.

=========

309 +add_qml_test(Notifications NotificationRenderer)

326 === added file 'tests/qmltests/Notifications/tst_NotificationRenderer.qml'
327 --- tests/qmltests/Notifications/tst_NotificationRenderer.qml 1970-01-01 ...

Read more...

review: Needs Fixing
Revision history for this message
Michał Sawicz (saviq) wrote :

Ah one more thing, please reduce the set of test assets to a minimum. You can use those from /graphics instead.

Obviously we need to clean those up, but let's not add more instead.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:579
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/740/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/568
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/741
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/741/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/739
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/616
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/616/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/620
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1284

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/740/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:582
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/742/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/572
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/744
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/744/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/742
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/618
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/618/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/622
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1286

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/742/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:584
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/744/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/575
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/746
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/746/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/744
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/620
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/620/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/624
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1289

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/744/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Mirco Müller (macslow) wrote :

Hm... CI of my branch failed because of a failure with test_hide_hud_click.

Revision history for this message
Mirco Müller (macslow) wrote :

qml_phone_shell.tests.testconfigurations.TestNexus10.test_hide_hud_click() to be precise

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:588
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/745/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/578
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/747
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/747/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/745
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/621
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/621/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/625
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1291

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/745/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:588
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/746/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/581
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/748
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/748/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/746
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/622
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/622/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/626
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1294

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/746/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:588
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/749/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/751
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/751/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/749
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/625
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/625/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/629
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1297

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/749/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:588
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/750/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/585
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/752
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/752/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/750
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/626
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/626/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/630
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1298

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/750/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:589
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/763/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/621
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/765
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/765/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/763
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/639
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/639/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/643
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1313

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/763/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:589
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/765/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-phablet-qmluitests/631/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/767
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/767/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/765
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/641/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/645/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/765/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:590
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/766/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-phablet-qmluitests/632/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/768
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-armhf-ci/768/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-quantal-i386-ci/766
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/642/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/646/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/766/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:590
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/775/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/652
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/651/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/655/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/775/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:592
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/776/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/654
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/652
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/652/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/656
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1323

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/776/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:592
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/777/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/655
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/653
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/653/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/657
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1324

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/777/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:593
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/853/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/774
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/729
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/729/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/733

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/853/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:593
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/861/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/784
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/737
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/737/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/741

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/861/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:593
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/862/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/786
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/738
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/738/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/742

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/862/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:594
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/899/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/834
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/775
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/775/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/779

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/899/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:595
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/907/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/858
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/783/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/787
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1439

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/907/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:595
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/908/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/860
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/784
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/784/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/788
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1441

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/908/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:595
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/910/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/865
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/786
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/786/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/790
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1443

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/910/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:597
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/912/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/867
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/788
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/788/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/792
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1445

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/912/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:598
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/913/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/871
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/789
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/789/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/793
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1447

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/913/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:598
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/915/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/873
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/791
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/791/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/795
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1449

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/915/rebuild

review: Approve (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

Looks good to me now.

review: Approve
Revision history for this message
Michał Sawicz (saviq) wrote :

Hey, some last still-remaining tweaks:

=====

 77 + spacing: units.gu(1)

Should be contentColumn.spacing

=====

 58 + enabled: type == "Notifications.Type.Interactive"

150 + visible: type == "Notifications.Type.SnapDecision"

157 + model: actions

Should be notification.type ==, notification.actions

=====

162 + width: (topRow.width - buttonRow.spacing) / 2

Why topRow? Shouldn't this be buttonRow.width?

=====

That's it.

review: Needs Fixing
Revision history for this message
Mirco Müller (macslow) wrote :

Did those requested tweaks.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:599
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/928/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/886
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/804
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/804/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/808
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1458

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/928/rebuild

review: Approve (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :

Yup!

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity-phablet-autolanding/325/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/900
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-autolanding/259
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-autolanding/259/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-autolanding/259
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1480/console

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2013-04-26 14:54:51 +0000
3+++ CMakeLists.txt 2013-05-15 15:17:27 +0000
4@@ -183,6 +183,7 @@
5 Panel
6 Launcher
7 SideStage
8+ Notifications
9 )
10
11 install(DIRECTORY ${QML_DIRS}
12
13=== added directory 'Notifications'
14=== added file 'Notifications/Notification.qml'
15--- Notifications/Notification.qml 1970-01-01 00:00:00 +0000
16+++ Notifications/Notification.qml 2013-05-15 15:17:27 +0000
17@@ -0,0 +1,153 @@
18+/*
19+ * Copyright (C) 2013 Canonical, Ltd.
20+ *
21+ * This program is free software; you can redistribute it and/or modify
22+ * it under the terms of the GNU General Public License as published by
23+ * the Free Software Foundation; version 3.
24+ *
25+ * This program is distributed in the hope that it will be useful,
26+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
27+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
28+ * GNU General Public License for more details.
29+ *
30+ * You should have received a copy of the GNU General Public License
31+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
32+ */
33+
34+import QtQuick 2.0
35+import Ubuntu.Components 0.1
36+
37+UbuntuShape {
38+ id: notification
39+
40+ property string type
41+ property alias iconSource: avatarIcon.source
42+ property alias secondaryIconSource: secondaryIcon.source
43+ property alias summary: summaryLabel.text
44+ property alias body: bodyLabel.text
45+ property var actions
46+
47+ signal actionInvoked(string buttonId)
48+
49+ implicitHeight: childrenRect.height
50+ color: Qt.rgba(0, 0, 0, 0.85)
51+ opacity: 0
52+
53+ MouseArea {
54+ id: interactiveArea
55+
56+ anchors.fill: contentColumn
57+ objectName: "interactiveArea"
58+ enabled: notification.type == "Notifications.Type.Interactive"
59+ onClicked: notification.actionInvoked(actions.get(0).id)
60+ }
61+
62+ Column {
63+ id: contentColumn
64+
65+ anchors {
66+ left: parent.left
67+ right: parent.right
68+ top: parent.top
69+ margins: spacing
70+ }
71+ height: childrenRect.height + spacing
72+ spacing: units.gu(1)
73+
74+ Row {
75+ id: topRow
76+
77+ spacing: contentColumn.spacing
78+ anchors {
79+ left: parent.left
80+ right: parent.right
81+ }
82+
83+ UbuntuShape {
84+ id: icon
85+
86+ objectName: "icon"
87+ width: units.gu(6)
88+ height: units.gu(6)
89+ visible: iconSource !== undefined && iconSource != ""
90+ image: Image {
91+ id: avatarIcon
92+
93+ fillMode: Image.PreserveAspectCrop
94+ }
95+ }
96+
97+ Image {
98+ id: secondaryIcon
99+
100+ objectName: "secondaryIcon"
101+ width: units.gu(2)
102+ height: units.gu(2)
103+ visible: source !== undefined && source != ""
104+ fillMode: Image.PreserveAspectCrop
105+ }
106+
107+ Column {
108+ id: labelColumn
109+ width: parent.width - x
110+
111+ Label {
112+ id: summaryLabel
113+
114+ objectName: "summaryLabel"
115+ anchors {
116+ left: parent.left
117+ right: parent.right
118+ }
119+ fontSize: "medium"
120+ font.bold: true
121+ color: "#f3f3e7"
122+ elide: Text.ElideRight
123+ }
124+
125+ Label {
126+ id: bodyLabel
127+
128+ objectName: "bodyLabel"
129+ anchors {
130+ left: parent.left
131+ right: parent.right
132+ }
133+ visible: body != ""
134+ fontSize: "small"
135+ color: "#f3f3e7"
136+ opacity: 0.6
137+ wrapMode: Text.WordWrap
138+ maximumLineCount: 10
139+ elide: Text.ElideRight
140+ }
141+ }
142+ }
143+
144+ Row {
145+ id: buttonRow
146+
147+ objectName: "buttonRow"
148+ spacing: contentColumn.spacing
149+ layoutDirection: Qt.RightToLeft
150+ visible: notification.type == "Notifications.Type.SnapDecision"
151+ anchors {
152+ left: parent.left
153+ right: parent.right
154+ }
155+
156+ Repeater {
157+ model: notification.actions
158+
159+ Button {
160+ objectName: "button" + index
161+ color: Positioner.isFirstItem ? "#d85317" : "#cdcdcb"
162+ width: (buttonRow.width - buttonRow.spacing) / 2
163+ height: units.gu(4)
164+ text: label
165+ onClicked: notification.actionInvoked(id)
166+ }
167+ }
168+ }
169+ }
170+}
171
172=== added file 'Notifications/Notifications.qml'
173--- Notifications/Notifications.qml 1970-01-01 00:00:00 +0000
174+++ Notifications/Notifications.qml 2013-05-15 15:17:27 +0000
175@@ -0,0 +1,77 @@
176+/*
177+ * Copyright (C) 2013 Canonical, Ltd.
178+ *
179+ * This program is free software; you can redistribute it and/or modify
180+ * it under the terms of the GNU General Public License as published by
181+ * the Free Software Foundation; version 3.
182+ *
183+ * This program is distributed in the hope that it will be useful,
184+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
185+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
186+ * GNU General Public License for more details.
187+ *
188+ * You should have received a copy of the GNU General Public License
189+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
190+ */
191+
192+import QtQuick 2.0
193+import Ubuntu.Components 0.1
194+import "../Components"
195+import "timings.js" as Timings
196+
197+ListView {
198+ id: notificationRenderer
199+
200+ objectName: "notificationRenderer"
201+ interactive: false
202+
203+ spacing: units.gu(.5)
204+ delegate: Notification {
205+ objectName: "notification" + index
206+ anchors {
207+ left: parent.left
208+ right: parent.right
209+ }
210+ type: model.type
211+ iconSource: model.icon
212+ secondaryIconSource: model.secondaryIcon
213+ summary: model.summary
214+ body: model.body
215+ actions: model.actions
216+ }
217+
218+ populate: Transition {
219+ NumberAnimation {
220+ property: "opacity"
221+ to: 1
222+ duration: Timings.snapBeat
223+ easing.type: Timings.easing
224+ }
225+ }
226+
227+ add: Transition {
228+ NumberAnimation {
229+ property: "opacity"
230+ to: 1
231+ duration: Timings.snapBeat
232+ easing.type: Timings.easing
233+ }
234+ }
235+
236+ remove: Transition {
237+ NumberAnimation {
238+ property: "opacity"
239+ to: 0
240+ duration: Timings.fastBeat
241+ easing.type: Timings.easing
242+ }
243+ }
244+
245+ displaced: Transition {
246+ NumberAnimation {
247+ properties: "x,y"
248+ duration: Timings.fastBeat
249+ easing.type: Timings.easing
250+ }
251+ }
252+}
253
254=== added file 'Notifications/timings.js'
255--- Notifications/timings.js 1970-01-01 00:00:00 +0000
256+++ Notifications/timings.js 2013-05-15 15:17:27 +0000
257@@ -0,0 +1,23 @@
258+/*
259+ * Copyright (C) 2013 Canonical, Ltd.
260+ *
261+ * This program is free software; you can redistribute it and/or modify
262+ * it under the terms of the GNU General Public License as published by
263+ * the Free Software Foundation; version 3.
264+ *
265+ * This program is distributed in the hope that it will be useful,
266+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
267+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
268+ * GNU General Public License for more details.
269+ *
270+ * You should have received a copy of the GNU General Public License
271+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
272+ */
273+
274+var rhythm = 700;
275+var fastBeat = 0.5 * rhythm;
276+var snapBeat = 0.5 * fastBeat;
277+var slowBeat = rhythm;
278+var sleepyBeat = slowBeat * 2;
279+
280+var easing = Easing.OutQuint;
281
282=== modified file 'debian/qml-phone-shell.install'
283--- debian/qml-phone-shell.install 2013-04-23 15:58:17 +0000
284+++ debian/qml-phone-shell.install 2013-05-15 15:17:27 +0000
285@@ -7,6 +7,7 @@
286 /usr/share/qml-phone-shell/Greeter/*
287 /usr/share/qml-phone-shell/Hud/*
288 /usr/share/qml-phone-shell/Launcher/*
289+/usr/share/qml-phone-shell/Notifications/*
290 /usr/share/qml-phone-shell/Panel/*
291 /usr/share/qml-phone-shell/Shell.qml
292 /usr/share/qml-phone-shell/SideStage/*
293
294=== modified file 'tests/qmltests/CMakeLists.txt'
295--- tests/qmltests/CMakeLists.txt 2013-05-14 13:42:33 +0000
296+++ tests/qmltests/CMakeLists.txt 2013-05-15 15:17:27 +0000
297@@ -48,6 +48,7 @@
298 add_qml_test(Hud Hud)
299 add_qml_test(Hud Result)
300 add_qml_test(Launcher Launcher)
301+add_qml_test(Notifications Notifications)
302 add_qml_test(Panel IndicatorRow)
303 add_qml_test(Panel Indicators)
304 add_qml_test(Panel MenuContent)
305
306=== added directory 'tests/qmltests/Notifications'
307=== added file 'tests/qmltests/Notifications/tst_Notifications.qml'
308--- tests/qmltests/Notifications/tst_Notifications.qml 1970-01-01 00:00:00 +0000
309+++ tests/qmltests/Notifications/tst_Notifications.qml 2013-05-15 15:17:27 +0000
310@@ -0,0 +1,335 @@
311+/*
312+ * Copyright (C) 2013 Canonical, Ltd.
313+ *
314+ * This program is free software; you can redistribute it and/or modify
315+ * it under the terms of the GNU General Public License as published by
316+ * the Free Software Foundation; version 3.
317+ *
318+ * This program is distributed in the hope that it will be useful,
319+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
320+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
321+ * GNU General Public License for more details.
322+ *
323+ * You should have received a copy of the GNU General Public License
324+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
325+ */
326+
327+import QtQuick 2.0
328+import QtTest 1.0
329+import ".."
330+import "../../../Notifications"
331+import Ubuntu.Components 0.1
332+import Unity.Test 0.1
333+
334+Row {
335+ id: rootRow
336+
337+ ListModel {
338+ id: mockModel
339+ }
340+
341+ function addSnapDecisionNotification() {
342+ var n = {
343+ type: "Notifications.Type.SnapDecision",
344+ summary: "Tom Ato",
345+ body: "Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua.",
346+ icon: "../graphics/avatars/funky.png",
347+ secondaryIcon: "../graphics/applicationIcons/facebook.png",
348+ actions: [{ id: "ok_id", label: "Ok"},
349+ { id: "cancel_id", label: "Cancel"}]
350+ }
351+
352+ mockModel.append(n)
353+ }
354+
355+ function addEphemeralNotification() {
356+ var n = {
357+ type: "Notifications.Type.Ephemeral",
358+ summary: "Cole Raby",
359+ body: "I did not expect it to be that late.",
360+ icon: "../graphics/avatars/amanda.png",
361+ secondaryIcon: "../graphics/applicationIcons/facebook.png",
362+ actions: []
363+ }
364+
365+ mockModel.append(n)
366+ }
367+
368+ function addEphemeralIconSummaryNotification() {
369+ var n = {
370+ type: "Notifications.Type.Ephemeral",
371+ summary: "Photo upload completed",
372+ body: "",
373+ icon: "",
374+ secondaryIcon: "../graphics/applicationIcons/facebook.png",
375+ actions: []
376+ }
377+
378+ mockModel.append(n)
379+ }
380+
381+ function addInteractiveNotification() {
382+ var n = {
383+ type: "Notifications.Type.Interactive",
384+ summary: "Interactive notification",
385+ body: "This is a notification that can be clicked",
386+ icon: "../graphics/avatars/anna_olsson.png",
387+ secondaryIcon: "",
388+ actions: [{ id: "reply_id", label: "Dummy"}],
389+ }
390+
391+ mockModel.append(n)
392+ }
393+
394+ function clearNotifications() {
395+ mockModel.clear()
396+ }
397+
398+ function remove1stNotification() {
399+ if (mockModel.count > 0)
400+ mockModel.remove(0)
401+ }
402+
403+ Rectangle {
404+ id: notificationsRect
405+
406+ width: units.gu(40)
407+ height: units.gu(71)
408+
409+ MouseArea{
410+ id: clickThroughCatcher
411+
412+ anchors.fill: parent
413+ }
414+
415+ Notifications {
416+ id: notifications
417+
418+ anchors.fill: parent
419+ anchors.margins: units.gu(1)
420+ model: mockModel
421+ }
422+ }
423+
424+ Rectangle {
425+ id: interactiveControls
426+
427+ width: units.gu(30)
428+ height: units.gu(71)
429+ color: "grey"
430+
431+ Column {
432+ spacing: units.gu(1)
433+ anchors.fill: parent
434+ anchors.margins: units.gu(1)
435+
436+ Button {
437+ width: parent.width
438+ text: "add a snap-decision"
439+ onClicked: addSnapDecisionNotification()
440+ }
441+
442+ Button {
443+ width: parent.width
444+ text: "add an ephemeral"
445+ onClicked: addEphemeralNotification()
446+ }
447+
448+ Button {
449+ width: parent.width
450+ text: "add an icon-summary"
451+ onClicked: addEphemeralIconSummaryNotification()
452+ }
453+
454+ Button {
455+ width: parent.width
456+ text: "add an interactive"
457+ onClicked: addInteractiveNotification()
458+ }
459+
460+ Button {
461+ width: parent.width
462+ text: "remove 1st notification"
463+ onClicked: remove1stNotification()
464+ }
465+
466+ Button {
467+ width: parent.width
468+ text: "clear model"
469+ onClicked: clearNotifications()
470+ }
471+ }
472+ }
473+
474+ UnityTestCase {
475+ id: root
476+ name: "NotificationRendererTest"
477+ when: windowShown
478+
479+ function test_NotificationRenderer_data() {
480+ return [
481+ {
482+ tag: "Snap Decision with secondary icon",
483+ type: "Notifications.Type.SnapDecision",
484+ summary: "Tom Ato",
485+ body: "Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat, sed diam voluptua.",
486+ icon: "../graphics/avatars/funky.png",
487+ secondaryIcon: "../graphics/applicationIcons/facebook.png",
488+ actions: [{ id: "ok_id", label: "Ok"},
489+ { id: "cancel_id", label: "Cancel"}],
490+ summaryVisible: true,
491+ bodyVisible: true,
492+ interactiveAreaEnabled: false,
493+ iconVisible: true,
494+ secondaryIconVisible: true,
495+ buttonRowVisible: true
496+ },
497+ {
498+ tag: "Ephemeral notification - icon-summary layout",
499+ type: "Notifications.Type.Ephemeral",
500+ summary: "Photo upload completed",
501+ body: "",
502+ icon: "",
503+ secondaryIcon: "../graphics/applicationIcons/facebook.png",
504+ actions: [],
505+ summaryVisible: true,
506+ bodyVisible: false,
507+ interactiveAreaEnabled: false,
508+ iconVisible: false,
509+ secondaryIconVisible: true,
510+ buttonRowVisible: false
511+ },
512+ {
513+ tag: "Ephemeral notification - check suppression of secondary icon for icon-summary layout",
514+ type: "Notifications.Type.Ephemeral",
515+ summary: "New comment successfully published",
516+ body: "",
517+ icon: "",
518+ secondaryIcon: "../graphics/applicationIcons/facebook.png",
519+ actions: [],
520+ summaryVisible: true,
521+ bodyVisible: false,
522+ interactiveAreaEnabled: false,
523+ iconVisible: false,
524+ secondaryIconVisible: true,
525+ buttonRowVisible: false
526+ },
527+ {
528+ tag: "Interactive notification",
529+ type: "Notifications.Type.Interactive",
530+ summary: "Interactive notification",
531+ body: "This is a notification that can be clicked",
532+ icon: "../graphics/avatars/amanda.png",
533+ secondaryIcon: "",
534+ actions: [{ id: "reply_id", label: "Dummy"}],
535+ summaryVisible: true,
536+ bodyVisible: true,
537+ interactiveAreaEnabled: true,
538+ iconVisible: true,
539+ secondaryIconVisible: false,
540+ buttonRowVisible: false
541+ },
542+ {
543+ tag: "Snap Decision without secondary icon",
544+ type: "Notifications.Type.SnapDecision",
545+ summary: "Bro Coly",
546+ body: "At vero eos et accusam et justo duo dolores et ea rebum. Stet clita kasd gubergren, no sea takimata sanctus est Lorem ipsum dolor sit amet.",
547+ icon: "../graphics/avatars/anna_olsson.png",
548+ secondaryIcon: "",
549+ actions: [{ id: "accept_id", label: "Accept"},
550+ { id: "reject_id", label: "Reject"}],
551+ summaryVisible: true,
552+ bodyVisible: true,
553+ interactiveAreaEnabled: false,
554+ iconVisible: true,
555+ secondaryIconVisible: false,
556+ buttonRowVisible: true
557+ },
558+ {
559+ tag: "Ephemeral notification",
560+ type: "Notifications.Type.Ephemeral",
561+ summary: "Cole Raby",
562+ body: "I did not expect it to be that late.",
563+ icon: "../graphics/avatars/funky.png",
564+ secondaryIcon: "../graphics/applicationIcons/facebook.png",
565+ actions: [],
566+ summaryVisible: true,
567+ bodyVisible: true,
568+ interactiveAreaEnabled: false,
569+ iconVisible: true,
570+ secondaryIconVisible: true,
571+ buttonRowVisible: false
572+ }
573+ ]
574+ }
575+
576+ SignalSpy {
577+ id: clickThroughSpy
578+
579+ target: clickThroughCatcher
580+ signalName: "clicked"
581+ }
582+
583+ SignalSpy {
584+ id: actionSpy
585+ signalName: "actionInvoked"
586+ }
587+
588+ function cleanup() {
589+ clickThroughSpy.clear()
590+ actionSpy.clear()
591+ }
592+
593+ function test_NotificationRenderer(data) {
594+ // populate model with some mock notifications
595+ mockModel.append(data)
596+
597+ // make sure the model is properly updated before going on
598+ tryCompare(notifications, "count", mockModel.count)
599+
600+ var notification = findChild(notifications, "notification" + (mockModel.count - 1))
601+ var icon = findChild(notification, "icon")
602+ var interactiveArea = findChild(notification, "interactiveArea")
603+ var secondaryIcon = findChild(notification, "secondaryIcon")
604+ var summaryLabel = findChild(notification, "summaryLabel")
605+ var bodyLabel = findChild(notification, "bodyLabel")
606+ var buttonRow = findChild(notification, "buttonRow")
607+ waitForRendering(buttonRow)
608+
609+ compare(icon.visible, data.iconVisible, "avatar-icon visibility is incorrect")
610+ compare(interactiveArea.enabled, data.interactiveAreaEnabled, "check for interactive area")
611+
612+ if(data.interactiveAreaEnabled) {
613+ actionSpy.target = notification
614+ mouseClick(notification, notification.width / 2, notification.height / 2)
615+ actionSpy.wait()
616+ compare(actionSpy.signalArguments[0][0], data.actions[0]["id"], "got wrong id for interactive action")
617+ compare(clickThroughSpy.count, 0, "click on interactive notification fell through")
618+ } else {
619+ mouseClick(notification, notification.width / 2, notification.height / 2)
620+ clickThroughSpy.wait()
621+ }
622+
623+ compare(secondaryIcon.visible, data.secondaryIconVisible, "secondary-icon visibility is incorrect")
624+ compare(summaryLabel.visible, data.summaryVisible, "summary-text visibility is incorrect")
625+ compare(bodyLabel.visible, data.bodyVisible, "body-text visibility is incorrect")
626+ compare(buttonRow.visible, data.buttonRowVisible, "button visibility is incorrect")
627+
628+ if(data.buttonRowVisible) {
629+ var buttonCancel = findChild(buttonRow, "button1")
630+ var buttonAccept = findChild(buttonRow, "button0")
631+
632+ waitForRendering(notification)
633+ actionSpy.target = notification
634+ mouseClick(buttonCancel, buttonCancel.width / 2, buttonCancel.height / 2)
635+ actionSpy.wait()
636+ compare(actionSpy.signalArguments[0][0], data.actions[1]["id"], "got wrong id for negative action")
637+ actionSpy.clear()
638+
639+ mouseClick(buttonAccept, buttonAccept.width / 2, buttonAccept.height / 2)
640+ actionSpy.wait()
641+ compare(actionSpy.signalArguments[0][0], data.actions[0]["id"], "got wrong id positive action")
642+ }
643+ }
644+ }
645+}

Subscribers

People subscribed via source and target branches