Merge lp:~aacid/unity-notifications/placeholderbeginning into lp:unity-notifications

[Albert Astals Cid]

Add a placeholder at the beginning of the notification list

This will, in the futuer, be controlled by a boolean confirmationPlaceholder property, but for now we'll just hide it unity8-side. It helps us with a focus issue in there.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:183
http://jenkins.qa.ubuntu.com/job/unity-notifications-ci/23/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-notifications-saucy-amd64-ci/18/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-notifications-saucy-armhf-ci/23/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-notifications-saucy-i386-ci/21/console

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity-notifications-ci/23/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:184
http://jenkins.qa.ubuntu.com/job/unity-notifications-ci/24/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-notifications-saucy-amd64-ci/19
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-notifications-saucy-armhf-ci/24
        deb: http://jenkins.qa.ubuntu.com/job/unity-notifications-saucy-armhf-ci/24/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-notifications-saucy-i386-ci/22

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity-notifications-ci/24/rebuild

review: Approve (continuous-integration)

[Michał Sawicz (saviq)]

Yes, this is working.

review: Approve

[Michał Sawicz (saviq)]

This breaks the append_hint usecase :/

review: Needs Fixing

[lp:~aacid/unity-notifications/placeholderbeginning]

184. By Albert Astals Cid on 2013-10-16

emit the correct dataChanged

[Michał Sawicz (saviq)]

Yes!

review: Approve

=== modified file 'include/Notification.h'
--- include/Notification.h	2013-10-14 11:51:41 +0000
+++ include/Notification.h	2013-10-16 13:51:11 +0000
@@ -52,7 +52,7 @@
 
 public:
     enum Urgency { Low, Normal, Critical };
-    enum Type { Confirmation, Ephemeral, Interactive, SnapDecision, PlaceHolder };
+    enum Type { PlaceHolder, Confirmation, Ephemeral, Interactive, SnapDecision };
 
 Q_SIGNALS:
     void bodyChanged(QString text);

=== modified file 'src/NotificationModel.cpp'
--- src/NotificationModel.cpp	2013-10-14 11:51:41 +0000
+++ src/NotificationModel.cpp	2013-10-16 13:51:11 +0000
@@ -40,6 +40,7 @@
 }
 
 NotificationModel::NotificationModel(QObject *parent) : QAbstractListModel(parent), p(new NotificationModelPrivate) {
+    p->displayedNotifications.append(QSharedPointer<Notification>(new Notification(0, -1, Notification::Normal, QString(), Notification::PlaceHolder)));
     connect(&(p->timer), SIGNAL(timeout()), this, SLOT(timeout()));
     p->timer.setSingleShot(true);
 }
@@ -60,7 +61,7 @@
 }
 
 int NotificationModel::rowCount(const QModelIndex &parent) const {
-    //printf("Count %d\n", displayedNotifications.size());
+    //printf("Count %d\n", p->displayedNotifications.size());
     return p->displayedNotifications.size();
 }
 
@@ -294,7 +295,8 @@
     for(int i=p->displayedNotifications.size()-1; i>=0; i--) {
         QSharedPointer<Notification> n = p->displayedNotifications[i];
         int shownTime = p->displayTimes[n->getID()];
-        if(shownTime >= n->getDisplayTime()) {
+        int totalTime = n->getDisplayTime();
+        if(totalTime >= 0 && shownTime >= totalTime) {
             deleteFromVisible(i);
         }
     }
@@ -323,12 +325,14 @@
     for(int i=0; i<p->displayedNotifications.size(); i++) {
         QSharedPointer<Notification> n = p->displayedNotifications[i];
         int totalTime = n->getDisplayTime();
-        int shownTime = p->displayTimes[n->getID()];
-        int remainingTime = totalTime - shownTime;
-        if(remainingTime < 0)
-            remainingTime = 0;
-        if(remainingTime < mintime)
-            mintime = remainingTime;
+        if (totalTime >= 0) {
+            int shownTime = p->displayTimes[n->getID()];
+            int remainingTime = totalTime - shownTime;
+            if(remainingTime < 0)
+                remainingTime = 0;
+            if(remainingTime < mintime)
+                mintime = remainingTime;
+        }
     }
     return mintime;
 }
@@ -422,7 +426,6 @@
 }
 
 int NotificationModel::insertionPoint(const QSharedPointer<Notification> n) const {
-    int i=0;
     if(n->getType() == Notification::Type::SnapDecision) {
         int loc = findFirst(Notification::Type::SnapDecision);
         int numSnaps = countShowing(Notification::Type::SnapDecision);
@@ -433,6 +436,7 @@
         }
         return loc+numSnaps;
     } else {
+        int i=0;
         for(; i<p->displayedNotifications.size(); i++) {
             if(p->displayedNotifications[i]->getType() > n->getType()) {
                 break;
@@ -452,7 +456,7 @@
     //QModelIndex insertionPoint = QAbstractItemModel::createIndex(location, 0);
     //beginInsertRows(insertionPoint, location, location);
     QModelIndex insertionPoint = QModelIndex();
-    beginInsertRows(insertionPoint, rowCount(insertionPoint), rowCount(insertionPoint));
+    beginInsertRows(insertionPoint, location, location);
     p->displayedNotifications.insert(location, n);
     endInsertRows();
     p->displayTimes[n->getID()] = 0;
@@ -546,8 +550,10 @@
 }
 
 void NotificationModel::onDataChanged(unsigned int id) {
-    // FIXME: not really the right way to do it by assuming
-    // it's always the first notification being displayed
-    // that's affected by a potential data-change
-    Q_EMIT dataChanged(index(0, 0), index(0, 0));
+    for(int i=0; i<p->displayedNotifications.size(); i++) {
+        if(p->displayedNotifications[i]->getID() == id) {
+            Q_EMIT dataChanged(index(i, 0), index(i, 0));
+            break;
+        }
+    }
 }

=== modified file 'test/notificationtest.cpp'
--- test/notificationtest.cpp	2013-06-18 08:36:08 +0000
+++ test/notificationtest.cpp	2013-10-16 13:51:11 +0000
@@ -10,11 +10,11 @@
     QSharedPointer<Notification> n(new Notification(42, timeout, Notification::Low, "this is text"));
     NotificationModel m;
 
-    assert(m.numNotifications() == 0);
+    assert(m.numNotifications() == 1);
     m.insertNotification(n);
-    assert(m.numNotifications() == 1);
+    assert(m.numNotifications() == 2);
     m.removeNotification(n->getID());
-    assert(m.numNotifications() == 0);
+    assert(m.numNotifications() == 1);
 }
 
 void testTypeSimple() {