From 34f05b075bd1406e961d8029b3d3eb8248dfe643 Mon Sep 17 00:00:00 2001 From: Julian Sparber Date: Mon, 12 Feb 2024 12:29:35 +0100 Subject: [PATCH] messageTray: Let the tray decide whether to show a banner Always use the same code path to add new messages to a source, and let the `MessageTray` decide whether it shows a banner. Part-of: --- js/ui/components/autorunManager.js | 2 +- js/ui/components/networkAgent.js | 2 +- js/ui/extensionSystem.js | 2 +- js/ui/main.js | 4 +-- js/ui/messageTray.js | 41 +++++++++++++----------- js/ui/notificationDaemon.js | 51 ++++++++++++++++-------------- js/ui/overview.js | 2 +- js/ui/screenshot.js | 4 +-- js/ui/shellMountOperation.js | 2 +- js/ui/status/network.js | 2 +- js/ui/status/thunderbolt.js | 2 +- js/ui/windowAttentionHandler.js | 2 +- tests/shell/basic.js | 4 +-- 13 files changed, 65 insertions(+), 55 deletions(-) diff --git a/js/ui/components/autorunManager.js b/js/ui/components/autorunManager.js index 7e53f8396..1b8f71895 100644 --- a/js/ui/components/autorunManager.js +++ b/js/ui/components/autorunManager.js @@ -203,7 +203,7 @@ class AutorunDispatcher { }); notification.connect('destroy', () => this._notifications.delete(mount)); this._notifications.set(mount, notification); - source.showNotification(notification); + source.addNotification(notification); } addMount(mount, apps, contentTypes) { diff --git a/js/ui/components/networkAgent.js b/js/ui/components/networkAgent.js index a5d603564..bab13b184 100644 --- a/js/ui/components/networkAgent.js +++ b/js/ui/components/networkAgent.js @@ -797,7 +797,7 @@ class NetworkAgent { delete this._notifications[requestId]; }); - source.showNotification(notification); + source.addNotification(notification); } _newRequest(agent, requestId, connection, settingName, hints, flags) { diff --git a/js/ui/extensionSystem.js b/js/ui/extensionSystem.js index 76ae58ff4..a74e08bac 100644 --- a/js/ui/extensionSystem.js +++ b/js/ui/extensionSystem.js @@ -354,7 +354,7 @@ export class ExtensionManager extends Signals.EventEmitter { _('Extension updates are ready to be installed.')); notification.connect('activated', () => source.open()); - source.showNotification(notification); + source.addNotification(notification); } } diff --git a/js/ui/main.js b/js/ui/main.js index cfb6d8735..a090a76ae 100644 --- a/js/ui/main.js +++ b/js/ui/main.js @@ -280,7 +280,7 @@ async function _initializeUI() { notification.addAction(_('Undo'), () => (global.context.unsafe_mode = false)); notification.setTransient(true); - source.showNotification(notification); + source.addNotification(notification); }); // Provide the bus object for gnome-session to @@ -616,7 +616,7 @@ export function notify(msg, details) { const source = MessageTray.getSystemSource(); let notification = new MessageTray.Notification(source, msg, details); notification.setTransient(true); - source.showNotification(notification); + source.addNotification(notification); } /** diff --git a/js/ui/messageTray.js b/js/ui/messageTray.js index a8732e6e0..f9c70479b 100644 --- a/js/ui/messageTray.js +++ b/js/ui/messageTray.js @@ -590,7 +590,7 @@ export const Source = GObject.registerClass({ 'destroy': {param_types: [GObject.TYPE_UINT]}, 'notification-added': {param_types: [Notification.$gtype]}, 'notification-removed': {param_types: [Notification.$gtype]}, - 'notification-show': {param_types: [Notification.$gtype]}, + 'notification-request-banner': {param_types: [Notification.$gtype]}, }, }, class Source extends MessageList.Source { constructor(params) { @@ -647,7 +647,7 @@ export const Source = GObject.registerClass({ this.destroy(); } - pushNotification(notification) { + addNotification(notification) { if (this.notifications.includes(notification)) return; @@ -655,22 +655,17 @@ export const Source = GObject.registerClass({ this.notifications.shift().destroy(NotificationDestroyedReason.EXPIRED); notification.connect('destroy', this._onNotificationDestroy.bind(this)); - notification.connect('notify::acknowledged', this.countUpdated.bind(this)); + notification.connect('notify::acknowledged', () => { + this.countUpdated(); + + // If acknowledged was set to false try to show the notification again + if (!notification.acknowledged) + this.emit('notification-request-banner', notification); + }); this.notifications.push(notification); + this.emit('notification-added', notification); - - this.countUpdated(); - } - - showNotification(notification) { - notification.acknowledged = false; - this.pushNotification(notification); - - if (notification.urgency === Urgency.LOW) - return; - - if (this.policy.showBanners || notification.urgency === Urgency.CRITICAL) - this.emit('notification-show', notification); + this.emit('notification-request-banner', notification); } destroy(reason) { @@ -876,7 +871,7 @@ export const MessageTray = GObject.registerClass({ this._sources.add(source); source.connectObject( - 'notification-show', this._onNotificationShow.bind(this), + 'notification-request-banner', this._onNotificationRequestBanner.bind(this), 'notification-removed', this._onNotificationRemoved.bind(this), 'destroy', () => this._removeSource(source), this); @@ -923,7 +918,17 @@ export const MessageTray = GObject.registerClass({ } } - _onNotificationShow(_source, notification) { + _onNotificationRequestBanner(_source, notification) { + // We never display a banner for already acknowledged notifications + if (notification.acknowledged) + return; + + if (notification.urgency === Urgency.LOW) + return; + + if (!notification.source.policy.showBanners && notification.urgency !== Urgency.CRITICAL) + return; + if (this._notification === notification) { // If a notification that is being shown is updated, we update // how it is shown and extend the time until it auto-hides. diff --git a/js/ui/notificationDaemon.js b/js/ui/notificationDaemon.js index a8c03ac11..ed3f58720 100644 --- a/js/ui/notificationDaemon.js +++ b/js/ui/notificationDaemon.js @@ -200,6 +200,7 @@ class FdoNotificationDaemon { const soundFile = 'sound-file' in hints ? Gio.File.new_for_path(hints['sound-file']) : null; + notification.acknowledged = false; notification.update(summary, body, { gicon, bannerMarkup: true, @@ -355,10 +356,12 @@ class FdoNotificationDaemonSource extends MessageTray.Source { } let tracker = Shell.WindowTracker.get_default(); + // Acknowledge notifications that are resident and their app has the + // current focus so that we don't show a banner. if (notification.resident && this.app && tracker.focus_app === this.app) - this.pushNotification(notification); - else - this.showNotification(notification); + notification.acknowledged = true; + + this.addNotification(notification); } open() { @@ -402,9 +405,10 @@ const PRIORITY_URGENCY_MAP = { const GtkNotificationDaemonNotification = GObject.registerClass( class GtkNotificationDaemonNotification extends MessageTray.Notification { - _init(source, notification) { + _init(source, id, notification) { super._init(source); this._serialized = GLib.Variant.new('a{sv}', notification); + this.id = id; const { title, @@ -499,15 +503,12 @@ class GtkNotificationDaemonAppSource extends MessageTray.Source { this._notificationPending = false; } - _createNotification(params) { - return new GtkNotificationDaemonNotification(this, params); - } - activateAction(actionId, target) { const params = target ? GLib.Variant.new('av', [target]) : null; this._app.activate_action(actionId, params, 0, -1, null).catch(error => { logError(error, `Failed to activate action for ${this._appId}`); }); + Main.overview.hide(); Main.panel.closeCalendar(); } @@ -518,22 +519,18 @@ class GtkNotificationDaemonAppSource extends MessageTray.Source { Main.panel.closeCalendar(); } - addNotification(notificationId, notificationParams, showBanner) { + addNotification(notification) { this._notificationPending = true; - if (this._notifications[notificationId]) - this._notifications[notificationId].destroy(MessageTray.NotificationDestroyedReason.REPLACED); + this._notifications[notification.id]?.destroy( + MessageTray.NotificationDestroyedReason.REPLACED); - let notification = this._createNotification(notificationParams); notification.connect('destroy', () => { - delete this._notifications[notificationId]; + delete this._notifications[notification.id]; }); - this._notifications[notificationId] = notification; + this._notifications[notification.id] = notification; - if (showBanner) - this.showNotification(notification); - else - this.pushNotification(notification); + super.addNotification(notification); this._notificationPending = false; } @@ -609,8 +606,13 @@ class GtkNotificationDaemon { throw e; } - notifications.forEach(([notificationId, notification]) => { - source.addNotification(notificationId, notification.deepUnpack(), false); + notifications.forEach(([notificationId, notificationPacked]) => { + const notification = new GtkNotificationDaemonNotification(source, + notificationId, + notificationPacked.deepUnpack()); + // Acknowledge all stored notification so that we don't show a banner again + notification.acknowledged = true; + source.addNotification(notification); }); }); } @@ -635,7 +637,7 @@ class GtkNotificationDaemon { } AddNotificationAsync(params, invocation) { - let [appId, notificationId, notification] = params; + let [appId, notificationId, notificationSerialized] = params; let source; try { @@ -651,9 +653,12 @@ class GtkNotificationDaemon { } let timestamp = GLib.DateTime.new_now_local().to_unix(); - notification['timestamp'] = new GLib.Variant('x', timestamp); + notificationSerialized['timestamp'] = new GLib.Variant('x', timestamp); - source.addNotification(notificationId, notification, true); + const notification = new GtkNotificationDaemonNotification(source, + notificationId, + notificationSerialized); + source.addNotification(notification); invocation.return_value(null); } diff --git a/js/ui/overview.js b/js/ui/overview.js index 2d6c2a4ab..32a9df89d 100644 --- a/js/ui/overview.js +++ b/js/ui/overview.js @@ -50,7 +50,7 @@ class ShellInfo { if (undoCallback) this._notification.addAction(_('Undo'), () => undoCallback()); - source.showNotification(this._notification); + source.addNotification(this._notification); } } diff --git a/js/ui/screenshot.js b/js/ui/screenshot.js index f78eeacd1..d19c6e7b9 100644 --- a/js/ui/screenshot.js +++ b/js/ui/screenshot.js @@ -2122,7 +2122,7 @@ export const ScreenshotUI = GObject.registerClass({ } Main.messageTray.add(source); - source.showNotification(notification); + source.addNotification(notification); } get screencast_in_progress() { @@ -2364,7 +2364,7 @@ function _storeScreenshot(bytes, pixbuf) { notification.setTransient(true); Main.messageTray.add(source); - source.showNotification(notification); + source.addNotification(notification); return file; } diff --git a/js/ui/shellMountOperation.js b/js/ui/shellMountOperation.js index b02b65b6c..9188410a5 100644 --- a/js/ui/shellMountOperation.js +++ b/js/ui/shellMountOperation.js @@ -200,7 +200,7 @@ export class ShellMountOperation { this._notification.setTransient(true); this._notification.iconName = 'media-removable-symbolic'; this._notification.connect('destroy', () => delete this._notification); - source.showNotification(this._notification); + source.addNotification(this._notification); } _showUnmountNotificationDone(message) { diff --git a/js/ui/status/network.js b/js/ui/status/network.js index dad668358..86cfa95f8 100644 --- a/js/ui/status/network.js +++ b/js/ui/status/network.js @@ -2039,7 +2039,7 @@ class Indicator extends SystemIndicator { this._notification.connect('destroy', () => (this._notification = null)); - source.showNotification(this._notification); + source.addNotification(this._notification); } _syncMainConnection() { diff --git a/js/ui/status/thunderbolt.js b/js/ui/status/thunderbolt.js index 9e33c21f5..96d84d368 100644 --- a/js/ui/status/thunderbolt.js +++ b/js/ui/status/thunderbolt.js @@ -268,7 +268,7 @@ class Indicator extends SystemIndicator { const app = Shell.AppSystem.get_default().lookup_app('gnome-thunderbolt-panel.desktop'); app?.activate(); }); - source.showNotification(this._notification); + source.addNotification(this._notification); } /* Session callbacks */ diff --git a/js/ui/windowAttentionHandler.js b/js/ui/windowAttentionHandler.js index 1525dae29..ee701394c 100644 --- a/js/ui/windowAttentionHandler.js +++ b/js/ui/windowAttentionHandler.js @@ -45,7 +45,7 @@ export class WindowAttentionHandler { }); notification.setForFeedback(true); - source.showNotification(notification); + source.addNotification(notification); window.connectObject('notify::title', () => { [title, banner] = this._getTitleAndBanner(app, window); diff --git a/tests/shell/basic.js b/tests/shell/basic.js index e5bd8c187..5310a31b2 100644 --- a/tests/shell/basic.js +++ b/tests/shell/basic.js @@ -53,12 +53,12 @@ export async function run() { const source = new MessageTray.getSystemSource(); Scripting.scriptEvent('notificationShowStart'); - source.connect('notification-show', + source.connect('notification-request-banner', () => Scripting.scriptEvent('notificationShowDone')); const notification = new MessageTray.Notification(source, 'A test notification'); - source.showNotification(notification); + source.addNotification(notification); await Scripting.sleep(400); console.debug('Show date menu');