diff --git a/shell/plugins/notifications/NotificationLogic.js b/shell/plugins/notifications/NotificationLogic.js index 128369de..0e428641 100644 --- a/shell/plugins/notifications/NotificationLogic.js +++ b/shell/plugins/notifications/NotificationLogic.js @@ -92,6 +92,17 @@ function snapshotOf(notification, timestamp) { } } +// A client updating a notification through replaces_id keeps the identity of +// the popup it took over: the file name is the timestamp and id the popup was +// first persisted under, and the restore, replace and archive paths all key +// off that name. Only what the card draws comes from the updated object. +function replacementSnapshot(notification, originalId, timestamp) { + var updated = snapshotOf(notification, timestamp) + updated.id = originalId + updated.originalId = originalId + return updated +} + function historyEntry(value, normalUrgency) { var e = value || {} return { @@ -265,6 +276,7 @@ if (typeof module !== "undefined") { execFromHints: execFromHints, shouldRenderCompactGlyph: shouldRenderCompactGlyph, snapshotOf: snapshotOf, + replacementSnapshot: replacementSnapshot, historyEntry: historyEntry, parseSettings: parseSettings, historyRows: historyRows, diff --git a/shell/plugins/notifications/Service.qml b/shell/plugins/notifications/Service.qml index 0e810552..e3245b6f 100644 --- a/shell/plugins/notifications/Service.qml +++ b/shell/plugins/notifications/Service.qml @@ -177,6 +177,7 @@ Item { } persistPopupFile(snapshot) + watchForUpdates(notification, snapshot) // Qt.callLater avoids "QV4::Object::insertMember" crashes when a // Repeater is mid-incubation while we mutate its model. Qt.callLater(function() { @@ -185,6 +186,63 @@ Item { }) } + // Everything the card draws. A change to any of these is a client updating + // the notification in place, which is the only kind of update we ever hear + // about after the popup exists. + readonly property var updateSignals: [ + "summaryChanged", "bodyChanged", "appNameChanged", "appIconChanged", + "imageChanged", "urgencyChanged", "expireTimeoutChanged", "hintsChanged" + ] + + // A client that updates a notification through replaces_id does not produce + // a second onNotification: the server writes the new content onto the object + // we are already holding. The card draws a snapshot copied out of that + // object — deliberately, since the object itself must stay out of the model + // — so nothing reaches the screen until we copy it again. + function watchForUpdates(notification, snapshot) { + function refresh() { + service.refreshPopup(notification, snapshot.originalId, snapshot.timestamp) + } + + for (var i = 0; i < updateSignals.length; i++) { + var signal = notification[updateSignals[i]] + if (signal && typeof signal.connect === "function") signal.connect(refresh) + } + } + + function refreshPopup(notification, originalId, timestamp) { + // A newer notification may have taken this id over, and the object may + // outlive its popup — in both cases there is nothing here to refresh. + if (service.liveRefs[originalId] !== notification) return + + var updated + try { + updated = NotificationLogic.replacementSnapshot(notification, originalId, timestamp) + } catch (e) { + // Object torn down by the server while the signal was in flight. + return + } + + for (var i = 0; i < popupModel.count; i++) { + var row = popupModel.get(i) + if (!row || row.originalId !== originalId || row.timestamp !== timestamp) continue + popupModel.setProperty(i, "app", updated.app) + popupModel.setProperty(i, "appIcon", updated.appIcon) + popupModel.setProperty(i, "summary", updated.summary) + popupModel.setProperty(i, "body", updated.body) + popupModel.setProperty(i, "image", updated.image) + popupModel.setProperty(i, "glyph", updated.glyph) + popupModel.setProperty(i, "exec", updated.exec) + popupModel.setProperty(i, "urgency", updated.urgency) + popupModel.setProperty(i, "expireTimeout", updated.expireTimeout) + // The file name is the timestamp and id this popup was persisted under, + // so the rewrite lands on the same file: a restart restores the version + // last shown, and so does the copy that ends up in history. + persistPopupFile(updated) + return + } + } + // A restored row carries an id from the previous server generation, and // the new server hands out ids from 1 again — so a fresh notification // with the same originalId is a coincidence, not the same notification. @@ -809,6 +867,16 @@ Item { property real remainingLifetime: 1.0 readonly property bool ticking: cardSlot.lifetime > 0 && !card.hovered + // A client updating this notification in place rewrites the row + // under the card (see refreshPopup). New text deserves a full look, + // so the countdown starts over instead of running out the clock the + // superseded text was already most of the way through. Delegates + // keep their own row as the model changes around them, so only a + // real content change lands here. + onSummaryChanged: cardSlot.remainingLifetime = 1.0 + onBodyChanged: cardSlot.remainingLifetime = 1.0 + onImageChanged: cardSlot.remainingLifetime = 1.0 + Timer { interval: 50 repeat: true diff --git a/test/shell.d/notifications-test.sh b/test/shell.d/notifications-test.sh index 44692373..bd59750b 100644 --- a/test/shell.d/notifications-test.sh +++ b/test/shell.d/notifications-test.sh @@ -124,6 +124,53 @@ assertDeepEqual( 'notifications create stable snapshots' ) +// An in-place update keeps the popup's identity — the file name it was +// persisted under — and takes everything the card draws from the new content. +const replacement = notifications.replacementSnapshot( + { + id: 12, + appName: 'Slack', + summary: 'Thread v2', + body: 'message 2', + image: 'file:///tmp/new.png', + hints: { 'omarchy-glyph': '!' }, + urgency: 2, + expireTimeout: 4000 + }, + 12, + 12345 +) +assertDeepEqual( + { + id: replacement.id, + originalId: replacement.originalId, + timestamp: replacement.timestamp, + summary: replacement.summary, + body: replacement.body, + image: replacement.image, + glyph: replacement.glyph, + urgency: replacement.urgency, + expireTimeout: replacement.expireTimeout + }, + { + id: 12, + originalId: 12, + timestamp: 12345, + summary: 'Thread v2', + body: 'message 2', + image: 'file:///tmp/new.png', + glyph: '!', + urgency: 2, + expireTimeout: 4000 + }, + 'notifications take updated content without moving the popup it replaces' +) +assertEqual( + notifications.popupFileName(replacement), + '12345-12.json', + 'notifications keep the persisted file name across an in-place update' +) + const settings = notifications.parseSettings(JSON.stringify({ version: 3, dnd: true })) assertEqual(settings.dnd, true, 'notifications parse the persisted DND state') assertEqual(settings.legacy, false, 'notifications do not flag a current settings file as legacy') @@ -315,6 +362,22 @@ assert( /service\.replayCarryOver = liveRowsForReplay\(\)/.test(serviceQml), 'notifications service carries the toasts still on screen into the replay' ) +assert( + /watchForUpdates\(notification, snapshot\)/.test(serviceQml), + 'notifications service watches a shown notification for in-place updates' +) +assert( + /if \(signal && typeof signal\.connect === "function"\) signal\.connect\(refresh\)/.test(serviceQml), + 'notifications service refreshes the popup from every property the card draws' +) +assert( + /popupModel\.setProperty\(i, "summary", updated\.summary\)[\s\S]{0,600}?persistPopupFile\(updated\)/.test(serviceQml), + 'notifications service rewrites both the row and its file when a notification is updated in place' +) +assert( + /onSummaryChanged: cardSlot\.remainingLifetime = 1\.0/.test(serviceQml), + 'notifications service restarts the countdown when a toast is updated under it' +) assert( /awk 1 \\"\$1\\"\/\*\.json 2>\/dev\/null \|\| true", "--", historyDir/.test(serviceQml), 'notifications service replays history by reading the archived files'