unity-notifications

Merge lp:~lukas-kde/unity-notifications/visibleQueueFixes into lp:unity-notifications

  1. visibleQueueFixes
  2. Merge into trunk
Proposed by Lukáš Tinkl
Status: Merged
Approved by: Michael Zanetti
Approved revision: 245
Merged at revision: 238
Proposed branch: lp:~lukas-kde/unity-notifications/visibleQueueFixes
Merge into: lp:unity-notifications
Diff against target: 685 lines (+96/-168)
11 files modified
include/ActionModel.h (+3/-3)
include/Notification.h (+0/-5)
include/NotificationModel.h (+0/-1)
include/NotificationServer.h (+3/-4)
include/notify-backend.h.in (+0/-25)
src/ActionModel.cpp (+11/-11)
src/CMakeLists.txt (+0/-1)
src/Notification.cpp (+9/-15)
src/NotificationModel.cpp (+19/-61)
src/NotificationServer.cpp (+8/-13)
test/notificationtest.cpp (+43/-29)
To merge this branch: bzr merge lp:~lukas-kde/unity-notifications/visibleQueueFixes
Reviewer Review Type Date Requested Status
Michael Zanetti (community) Approve
Unity8 CI Bot continuous-integration Approve
Unity API Team Pending
Review via email: mp+301304@code.launchpad.net

Commit message

Visible queue improvements

Description of the change

Don't block the visible queue by snap decisions, allow for showing everything, sorted by type and urgency

Also do some cleanup and get rid of the Placeholder notification

Based on design request

To post a comment you must log in.
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:243
https://unity8-jenkins.ubuntu.com/job/lp-unity-notifications-ci/2/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/2391
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2419
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2306
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2306
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2306
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2299
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2299/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2299
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2299/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2299
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2299/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2299
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2299/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2299
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2299/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2299
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2299/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2299
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2299/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2299
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2299/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2299
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2299/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-notifications-ci/2/rebuild

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

Hmm, the code reads ok to me. Nice cleanup!

However, visually it looks a bit weird. Not sure if that's because of the removal of the placeholder notification but I'm afraid this requires some visual tweaks in unity8 too:

* http://i.imgur.com/MzHWx0N.png
  Spacing between notifications missing

* http://i.imgur.com/FdP0SYJ.png
  Having a SnapDecision and a Confirmation, it seems the incoming telegram message is put into the wrong place in the queue. I would think the SnapDecision should stay expanded and the telegram notification be the last one

* http://i.imgur.com/ZB7S9Ju.png
  Also without SnapDecision, the spacing seems broken now (probably really because of the removal of the placeholder)

review: Needs Information
lp:~lukas-kde/unity-notifications/visibleQueueFixes updated
244. By Lukáš Tinkl

respect the order

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:244
https://unity8-jenkins.ubuntu.com/job/lp-unity-notifications-ci/3/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/2423
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2451
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2338
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2338
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2338
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2331
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2331/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2331
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2331/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2331
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2331/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2331
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2331/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2331
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2331/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2331
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2331/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2331
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2331/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2331
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2331/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2331
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2331/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-notifications-ci/3/rebuild

review: Approve (continuous-integration)
lp:~lukas-kde/unity-notifications/visibleQueueFixes updated
245. By Lukáš Tinkl

append the notification, not prepend

side effect of the placeholder :S

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:245
https://unity8-jenkins.ubuntu.com/job/lp-unity-notifications-ci/4/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/2425
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2453
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2340
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2340
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2340
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2333
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2333/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2333
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2333/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2333
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2333/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2333
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2333/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2333
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2333/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2333
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2333/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2333
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2333/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2333
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2333/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2333
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2333/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity-notifications-ci/4/rebuild

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

Mostly good now. Regular notifications are now displayed all at once too atm. Need to check with design if that's ok or not.

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

