tdesktop
tdesktop copied to clipboard
Added a 'auto-hide' notifications setting
Hi! I added an "auto-hide"notification setting (it was requested in a now closed issue #4850).
Motivation
On Telegram Desktop, notification will not hide themselves unless the app detects any user input. This can be annoying when e.g. the user is watching a video/movie and has to move the mouse to make the notification disappear.
What I did
Simply added a "auto-hide" notifications setting, which, when enabled, will make the notifications hide themselves after a few seconds (2 at the moment). I'm attaching a screenshot below of the new setting.
Possible improvements
- The amount of seconds to make a notification disappear is hardcoded to 2 seconds at the moment. Could it be useful to let the user decide this quantity?
- I added the english string shown in the settings. Since I'm a native italian speaker, I could add that as well if you wish.
Greetings, levnikmyskin
Please, use tabs for indentations.
Thank you, I added and committed the suggested changes
I always thought that this is a bug with between player's layer and window manager interaction :open_mouth:
Integrated 23rd suggestions and merged new commits from upstream
This PR could fix the issue when notification don't disappear when watching a movie or something else in full screen (even outside from TDesktop). The notification remain on screen forever, unless I move the mouse or type something.
This PR could fix the issue when notification don't disappear when watching a movie or something else in full screen (even outside from TDesktop). The notification remain on screen forever, unless I move the mouse or type something.
From what I gathered, this has never been an "issue" (or a bug), but a dev choice. I think it'd be nice to have this setting so the "choice" is moved to the user.
It need some tests to see if it don't break anything, but overall, I tried and it does a good job. @john-preston, I heard that you eventually don't accept changes that mess with screen, but IMO, you don't need the option inside Notifications, just his method, to force close notifications after a while. But again, @levnikmyskin do more tests about notifications and you could push the PR without that option, only the method?
I think it will be a helpful feature if implemented!
Conflicting files Telegram/SourceFiles/core/core_settings.cpp
@levnikmyskin
Conflicting files
looks like preston is not going to merge this anyway
Whenever you think you want to merge this, just let me know and I'll resolve the conflicts
Whenever you think you want to merge this, just let me know and I'll resolve the conflicts
The problem with this is the option (preston doesn't like to add new options). If you'll find a way to detect when the user is watching something and apply this logic automatically then this PR will have all chances to be merged.
Whenever you think you want to merge this, just let me know and I'll resolve the conflicts
The problem with this is the option (preston doesn't like to add new options). If you'll find a way to detect when the user is watching something and apply this logic automatically then this PR will have all chances to be merged.
I think making this auto-hide notification by default can be a solution. Because who wants a popup notification always appeared on his screen!
I think making this auto-hide notification by default can be a solution.
I honestly don't know why this behavior is chosen, but I guess preston won't allow just remove this logic
I propose this:
diff --git a/Telegram/SourceFiles/platform/linux/notifications_manager_linux.cpp b/Telegram/SourceFiles/platform/linux/notifications_manager_linux.cpp
index cf5a07099..11c859d6c 100644
--- a/Telegram/SourceFiles/platform/linux/notifications_manager_linux.cpp
+++ b/Telegram/SourceFiles/platform/linux/notifications_manager_linux.cpp
@@ -727,6 +727,10 @@ bool SkipFlashBounce() {
return Inhibited();
}
+bool WaitForInput() {
+ return true;
+}
+
bool Supported() {
return ServiceRegistered;
}
diff --git a/Telegram/SourceFiles/platform/linux/notifications_manager_linux_dummy.cpp b/Telegram/SourceFiles/platform/linux/notifications_manager_linux_dummy.cpp
index c51b92fed..cec5fa737 100644
--- a/Telegram/SourceFiles/platform/linux/notifications_manager_linux_dummy.cpp
+++ b/Telegram/SourceFiles/platform/linux/notifications_manager_linux_dummy.cpp
@@ -25,6 +25,10 @@ bool SkipFlashBounce() {
return false;
}
+bool WaitForInput() {
+ return true;
+}
+
bool Supported() {
return false;
}
diff --git a/Telegram/SourceFiles/platform/mac/notifications_manager_mac.mm b/Telegram/SourceFiles/platform/mac/notifications_manager_mac.mm
index 310b2829b..1993872a3 100644
--- a/Telegram/SourceFiles/platform/mac/notifications_manager_mac.mm
+++ b/Telegram/SourceFiles/platform/mac/notifications_manager_mac.mm
@@ -159,6 +159,10 @@ bool SkipFlashBounce() {
return SkipAudio();
}
+bool WaitForInput() {
+ return true;
+}
+
bool Supported() {
return Platform::IsMac10_8OrGreater();
}
diff --git a/Telegram/SourceFiles/platform/platform_notifications_manager.h b/Telegram/SourceFiles/platform/platform_notifications_manager.h
index eb3efbddb..4731a97bb 100644
--- a/Telegram/SourceFiles/platform/platform_notifications_manager.h
+++ b/Telegram/SourceFiles/platform/platform_notifications_manager.h
@@ -15,6 +15,7 @@ namespace Notifications {
[[nodiscard]] bool SkipAudio();
[[nodiscard]] bool SkipToast();
[[nodiscard]] bool SkipFlashBounce();
+[[nodiscard]] bool WaitForInput();
[[nodiscard]] bool Supported();
[[nodiscard]] bool Enforced();
diff --git a/Telegram/SourceFiles/platform/win/notifications_manager_win.cpp b/Telegram/SourceFiles/platform/win/notifications_manager_win.cpp
index 9ea6c222b..59b92667b 100644
--- a/Telegram/SourceFiles/platform/win/notifications_manager_win.cpp
+++ b/Telegram/SourceFiles/platform/win/notifications_manager_win.cpp
@@ -730,7 +730,6 @@ bool SkipToast() {
if (UserNotificationState == QUNS_PRESENTATION_MODE
|| UserNotificationState == QUNS_RUNNING_D3D_FULL_SCREEN
- //|| UserNotificationState == QUNS_BUSY
|| QuietHoursEnabled) {
return true;
}
@@ -741,5 +740,14 @@ bool SkipFlashBounce() {
return SkipToast();
}
+bool WaitForInput() {
+ querySystemNotificationSettings();
+
+ if (UserNotificationState == QUNS_BUSY) {
+ return false;
+ }
+ return true;
+}
+
} // namespace Notifications
} // namespace Platform
diff --git a/Telegram/SourceFiles/window/notifications_manager_default.cpp b/Telegram/SourceFiles/window/notifications_manager_default.cpp
index a5ea81f96..d6e216f18 100644
--- a/Telegram/SourceFiles/window/notifications_manager_default.cpp
+++ b/Telegram/SourceFiles/window/notifications_manager_default.cpp
@@ -667,7 +667,9 @@ void Notification::prepareActionsCache() {
bool Notification::checkLastInput(bool hasReplyingNotifications) {
if (!_waitingForInput) return true;
- const auto waitForUserInput = base::Platform::LastUserInputTimeSupported()
+ const auto shouldWaitForUserInput = Platform::Notifications::WaitForInput()
+ && base::Platform::LastUserInputTimeSupported();
+ const auto waitForUserInput = shouldWaitForUserInput
? (Core::App().lastNonIdleTime() <= _started)
: false;
if (!waitForUserInput) {
This should hide notifications automatically if something is running fullscreen (like video player). Implemented only for Windows, other platforms need implementations.
I propose this:
diff --git a/Telegram/SourceFiles/platform/linux/notifications_manager_linux.cpp b/Telegram/SourceFiles/platform/linux/notifications_manager_linux.cpp index cf5a07099..11c859d6c 100644 --- a/Telegram/SourceFiles/platform/linux/notifications_manager_linux.cpp +++ b/Telegram/SourceFiles/platform/linux/notifications_manager_linux.cpp @@ -727,6 +727,10 @@ bool SkipFlashBounce() { return Inhibited(); } +bool WaitForInput() { + return true; +} + bool Supported() { return ServiceRegistered; } diff --git a/Telegram/SourceFiles/platform/linux/notifications_manager_linux_dummy.cpp b/Telegram/SourceFiles/platform/linux/notifications_manager_linux_dummy.cpp index c51b92fed..cec5fa737 100644 --- a/Telegram/SourceFiles/platform/linux/notifications_manager_linux_dummy.cpp +++ b/Telegram/SourceFiles/platform/linux/notifications_manager_linux_dummy.cpp @@ -25,6 +25,10 @@ bool SkipFlashBounce() { return false; } +bool WaitForInput() { + return true; +} + bool Supported() { return false; } diff --git a/Telegram/SourceFiles/platform/mac/notifications_manager_mac.mm b/Telegram/SourceFiles/platform/mac/notifications_manager_mac.mm index 310b2829b..1993872a3 100644 --- a/Telegram/SourceFiles/platform/mac/notifications_manager_mac.mm +++ b/Telegram/SourceFiles/platform/mac/notifications_manager_mac.mm @@ -159,6 +159,10 @@ bool SkipFlashBounce() { return SkipAudio(); } +bool WaitForInput() { + return true; +} + bool Supported() { return Platform::IsMac10_8OrGreater(); } diff --git a/Telegram/SourceFiles/platform/platform_notifications_manager.h b/Telegram/SourceFiles/platform/platform_notifications_manager.h index eb3efbddb..4731a97bb 100644 --- a/Telegram/SourceFiles/platform/platform_notifications_manager.h +++ b/Telegram/SourceFiles/platform/platform_notifications_manager.h @@ -15,6 +15,7 @@ namespace Notifications { [[nodiscard]] bool SkipAudio(); [[nodiscard]] bool SkipToast(); [[nodiscard]] bool SkipFlashBounce(); +[[nodiscard]] bool WaitForInput(); [[nodiscard]] bool Supported(); [[nodiscard]] bool Enforced(); diff --git a/Telegram/SourceFiles/platform/win/notifications_manager_win.cpp b/Telegram/SourceFiles/platform/win/notifications_manager_win.cpp index 9ea6c222b..59b92667b 100644 --- a/Telegram/SourceFiles/platform/win/notifications_manager_win.cpp +++ b/Telegram/SourceFiles/platform/win/notifications_manager_win.cpp @@ -730,7 +730,6 @@ bool SkipToast() { if (UserNotificationState == QUNS_PRESENTATION_MODE || UserNotificationState == QUNS_RUNNING_D3D_FULL_SCREEN - //|| UserNotificationState == QUNS_BUSY || QuietHoursEnabled) { return true; } @@ -741,5 +740,14 @@ bool SkipFlashBounce() { return SkipToast(); } +bool WaitForInput() { + querySystemNotificationSettings(); + + if (UserNotificationState == QUNS_BUSY) { + return false; + } + return true; +} + } // namespace Notifications } // namespace Platform diff --git a/Telegram/SourceFiles/window/notifications_manager_default.cpp b/Telegram/SourceFiles/window/notifications_manager_default.cpp index a5ea81f96..d6e216f18 100644 --- a/Telegram/SourceFiles/window/notifications_manager_default.cpp +++ b/Telegram/SourceFiles/window/notifications_manager_default.cpp @@ -667,7 +667,9 @@ void Notification::prepareActionsCache() { bool Notification::checkLastInput(bool hasReplyingNotifications) { if (!_waitingForInput) return true; - const auto waitForUserInput = base::Platform::LastUserInputTimeSupported() + const auto shouldWaitForUserInput = Platform::Notifications::WaitForInput() + && base::Platform::LastUserInputTimeSupported(); + const auto waitForUserInput = shouldWaitForUserInput ? (Core::App().lastNonIdleTime() <= _started) : false; if (!waitForUserInput) {
This should hide notifications automatically if something is running fullscreen (like video player). Implemented only for Windows, other platforms need implementations.
On bool WaitForInput()
function will it be UserNotificationState == QUNS_BUSY
or UserNotificationState == QUNS_RUNNING_D3D_FULL_SCREEN
?
On
bool WaitForInput()
function will it beUserNotificationState == QUNS_BUSY
orUserNotificationState == QUNS_RUNNING_D3D_FULL_SCREEN
?
QUNS_BUSY. QUNS_RUNNING_D3D_FULL_SCREEN is checked in SkipToast, so it shouldn't show the toast at all. QUNS_RUNNING_D3D_FULL_SCREEN should prevent showing notifications when some game is running, but it doesn't work with all games for some reason (#2290)
On
bool WaitForInput()
function will it beUserNotificationState == QUNS_BUSY
orUserNotificationState == QUNS_RUNNING_D3D_FULL_SCREEN
?QUNS_BUSY. QUNS_RUNNING_D3D_FULL_SCREEN is checked in SkipToast, so it shouldn't show the toast at all. QUNS_RUNNING_D3D_FULL_SCREEN should prevent showing notifications when some game is running, but it doesn't work with all games for some reason (#2290)
Yeah, I see. But the function will be more reasonable with the name "SkipNotification()" or something to help debug the codes later.
But the function will be more reasonable with the name "SkipNotification()"
- What function?
- It's already in Notifications namespace.
The idea of current beahaviour is to keep notifications on screen while the user is away from his computer. When he returns he sees the notifications. I'm not against removing notifications completely if the user is busy at the computer without input events. I think if user watches a movie the notifications should not appear, maybe only sound can be played.
@john-preston won't people from #473 complain again?
The idea of current beahaviour is to keep notifications on screen while the user is away from his computer. When he returns he sees the notifications. I'm not against removing notifications completely if the user is busy at the computer without input events. I think if user watches a movie the notifications should not appear, maybe only sound can be played.
ilya-fedin's code is to solve the issue to detect if users are playing any video in fullscreen.
@john-preston won't people from #473 complain again?
Looks like there're still some folks who want notification during watching a movie or something?
Looks like there're still some folks who want notification during watching a movie or something?
Probably
Looks like there're still some folks who want notification during watching a movie or something?
Probably
Let that be an option though, some of us do want that notification to go away, think the notification count number should be enough for when "the user comes back and realizes he has new messages".
Personally I quit the app when it happens to me because it's really annoying.
The option to control that behavior should be enough, with a secondary option to execute that method only when there's a video playing in full screen or something would be ideal
Any update on this?
It seems @john-preston doesn't want this
Exactly as @fguille94 said. I really need this feature. It's so annoying to use the app without it @john-preston Why is it still not there?
to this day I still rather close the whole application just to not have the permanent-when-idle notification staring at my AFK face.
we seriously need this feature implemented ASAP