tdesktop icon indicating copy to clipboard operation
tdesktop copied to clipboard

Added a 'auto-hide' notifications setting

Open levnikmyskin opened this issue 4 years ago • 44 comments

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

image

levnikmyskin avatar Dec 31 '20 11:12 levnikmyskin

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Dec 31 '20 11:12 CLAassistant

Please, use tabs for indentations.

23rd avatar Dec 31 '20 11:12 23rd

Thank you, I added and committed the suggested changes

levnikmyskin avatar Dec 31 '20 11:12 levnikmyskin

I always thought that this is a bug with between player's layer and window manager interaction :open_mouth:

ilya-fedin avatar Dec 31 '20 19:12 ilya-fedin

Integrated 23rd suggestions and merged new commits from upstream

levnikmyskin avatar Jan 24 '21 11:01 levnikmyskin

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.

ThigSchuch avatar Jan 24 '21 17:01 ThigSchuch

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.

levnikmyskin avatar Jan 24 '21 22:01 levnikmyskin

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?

ThigSchuch avatar Feb 14 '21 21:02 ThigSchuch

I think it will be a helpful feature if implemented!

Nirzak avatar Apr 05 '21 11:04 Nirzak

Conflicting files Telegram/SourceFiles/core/core_settings.cpp

@levnikmyskin

Aokromes avatar Apr 05 '21 11:04 Aokromes

Conflicting files

looks like preston is not going to merge this anyway

ilya-fedin avatar Apr 05 '21 18:04 ilya-fedin

Whenever you think you want to merge this, just let me know and I'll resolve the conflicts

levnikmyskin avatar Apr 06 '21 08:04 levnikmyskin

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.

ilya-fedin avatar Apr 07 '21 12:04 ilya-fedin

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!

Nirzak avatar Apr 07 '21 13:04 Nirzak

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

ilya-fedin avatar Apr 07 '21 13:04 ilya-fedin

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.

ilya-fedin avatar Apr 07 '21 13:04 ilya-fedin

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?

Nirzak avatar Apr 07 '21 14:04 Nirzak

On bool WaitForInput() function will it be UserNotificationState == QUNS_BUSY or UserNotificationState == 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)

ilya-fedin avatar Apr 07 '21 14:04 ilya-fedin

On bool WaitForInput() function will it be UserNotificationState == QUNS_BUSY or UserNotificationState == 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.

Nirzak avatar Apr 07 '21 14:04 Nirzak

But the function will be more reasonable with the name "SkipNotification()"

  1. What function?
  2. It's already in Notifications namespace.

ilya-fedin avatar Apr 07 '21 14:04 ilya-fedin

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 avatar Apr 07 '21 16:04 john-preston

@john-preston won't people from #473 complain again?

ilya-fedin avatar Apr 07 '21 16:04 ilya-fedin

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.

Nirzak avatar Apr 07 '21 16:04 Nirzak

@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?

Nirzak avatar Apr 07 '21 16:04 Nirzak

Looks like there're still some folks who want notification during watching a movie or something?

Probably

ilya-fedin avatar Apr 07 '21 16:04 ilya-fedin

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

fguille94 avatar Dec 20 '21 02:12 fguille94

Any update on this?

nicofirst1 avatar Mar 24 '22 09:03 nicofirst1

It seems @john-preston doesn't want this

ilya-fedin avatar Mar 24 '22 10:03 ilya-fedin

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?

EPecherkin avatar Jul 15 '22 15:07 EPecherkin

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

fguille94 avatar Jul 15 '22 18:07 fguille94