Ok. Got confirmation from design that this is ok.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/ActionModel.h'
2--- include/ActionModel.h 2015-10-13 14:11:24 +0000
3+++ include/ActionModel.h 2016-08-01 14:19:24 +0000
4@@ -29,9 +29,9 @@
5 ActionModel(QObject *parent=nullptr);
6 virtual ~ActionModel();
7
8- virtual int rowCount(const QModelIndex &index) const override;
9- virtual QVariant data(const QModelIndex &index, int role) const override;
10- virtual QHash<int, QByteArray> roleNames() const override;
11+ int rowCount(const QModelIndex &index) const override;
12+ QVariant data(const QModelIndex &index, int role) const override;
13+ QHash<int, QByteArray> roleNames() const override;
14
15 enum ActionsRoles {
16 RoleActionLabel = Qt::UserRole + 1,
17
18=== modified file 'include/Notification.h'
19--- include/Notification.h 2015-10-12 15:21:27 +0000
20+++ include/Notification.h 2016-08-01 14:19:24 +0000
21@@ -26,7 +26,6 @@
22 #include <QString>
23 #include <QObject>
24 #include <QScopedPointer>
25-#include <QImage>
26 #include <QStringList>
27
28 struct NotificationPrivate;
29@@ -70,10 +69,6 @@
30 void dismissed();
31 void completed(unsigned int id);
32
33-public Q_SLOTS:
34- void onHovered();
35- void onDisplayed();
36-
37 public:
38 Notification(QObject *parent=nullptr);
39 Notification(NotificationID id,
40
41=== modified file 'include/NotificationModel.h'
42--- include/NotificationModel.h 2015-10-12 15:21:27 +0000
43+++ include/NotificationModel.h 2016-08-01 14:19:24 +0000
44@@ -79,7 +79,6 @@
45 int nextTimeout() const;
46 void incrementDisplayTimes(const int displayedTime) const;
47 void pruneExpired();
48- void removeNonSnap();
49
50 int insertionPoint(const QSharedPointer<Notification> &n) const;
51 void insertEphemeral(const QSharedPointer<Notification> &n);
52
53=== modified file 'include/NotificationServer.h'
54--- include/NotificationServer.h 2015-07-06 18:13:16 +0000
55+++ include/NotificationServer.h 2016-08-01 14:19:24 +0000
56@@ -55,7 +55,7 @@
57
58 Q_OBJECT
59
60- friend NotificationsAdaptor;
61+ friend class NotificationsAdaptor;
62
63 public:
64 NotificationServer(const QDBusConnection& connection, NotificationModel &m, QObject *parent=nullptr);
65@@ -82,15 +82,14 @@
66
67 private:
68 void incrementCounter();
69- QString messageSender();
70- bool isCmdLine();
71+ QString messageSender() const;
72+ bool isCmdLine() const;
73
74 QSharedPointer<Notification> buildNotification(NotificationID id, const QVariantMap &hints);
75 NotificationModel &model;
76 unsigned int idCounter;
77 QDBusConnection m_connection;
78 QDBusServiceWatcher m_watcher;
79-
80 };
81
82 #endif
83
84=== modified file 'include/notify-backend.h.in'
85--- include/notify-backend.h.in 2014-11-04 13:03:18 +0000
86+++ include/notify-backend.h.in 2016-08-01 14:19:24 +0000
87@@ -22,38 +22,13 @@
88 #ifndef NOTIFY_BACKEND_
89 #define NOTIFY_BACKEND_
90
91-#include <cstdlib>
92-
93 typedef unsigned int NotificationID;
94
95-/*enum Urgency {
96- URGENCY_LOW,
97- URGENCY_NORMAL,
98- URGENCY_CRITICAL
99-};
100-
101-enum NotificationType {
102- SYNCHRONOUS,
103- SNAP,
104- INTERACTIVE,
105- ASYNCHRONOUS
106-};*/
107-
108 const unsigned int MAX_NOTIFICATIONS = 50;
109
110-class Renderer;
111-class NotificationBackend;
112-class Notification;
113-
114-#if 0
115-#define DBUS_SERVICE_NAME "com.canonical.notificationproto"
116-#define DBUS_INTERFACE "com.canonical.notificationproto"
117-#define DBUS_PATH "/com/canonical/notificationproto"
118-#else
119 #define DBUS_SERVICE_NAME "org.freedesktop.Notifications"
120 #define DBUS_INTERFACE "org.freedesktop.Notifications"
121 #define DBUS_PATH "/org/freedesktop/Notifications"
122-#endif
123
124 #define VALUE_HINT "value"
125 #define VALUE_TINT_HINT "x-canonical-value-bar-tint"
126
127=== modified file 'src/ActionModel.cpp'
128--- src/ActionModel.cpp 2015-06-16 08:11:33 +0000
129+++ src/ActionModel.cpp 2016-08-01 14:19:24 +0000
130@@ -17,8 +17,8 @@
131 #include "ActionModel.h"
132
133 struct ActionModelPrivate {
134- QList<QString> labels;
135- QList<QString> ids;
136+ QStringList labels;
137+ QStringList ids;
138 };
139
140 ActionModel::ActionModel(QObject *parent) : QStringListModel(parent), p(new ActionModelPrivate) {
141@@ -33,17 +33,17 @@
142
143 QVariant ActionModel::data(const QModelIndex &index, int role) const {
144 if (!index.isValid())
145- return QVariant();
146+ return QVariant();
147
148 switch(role) {
149- case RoleActionLabel:
150- return QVariant(p->labels[index.row()]);
151-
152- case RoleActionId:
153- return QVariant(p->ids[index.row()]);
154-
155- default:
156- return QVariant();
157+ case RoleActionLabel:
158+ return p->labels.at(index.row());
159+
160+ case RoleActionId:
161+ return p->ids.at(index.row());
162+
163+ default:
164+ return QVariant();
165 }
166 }
167
168
169=== modified file 'src/CMakeLists.txt'
170--- src/CMakeLists.txt 2015-06-23 09:11:39 +0000
171+++ src/CMakeLists.txt 2016-08-01 14:19:24 +0000
172@@ -1,4 +1,3 @@
173-include(FindPkgConfig)
174 pkg_check_modules(NOTIFICATIONS_API REQUIRED unity-shell-notifications=3)
175
176 set(CORE_SRCS
177
178=== modified file 'src/Notification.cpp'
179--- src/Notification.cpp 2015-10-12 15:04:29 +0000
180+++ src/Notification.cpp 2016-08-01 14:19:24 +0000
181@@ -1,5 +1,5 @@
182 /*
183- * Copyright (C) 2013 Canonical, Ltd.
184+ * Copyright (C) 2013-2016 Canonical, Ltd.
185 *
186 * Authors:
187 * Jussi Pakkanen <jussi.pakkanen@canonical.com>
188@@ -20,7 +20,7 @@
189
190 #include "NotificationServer.h"
191 #include "Notification.h"
192-#include <string>
193+
194 #include <QXmlStreamReader>
195
196 using namespace std;
197@@ -74,8 +74,9 @@
198 }
199
200 Notification::~Notification() {
201- if(p->server)
202+ if (p->server) {
203 p->server->forceCloseNotification(p->id);
204+ }
205 }
206
207 QString Notification::getBody() const {
208@@ -116,8 +117,8 @@
209 }
210
211 void Notification::setIcon(const QString &icon) {
212- if (icon.startsWith(" ") || icon.size() == 0) {
213- p->icon = nullptr;
214+ if (icon.startsWith(" ") || icon.isEmpty()) {
215+ p->icon.clear();
216 }
217 else {
218 p->icon = icon;
219@@ -136,8 +137,8 @@
220 }
221
222 void Notification::setSecondaryIcon(const QString &secondaryIcon) {
223- if (secondaryIcon.startsWith(" ") || secondaryIcon.size() == 0) {
224- p->secondaryIcon = nullptr;
225+ if (secondaryIcon.startsWith(" ") || secondaryIcon.isEmpty()) {
226+ p->secondaryIcon.clear();
227 }
228 else {
229 p->secondaryIcon = secondaryIcon;
230@@ -177,6 +178,7 @@
231 Notification::Urgency Notification::getUrgency() const {
232 return p->urg;
233 }
234+
235 void Notification::setUrgency(Notification::Urgency urg) {
236 if(p->urg != urg) {
237 p->urg = urg;
238@@ -238,14 +240,6 @@
239 }
240 }
241
242-void Notification::onHovered() {
243-
244-}
245-
246-void Notification::onDisplayed() {
247-
248-}
249-
250 void Notification::invokeAction(const QString &action) {
251 for(int i=0; i<p->actions.size(); i++) {
252 if(p->actions[i] == action) {
253
254=== modified file 'src/NotificationModel.cpp'
255--- src/NotificationModel.cpp 2015-10-13 13:49:02 +0000
256+++ src/NotificationModel.cpp 2016-08-01 14:19:24 +0000
257@@ -1,5 +1,5 @@
258 /*
259- * Copyright (C) 2013 Canonical, Ltd.
260+ * Copyright (C) 2013-2016 Canonical, Ltd.
261 *
262 * Authors:
263 * Jussi Pakkanen <jussi.pakkanen@canonical.com>
264@@ -46,8 +46,6 @@
265 }
266
267 NotificationModel::NotificationModel(QObject *parent) : QAbstractListModel(parent), p(new NotificationModelPrivate) {
268- p->displayedNotifications.append(QSharedPointer<Notification>(new Notification(0, -1, Notification::Normal, QString(), Notification::PlaceHolder),
269- &QObject::deleteLater));
270 connect(&(p->timer), SIGNAL(timeout()), this, SLOT(timeout()));
271 p->timer.setSingleShot(true);
272 }
273@@ -178,8 +176,7 @@
274 }
275 }
276
277- QSharedPointer<Notification> empty;
278- return empty;
279+ return QSharedPointer<Notification>();
280 }
281
282 QSharedPointer<Notification> NotificationModel::getNotification(const QString &summary) const {
283@@ -204,16 +201,14 @@
284 }
285 }
286
287- QSharedPointer<Notification> empty;
288- return empty;
289+ return QSharedPointer<Notification>();
290 }
291
292 QSharedPointer<Notification> NotificationModel::getDisplayedNotification(int index) const {
293 if (index < p->displayedNotifications.size()) {
294 return p->displayedNotifications[index];
295 } else {
296- QSharedPointer<Notification> empty;
297- return empty;
298+ return QSharedPointer<Notification>();
299 }
300 }
301
302@@ -318,8 +313,7 @@
303 }
304
305 QSharedPointer<Notification> NotificationModel::deleteFromVisible(int loc) {
306- QModelIndex deletePoint = QModelIndex();
307- beginRemoveRows(deletePoint, loc, loc);
308+ beginRemoveRows(QModelIndex(), loc, loc);
309 QSharedPointer<Notification> n = p->displayedNotifications[loc];
310 p->displayTimes.erase(p->displayTimes.find(n->getID()));
311 auto notification = p->displayedNotifications.takeAt(loc);
312@@ -345,8 +339,7 @@
313 // Snap decisions override everything.
314 if(showingNotificationOfType(Notification::Type::SnapDecision) || !p->snapQueue.empty()) {
315 if(countShowing(Notification::Type::SnapDecision) < maxSnapsShown && !p->snapQueue.empty()) {
316- QSharedPointer<Notification> n = p->snapQueue[0];
317- p->snapQueue.pop_front();
318+ QSharedPointer<Notification> n = p->snapQueue.takeFirst();
319 insertToVisible(n, insertionPoint(n));
320 restartTimer = true;
321 Q_EMIT queueSizeChanged(queued());
322@@ -365,15 +358,13 @@
323 bool NotificationModel::nonSnapTimeout() {
324 bool restartTimer;
325 if(!showingNotificationOfType(Notification::Type::Interactive) && !p->interactiveQueue.empty()) {
326- QSharedPointer<Notification> n = p->interactiveQueue[0];
327- p->interactiveQueue.pop_front();
328+ QSharedPointer<Notification> n = p->interactiveQueue.takeFirst();
329 insertToVisible(n, insertionPoint(n));
330 restartTimer = true;
331 Q_EMIT queueSizeChanged(queued());
332 }
333 if(!showingNotificationOfType(Notification::Type::Ephemeral) && !p->ephemeralQueue.empty()) {
334- QSharedPointer<Notification> n = p->ephemeralQueue[0];
335- p->ephemeralQueue.pop_front();
336+ QSharedPointer<Notification> n = p->ephemeralQueue.takeFirst();
337 insertToVisible(n, insertionPoint(n));
338 restartTimer = true;
339 Q_EMIT queueSizeChanged(queued());
340@@ -392,19 +383,6 @@
341 }
342 }
343
344-void NotificationModel::removeNonSnap() {
345- for(int i=p->displayedNotifications.size()-1; i>=0; i--) {
346- QSharedPointer<Notification> n = p->displayedNotifications[i];
347- switch(n->getType()) {
348- case Notification::Type::SnapDecision : break;
349- case Notification::Type::Confirmation : deleteFromVisible(i); break;
350- case Notification::Type::Ephemeral : deleteFromVisible(i); p->ephemeralQueue.push_front(n); queueSizeChanged(queued()); break;
351- case Notification::Type::Interactive : deleteFromVisible(i); p->interactiveQueue.push_front(n); queueSizeChanged(queued()); break;
352- case Notification::Type::PlaceHolder : break;
353- }
354- }
355-}
356-
357 int NotificationModel::nextTimeout() const {
358 int mintime = INT_MAX;
359 if(p->displayedNotifications.empty()) {
360@@ -430,51 +408,35 @@
361 void NotificationModel::insertEphemeral(const QSharedPointer<Notification> &n) {
362 Q_ASSERT(n->getType() == Notification::Type::Ephemeral);
363
364- if(showingNotificationOfType(Notification::Type::SnapDecision)) {
365- p->ephemeralQueue.push_back(n);
366- qStableSort(p->ephemeralQueue.begin(), p->ephemeralQueue.end(), notificationCompare);
367- Q_EMIT queueSizeChanged(queued());
368- } else if(showingNotificationOfType(Notification::Type::Ephemeral)) {
369+ if(showingNotificationOfType(Notification::Type::Ephemeral)) {
370 int loc = findFirst(Notification::Type::Ephemeral);
371 QSharedPointer<Notification> showing = p->displayedNotifications[loc];
372 if(n->getUrgency() > showing->getUrgency()) {
373- deleteFromVisible(loc);
374- insertToVisible(n, loc);
375- p->ephemeralQueue.push_front(showing);
376+ insertToVisible(n, qMax(0, loc-1));
377 } else {
378- p->ephemeralQueue.push_back(n);
379+ insertToVisible(n, loc+1);
380 }
381- qStableSort(p->ephemeralQueue.begin(), p->ephemeralQueue.end(), notificationCompare);
382- Q_EMIT queueSizeChanged(queued());
383 } else {
384- insertToVisible(n);
385+ int loc = insertionPoint(n);
386+ insertToVisible(n, loc);
387 }
388 }
389
390 void NotificationModel::insertInteractive(const QSharedPointer<Notification> &n) {
391 Q_ASSERT(n->getType() == Notification::Type::Interactive);
392
393- if(showingNotificationOfType(Notification::Type::SnapDecision)) {
394- p->interactiveQueue.push_back(n);
395- qStableSort(p->interactiveQueue.begin(), p->interactiveQueue.end(), notificationCompare);
396- Q_EMIT queueSizeChanged(queued());
397- } else if(showingNotificationOfType(Notification::Type::Interactive)) {
398+ if(showingNotificationOfType(Notification::Type::Interactive)) {
399 int loc = findFirst(Notification::Type::Interactive);
400 QSharedPointer<Notification> showing = p->displayedNotifications[loc];
401 if(n->getUrgency() > showing->getUrgency()) {
402- deleteFromVisible(loc);
403- insertToVisible(n, loc);
404- p->interactiveQueue.push_front(showing);
405+ insertToVisible(n, qMax(0, loc-1));
406 } else {
407- p->interactiveQueue.push_back(n);
408+ insertToVisible(n, loc+1);
409 }
410- qStableSort(p->interactiveQueue.begin(), p->interactiveQueue.end(), notificationCompare);
411- Q_EMIT queueSizeChanged(queued());
412 } else {
413 int loc = insertionPoint(n);
414 insertToVisible(n, loc);
415 }
416-
417 }
418
419
420@@ -489,7 +451,6 @@
421
422 void NotificationModel::insertSnap(const QSharedPointer<Notification> &n) {
423 Q_ASSERT(n->getType() == Notification::Type::SnapDecision);
424- removeNonSnap();
425 int showing = countShowing(n->getType());
426 if(showing >= maxSnapsShown) {
427 int loc = findFirst(Notification::Type::SnapDecision);
428@@ -522,7 +483,7 @@
429 }
430
431 if (!inserted) {
432- insertToVisible(n, 1);
433+ insertToVisible(n, showingNotificationOfType(Notification::Type::Confirmation) ? 1 : 0);
434 }
435 }
436 }
437@@ -541,7 +502,7 @@
438 int i=0;
439 for(; i<p->displayedNotifications.size(); i++) {
440 if(p->displayedNotifications[i]->getType() > n->getType()) {
441- break;
442+ return i+1;
443 }
444 }
445 return i;
446@@ -555,10 +516,7 @@
447 printf("Bad insert.\n");
448 return;
449 }
450- //QModelIndex insertionPoint = QAbstractItemModel::createIndex(location, 0);
451- //beginInsertRows(insertionPoint, location, location);
452- QModelIndex insertionPoint = QModelIndex();
453- beginInsertRows(insertionPoint, location, location);
454+ beginInsertRows(QModelIndex(), location, location);
455 p->displayedNotifications.insert(location, n);
456 endInsertRows();
457 p->displayTimes[n->getID()] = 0;
458
459=== modified file 'src/NotificationServer.cpp'
460--- src/NotificationServer.cpp 2015-10-13 11:15:28 +0000
461+++ src/NotificationServer.cpp 2016-08-01 14:19:24 +0000
462@@ -1,5 +1,5 @@
463 /*
464- * Copyright (C) 2013 Canonical, Ltd.
465+ * Copyright (C) 2013-2016 Canonical, Ltd.
466 *
467 * Authors:
468 * Jussi Pakkanen <jussi.pakkanen@canonical.com>
469@@ -28,7 +28,7 @@
470
471 static const char* LOCAL_OWNER = "local";
472
473-static bool isAuthorised(const QString& clientId, QSharedPointer<Notification> notification)
474+static bool isAuthorised(const QString& clientId, const QSharedPointer<Notification> &notification)
475 {
476 return (clientId == LOCAL_OWNER) || (notification->getClientId() == clientId);
477 }
478@@ -97,7 +97,7 @@
479
480 NotificationDataList NotificationServer::GetNotifications(const QString &app_name) {
481 NotificationDataList results;
482- for (auto notification: model.getAllNotifications()) {
483+ Q_FOREACH(const auto &notification, model.getAllNotifications()) {
484 NotificationData data;
485 data.appName = app_name;
486 data.id = notification->getID();
487@@ -159,7 +159,7 @@
488 }
489 }
490
491-QString NotificationServer::messageSender() {
492+QString NotificationServer::messageSender() const {
493 QString sender(LOCAL_OWNER);
494 if (calledFromDBus()) {
495 sender = message().service();
496@@ -167,7 +167,7 @@
497 return sender;
498 }
499
500-bool NotificationServer::isCmdLine() {
501+bool NotificationServer::isCmdLine() const {
502 if (!calledFromDBus()) {
503 return false;
504 }
505@@ -179,8 +179,8 @@
506
507 void NotificationServer::serviceUnregistered(const QString &clientId) {
508 m_watcher.removeWatchedService(clientId);
509- auto notifications = model.removeAllNotificationsForClient(clientId);
510- for (auto notification: notifications) {
511+ const auto notifications = model.removeAllNotificationsForClient(clientId);
512+ Q_FOREACH(const auto &notification, notifications) {
513 Q_EMIT NotificationClosed(notification->getID(), 1);
514 }
515 }
516@@ -276,12 +276,7 @@
517 notification->setBody(body);
518 notification->setIcon(app_icon);
519 notification->setSummary(summary);
520-
521- QVariantMap notifyHints;
522- for (auto iter = hints.constBegin(), end = hints.constEnd(); iter != end; ++iter) {
523- notifyHints[iter.key()] = iter.value();
524- }
525- notification->setHints(notifyHints);
526+ notification->setHints(hints);
527
528 QVariant secondaryIcon = hints[SECONDARY_ICON_HINT];
529 notification->setSecondaryIcon(secondaryIcon.toString());
530
531=== modified file 'test/notificationtest.cpp'
532--- test/notificationtest.cpp 2015-10-12 15:21:27 +0000
533+++ test/notificationtest.cpp 2016-08-01 14:19:24 +0000
534@@ -4,11 +4,6 @@
535
536 #include <QtTest/QtTest>
537
538-typedef struct {
539- const char* before;
540- const char* expected;
541-} TextComparisons;
542-
543 class TestNotifications: public QObject
544 {
545 Q_OBJECT
546@@ -33,11 +28,11 @@
547 QSharedPointer<Notification> n(new Notification(42, timeout, Notification::Low, "this is text"));
548 NotificationModel m;
549
550- QCOMPARE(m.numNotifications(), 1);
551+ QCOMPARE(m.numNotifications(), 0);
552 m.insertNotification(n);
553- QCOMPARE(m.numNotifications(), 2);
554+ QCOMPARE(m.numNotifications(), 1);
555 m.removeNotification(n->getID());
556- QCOMPARE(m.numNotifications(), 1);
557+ QCOMPARE(m.numNotifications(), 0);
558 }
559
560 void TestNotifications::testTypeSimple() {
561@@ -60,9 +55,9 @@
562 const int timeout = 1000;
563 static NotificationModel *m = new NotificationModel(this);
564 static NotificationServer *s = new NotificationServer(QDBusConnection::sessionBus(), *m);
565- QStringList actions = QStringList();
566+ QStringList actions;
567 QVariantMap hints;
568- int id[3];
569+ int id[4];
570
571 hints[URGENCY_HINT] = QVariant::fromValue(Notification::Urgency::Low);
572 id[0] = s->Notify ("test-name-low", 0, "icon-low", "summary-low", "body-low", actions, hints, timeout);
573@@ -70,22 +65,41 @@
574
575 hints[URGENCY_HINT] = QVariant::fromValue(Notification::Urgency::Normal);
576 id[1] = s->Notify ("test-name-normal", 0, "icon-normal", "summary-normal", "body-normal", actions, hints, timeout);
577- QVERIFY(!m->showingNotification(id[0]));
578+ QVERIFY(m->showingNotification(id[0]));
579 QVERIFY(m->showingNotification(id[1]));
580- QVERIFY(m->queued() == 1);
581+ QVERIFY(m->getNotification(id[1]) == m->getDisplayedNotification(0)); // verify it's ordered by urgency
582+ QVERIFY(m->queued() == 0);
583
584 hints[URGENCY_HINT] = QVariant::fromValue(Notification::Urgency::Critical);
585 id[2] = s->Notify ("test-name-critical", 0, "icon-critical", "summary-critical", "body-critical", actions, hints, timeout);
586- QVERIFY(!m->showingNotification(id[0]));
587- QVERIFY(!m->showingNotification(id[1]));
588- QVERIFY(m->showingNotification(id[2]));
589- QCOMPARE(m->queued(), 2);
590+ QVERIFY(m->showingNotification(id[0]));
591+ QVERIFY(m->showingNotification(id[1]));
592+ QVERIFY(m->showingNotification(id[2]));
593+ QVERIFY(m->getNotification(id[2]) == m->getDisplayedNotification(0)); // verify it's ordered by urgency
594+ QCOMPARE(m->queued(), 0);
595+
596+ hints[SNAP_HINT] = QStringLiteral("true");
597+ hints[URGENCY_HINT] = QVariant::fromValue(Notification::Urgency::Normal);
598+ id[3] = s->Notify ("test-name-snap", 0, "icon-snap", "summary-snap", "body-snap", {"snap", "decision", "needs", "actions"}, hints, timeout);
599+ QVERIFY(m->showingNotification(id[0]));
600+ QVERIFY(m->showingNotification(id[1]));
601+ QVERIFY(m->showingNotification(id[2]));
602+ QVERIFY(m->showingNotification(id[3]));
603+ QVERIFY(m->getNotification(id[3]) == m->getDisplayedNotification(0)); // verify it's ordered by urgency and type, snap comes first
604+ QCOMPARE(m->queued(), 0);
605+
606+ m->removeNotification(id[3]);
607+ QVERIFY(m->showingNotification(id[0]));
608+ QVERIFY(m->showingNotification(id[1]));
609+ QVERIFY(m->showingNotification(id[2]));
610+ QVERIFY(!m->showingNotification(id[3]));
611+ QCOMPARE(m->queued(), 0);
612
613 m->removeNotification(id[2]);
614- QVERIFY(!m->showingNotification(id[0]));
615+ QVERIFY(m->showingNotification(id[0]));
616 QVERIFY(m->showingNotification(id[1]));
617 QVERIFY(!m->showingNotification(id[2]));
618- QCOMPARE(m->queued(), 1);
619+ QCOMPARE(m->queued(), 0);
620
621 m->removeNotification(id[1]);
622 QVERIFY(m->showingNotification(id[0]));
623@@ -173,7 +187,7 @@
624 m.insertNotification(n3);
625 m.insertNotification(n4);
626
627- QCOMPARE(m.getDisplayedNotification(1)->getBody(), QString("snap-decision-critical"));
628+ QCOMPARE(m.getDisplayedNotification(0)->getBody(), QString("snap-decision-critical"));
629 }
630
631 void TestNotifications::testVisualSDQueueWithoutCritical() {
632@@ -190,10 +204,10 @@
633 m.insertNotification(n3);
634 m.insertNotification(n4);
635
636- QCOMPARE(m.getDisplayedNotification(4)->getBody(), QString("snap-decision-1"));
637- QCOMPARE(m.getDisplayedNotification(3)->getBody(), QString("snap-decision-2"));
638- QCOMPARE(m.getDisplayedNotification(2)->getBody(), QString("snap-decision-3"));
639- QCOMPARE(m.getDisplayedNotification(1)->getBody(), QString("snap-decision-4"));
640+ QCOMPARE(m.getDisplayedNotification(3)->getBody(), QString("snap-decision-1"));
641+ QCOMPARE(m.getDisplayedNotification(2)->getBody(), QString("snap-decision-2"));
642+ QCOMPARE(m.getDisplayedNotification(1)->getBody(), QString("snap-decision-3"));
643+ QCOMPARE(m.getDisplayedNotification(0)->getBody(), QString("snap-decision-4"));
644 }
645
646 void TestNotifications::testConfirmationValue() {
647@@ -213,8 +227,8 @@
648 m.insertNotification(ephemeral);
649 m.insertNotification(confirmation);
650
651- QCOMPARE(m.getDisplayedNotification(2)->getBody(), QString("ephemeral"));
652- QCOMPARE(m.getDisplayedNotification(1)->getBody(), QString(""));
653+ QCOMPARE(m.getDisplayedNotification(1)->getBody(), QString("ephemeral"));
654+ QCOMPARE(m.getDisplayedNotification(0)->getBody(), QString(""));
655 }
656
657 void TestNotifications::testTextFilter_data() {
658@@ -273,11 +287,11 @@
659 const int max = 20;
660 static NotificationModel *m = new NotificationModel();
661 static NotificationServer *s = new NotificationServer(QDBusConnection::sessionBus(), *m);
662- QStringList actions = QStringList();
663+ QStringList actions;
664 QVariantMap hints;
665 int before = m->numNotifications();
666
667- for (int i = 1; i <= max; i++) {
668+ for (int i = 0; i < max; i++) {
669 s->Notify ("test-app-name",
670 0,
671 "test-icon",
672@@ -288,11 +302,11 @@
673 timeout);
674 }
675
676- QCOMPARE(m->numNotifications(), max+1);
677+ QCOMPARE(m->numNotifications(), max);
678
679 for (int i = max; i >= 1; i--) {
680 m->getNotification(i)->close();
681- QCOMPARE(m->numNotifications(), i);
682+ QCOMPARE(m->numNotifications(), i-1);
683 }
684
685 QCOMPARE(m->numNotifications(), before);

Subscribers

People subscribed via source and target branches

to all changes: