Bug 388087 - Notifications disappear too quickly
Summary: Notifications disappear too quickly
Status: RESOLVED FIXED
Alias: None
Product: Discover
Classification: Applications
Component: discover (show other bugs)
Version: 5.11.4
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Aleix Pol
URL:
Keywords:
: 389457 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-12-20 18:30 UTC by Nate Graham
Modified: 2018-02-07 19:50 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.12.1
Sentry Crash Report:


Attachments
Two passiveNotifications on app startup that quickly disappear (976.87 KB, video/webm)
2017-12-20 23:42 UTC, Nate Graham
Details
One passiveNotification after canceling a password prompt (1.10 MB, video/webm)
2017-12-20 23:43 UTC, Nate Graham
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nate Graham 2017-12-20 18:30:57 UTC
DiscoverMainWindow::showPassiveNotification() uses a very fast 100ms timer for passiveNotifications. This is so fast that many notifications vanish before you've read them. I'm just speaking for myself here; I imagine it's much worse for older folks without eyes as fast as us youngsters and who aren't as accustomed to immediately refocusing on something that just popped up.

Kirigami's showPassiveNotification() already has an timeout specifier so there's no need to wrap it in a QTimer in Discover. We could just use "long" as the string for showPassiveNotification()'s "timeout" argument. Notably, Kirigami's "short" duration is 1000ms--10 times longer than the one Discover uses. "long" is 4500ms, which is plenty long.

Ideally these messages would also have a close button (perhaps via Bug 388086) to make it obvious that you can dismiss the notifications after you've read them so they don't hang around forever. You can already click them to dismiss, but that's not at all obvious.
Comment 1 Nate Graham 2017-12-20 19:06:13 UTC
Something seems not quite right, since browsing the code, kirigami's showPassiveNotification() function is supposed to use a timeout of 4500ms by default if no timeout value is specified, and Discover invokes the method without a timeout argument:

QMetaObject::invokeMethod(rootObject(), "showPassiveNotification", Qt::QueuedConnection, Q_ARG(QVariant, msg), Q_ARG(QVariant, {}), Q_ARG(QVariant, {}), Q_ARG(QVariant, {}));


I tried removing the timer wrapping it:


diff --git a/discover/DiscoverMainWindow.cpp b/discover/DiscoverMainWindow.cpp
index 247268f..83436cd 100644
--- a/discover/DiscoverMainWindow.cpp
+++ b/discover/DiscoverMainWindow.cpp
@@ -401,7 +401,5 @@ QWindow* DiscoverMainWindow::rootObject() const
 
 void DiscoverMainWindow::showPassiveNotification(const QString& msg)
 {
-    QTimer::singleShot(100, this, [this, msg](){
-        QMetaObject::invokeMethod(rootObject(), "showPassiveNotification", Qt::QueuedConnection, Q_ARG(QVariant, msg), Q_ARG(QVariant, {}), Q_ARG(QVariant, {}), Q_ARG(QVariant, {}));
-    });
+    QMetaObject::invokeMethod(rootObject(), "showPassiveNotification", Qt::QueuedConnection, Q_ARG(QVariant, msg), Q_ARG(QVariant, {}), Q_ARG(QVariant, {}), Q_ARG(QVariant, {}));
 }



Then I tried manually specifying a "long" timeout:

diff --git a/discover/DiscoverMainWindow.cpp b/discover/DiscoverMainWindow.cpp
index 247268f..8c46aa2 100644
--- a/discover/DiscoverMainWindow.cpp
+++ b/discover/DiscoverMainWindow.cpp
@@ -401,7 +401,5 @@ QWindow* DiscoverMainWindow::rootObject() const
 
 void DiscoverMainWindow::showPassiveNotification(const QString& msg)
 {
-    QTimer::singleShot(100, this, [this, msg](){
-        QMetaObject::invokeMethod(rootObject(), "showPassiveNotification", Qt::QueuedConnection, Q_ARG(QVariant, msg), Q_ARG(QVariant, {}), Q_ARG(QVariant, {}), Q_ARG(QVariant, {}));
-    });
+    QMetaObject::invokeMethod(rootObject(), "showPassiveNotification", Qt::QueuedConnection, Q_ARG(QVariant, msg), Q_ARG(QVariant, QStringLiteral("long")), Q_ARG(QVariant, {}), Q_ARG(QVariant, {}));
 }



Both of them just made the notifications disappear almost instantly; it seems like the timeout was always getting set to 0 or something.
Comment 2 Nate Graham 2017-12-20 20:18:33 UTC
It looks like this is an issue in Kirigami not using the correct default timeout or honoring the one we pass it, or in Discover for somehow not calling it correctly. I can get all the other arguments to work properly though.
Comment 3 Nate Graham 2017-12-20 23:42:15 UTC
Created attachment 109463 [details]
Two passiveNotifications on app startup that quickly disappear
Comment 4 Nate Graham 2017-12-20 23:43:25 UTC
Created attachment 109464 [details]
One passiveNotification after canceling a password prompt
Comment 5 Aleix Pol 2017-12-22 02:05:02 UTC

*** This bug has been marked as a duplicate of bug 388099 ***
Comment 6 Nate Graham 2017-12-22 03:37:10 UTC
Are you sure this is the same issue? Even notifications that don't get squashed by others don't seem to last very long and can easily be missed.
Comment 7 Nate Graham 2018-01-26 19:56:34 UTC
*** Bug 389457 has been marked as a duplicate of this bug. ***
Comment 8 Nate Graham 2018-01-26 19:56:49 UTC
Not the same issue. Even when there aren't two consecutive notifications, they can still disappear before you've had a chance to read them. We should probably require the user to acknowledge and close them (and then make a big push to only show notifications that are clear and actionable).
Comment 9 Aleix Pol 2018-02-06 12:09:45 UTC
Git commit 4aebbb7041e5665ca2d5bc8e30e8b38de2643de7 by Aleix Pol.
Committed on 06/02/2018 at 12:08.
Pushed by apol into branch 'Plasma/5.12'.

Use the default passive notification amount

In practice it's 4500ms which is 33% longer than the previous 3000ms.

M  +2    -2    discover/qml/DiscoverWindow.qml

https://commits.kde.org/discover/4aebbb7041e5665ca2d5bc8e30e8b38de2643de7