Merge lp:~larsu/notify-osd/update-sync into lp:notify-osd

[Lars Karlitski]

Update contents of a synchronous notification when replacing it

Update contents of a synchronous notification when replacing it

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:476
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/~larsu/notify-osd/update-sync/+merge/194364/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/notify-osd-ci/11/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notify-osd-trusty-amd64-ci/7
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notify-osd-trusty-armhf-ci/7
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notify-osd-trusty-i386-ci/7

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/notify-osd-ci/11/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:476
http://jenkins.qa.ubuntu.com/job/notify-osd-ci/12/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notify-osd-trusty-amd64-ci/8
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notify-osd-trusty-armhf-ci/8
    SUCCESS: http://jenkins.qa.ubuntu.com/job/notify-osd-trusty-i386-ci/8

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/notify-osd-ci/12/rebuild

review: Approve (continuous-integration)

[Mirco Müller (macslow)]

While the patch is generally ok code-wise, this still is a shift from notify-osd's design-spec. Since I've not seen any feedback form Design on the bug in the comments regarding this change, I suggest getting their input on the issue.

Meanwhile I'll provide a solution in the bug's comments, which stays within notify-osd's initial design and addresses the requirement within this scope.

review: Needs Information

[Matthew Paul Thomas (mpt)]

It is not a shift from the spec: as I said in bug 404658 four years ago, the spec doesn't cover this situation at all. And even if it did, that would be a poor excuse for an API allowing both an icon and text, but ignoring the text!

When we designed Notify OSD, the only hardware-response notifications we considered were volume and brightness, and both of those just happened to involve meters rather than text. The wi-fi, Bluetooth, and display use cases make much more sense with text.

review: Approve (design)

[Mirco Müller (macslow)]

With mpt's blessing :) it's good to go. I did the usual notify-osd release testing and also wrote a new example, which exercises the new feature with the additional icons.

http://ubuntuone.com/7kO2H2MUKZCOj2CtbW6RWp

It would also be nice if you'd add the new examples I wrote available here http://pastebin.ubuntu.com/6519212 I don't want to mess with this branch and keep it blocked any longer.

review: Approve

=== modified file 'src/stack.c'
--- src/stack.c	2013-09-08 08:38:11 +0000
+++ src/stack.c	2013-11-07 15:39:43 +0000
@@ -674,6 +674,12 @@
 			{
 				g_object_unref (bubble);
 				bubble = sync_bubble;
+
+				bubble_set_title (sync_bubble, summary ? summary : "");
+				bubble_set_message_body (sync_bubble, body ? body : "");
+				bubble_set_value (sync_bubble, -2);
+
+				bubble_determine_layout (sync_bubble);
 			}
 
 			if (data && G_VALUE_HOLDS_STRING (data))