Show a notification the sender updated, instead of the version it replaced
A client that updates a notification through replaces_id does not produce a second onNotification: Quickshell writes the new content onto the Notification object the shell is already holding. The card draws a snapshot copied out of that object — deliberately, since a live QObject in a ListModel role becomes a dangling pointer the moment the server destroys it — so the toast kept showing the superseded text, and archived it to history when it left the screen. A Slack thread that updates in place read as stuck. Every property the card draws is now watched on the notification we hold, and a change rewrites both the model row and the file the popup was persisted under. The file name is that popup's identity, so the rewrite lands in place: a shell restart restores the version last shown, and so does the copy that reaches history. The countdown starts over when the content changes. New text arriving a second before the toast was due to expire deserves a full look, not the remainder of the clock the text it replaced had nearly run through. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
ab57ad65fd
commit
cd84583b56
@@ -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) {
|
function historyEntry(value, normalUrgency) {
|
||||||
var e = value || {}
|
var e = value || {}
|
||||||
return {
|
return {
|
||||||
@@ -265,6 +276,7 @@ if (typeof module !== "undefined") {
|
|||||||
execFromHints: execFromHints,
|
execFromHints: execFromHints,
|
||||||
shouldRenderCompactGlyph: shouldRenderCompactGlyph,
|
shouldRenderCompactGlyph: shouldRenderCompactGlyph,
|
||||||
snapshotOf: snapshotOf,
|
snapshotOf: snapshotOf,
|
||||||
|
replacementSnapshot: replacementSnapshot,
|
||||||
historyEntry: historyEntry,
|
historyEntry: historyEntry,
|
||||||
parseSettings: parseSettings,
|
parseSettings: parseSettings,
|
||||||
historyRows: historyRows,
|
historyRows: historyRows,
|
||||||
|
|||||||
@@ -177,6 +177,7 @@ Item {
|
|||||||
}
|
}
|
||||||
|
|
||||||
persistPopupFile(snapshot)
|
persistPopupFile(snapshot)
|
||||||
|
watchForUpdates(notification, snapshot)
|
||||||
// Qt.callLater avoids "QV4::Object::insertMember" crashes when a
|
// Qt.callLater avoids "QV4::Object::insertMember" crashes when a
|
||||||
// Repeater is mid-incubation while we mutate its model.
|
// Repeater is mid-incubation while we mutate its model.
|
||||||
Qt.callLater(function() {
|
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
|
// 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
|
// the new server hands out ids from 1 again — so a fresh notification
|
||||||
// with the same originalId is a coincidence, not the same notification.
|
// with the same originalId is a coincidence, not the same notification.
|
||||||
@@ -809,6 +867,16 @@ Item {
|
|||||||
property real remainingLifetime: 1.0
|
property real remainingLifetime: 1.0
|
||||||
readonly property bool ticking: cardSlot.lifetime > 0 && !card.hovered
|
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 {
|
Timer {
|
||||||
interval: 50
|
interval: 50
|
||||||
repeat: true
|
repeat: true
|
||||||
|
|||||||
@@ -124,6 +124,53 @@ assertDeepEqual(
|
|||||||
'notifications create stable snapshots'
|
'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 }))
|
const settings = notifications.parseSettings(JSON.stringify({ version: 3, dnd: true }))
|
||||||
assertEqual(settings.dnd, true, 'notifications parse the persisted DND state')
|
assertEqual(settings.dnd, true, 'notifications parse the persisted DND state')
|
||||||
assertEqual(settings.legacy, false, 'notifications do not flag a current settings file as legacy')
|
assertEqual(settings.legacy, false, 'notifications do not flag a current settings file as legacy')
|
||||||
@@ -315,6 +362,22 @@ assert(
|
|||||||
/service\.replayCarryOver = liveRowsForReplay\(\)/.test(serviceQml),
|
/service\.replayCarryOver = liveRowsForReplay\(\)/.test(serviceQml),
|
||||||
'notifications service carries the toasts still on screen into the replay'
|
'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(
|
assert(
|
||||||
/awk 1 \\"\$1\\"\/\*\.json 2>\/dev\/null \|\| true", "--", historyDir/.test(serviceQml),
|
/awk 1 \\"\$1\\"\/\*\.json 2>\/dev\/null \|\| true", "--", historyDir/.test(serviceQml),
|
||||||
'notifications service replays history by reading the archived files'
|
'notifications service replays history by reading the archived files'
|
||||||
|
|||||||
Reference in New Issue
Block a user