Skip to content

Commit 0f89114

Browse files
bernhardkaindlbernhardkaindl
committed
fix(desktop): address follow-up notification review
Signed-off-by: Bernhard Kaindl <[email protected]>
1 parent d814976 commit 0f89114

18 files changed

Lines changed: 669 additions & 53 deletions

‎desktop/src-tauri/Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎desktop/src-tauri/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ user-idle = { version = "0.6", default-features = false }
6969
# Native Windows toast notifications so the app registers with Windows Settings
7070
# > System > Notifications and click actions work through WinRT.
7171
tauri-winrt-notification = "0.7"
72+
windows = { version = "0.61", features = ["UI_Notifications"] }
7273

7374
[dependencies]
7475
atomic-write-file = "0.3"

‎desktop/src-tauri/src/commands/notifications.rs‎

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,14 @@ pub async fn show_native_notification(
5151
}
5252
}
5353

54+
#[cfg(target_os = "windows")]
55+
#[tauri::command]
56+
pub async fn windows_notification_permission_state(
57+
app: tauri::AppHandle,
58+
) -> Result<String, String> {
59+
windows::permission_state(app).await.map(str::to_string)
60+
}
61+
5462
#[cfg(target_os = "linux")]
5563
mod linux {
5664
use super::NATIVE_NOTIFICATION_ACTIVATED_EVENT;
@@ -123,8 +131,41 @@ mod linux {
123131
#[cfg(target_os = "windows")]
124132
mod windows {
125133
use super::NATIVE_NOTIFICATION_ACTIVATED_EVENT;
126-
use tauri::{Emitter, Manager};
134+
use tauri::Emitter;
127135
use tauri_winrt_notification::{Duration, Toast};
136+
use windows::{
137+
core::HSTRING,
138+
UI::Notifications::{NotificationSetting, ToastNotificationManager},
139+
};
140+
141+
fn permission_state_label(setting: NotificationSetting) -> &'static str {
142+
if setting == NotificationSetting::Enabled {
143+
"granted"
144+
} else {
145+
"denied"
146+
}
147+
}
148+
149+
pub async fn permission_state(app: tauri::AppHandle) -> Result<&'static str, String> {
150+
let app_id = app.config().identifier.clone();
151+
let (sender, receiver) = tokio::sync::oneshot::channel();
152+
153+
app.run_on_main_thread(move || {
154+
let result =
155+
ToastNotificationManager::CreateToastNotifierWithId(&HSTRING::from(app_id))
156+
.and_then(|notifier| notifier.Setting())
157+
.map(permission_state_label)
158+
.map_err(|error| {
159+
format!("failed to query Windows notification setting: {error}")
160+
});
161+
let _ = sender.send(result);
162+
})
163+
.map_err(|error| format!("failed to schedule Windows notification query: {error}"))?;
164+
165+
receiver
166+
.await
167+
.map_err(|_| "Windows notification query ended before completing".to_string())?
168+
}
128169

129170
pub async fn show(
130171
app: tauri::AppHandle,
@@ -165,4 +206,25 @@ mod windows {
165206
.await
166207
.map_err(|_| "Windows notification task ended before posting".to_string())?
167208
}
209+
210+
#[cfg(test)]
211+
mod tests {
212+
use super::*;
213+
214+
#[test]
215+
fn only_enabled_notification_setting_is_granted() {
216+
assert_eq!(
217+
permission_state_label(NotificationSetting::Enabled),
218+
"granted"
219+
);
220+
for setting in [
221+
NotificationSetting::DisabledForApplication,
222+
NotificationSetting::DisabledForUser,
223+
NotificationSetting::DisabledByGroupPolicy,
224+
NotificationSetting::DisabledByManifest,
225+
] {
226+
assert_eq!(permission_state_label(setting), "denied");
227+
}
228+
}
229+
}
168230
}

‎desktop/src-tauri/src/lib.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,8 @@ pub fn run() {
661661
get_event,
662662
get_events,
663663
show_native_notification,
664+
#[cfg(target_os = "windows")]
665+
windows_notification_permission_state,
664666
#[cfg(target_os = "macos")]
665667
macos_notifications::take_pending_activations,
666668
#[cfg(target_os = "macos")]

‎desktop/src/app/routes/ChannelRouteScreen.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,7 @@ export function ChannelRouteScreen({
306306
projects={projectsQuery.data ?? [projectHome]}
307307
targetMessageEvents={targetMessageEvents}
308308
targetMessageId={targetMessageId}
309+
targetMessageView={targetMessageView}
309310
/>
310311
);
311312
}

‎desktop/src/features/channels/ui/useChannelRouteTarget.test.mjs‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import assert from "node:assert/strict";
2+
import { after, before, test } from "node:test";
3+
import { JSDOM } from "jsdom";
4+
5+
const dom = new JSDOM("<!doctype html><html><body></body></html>", {
6+
url: "http://localhost",
7+
});
8+
before(() => {
9+
Object.assign(globalThis, {
10+
document: dom.window.document,
11+
HTMLElement: dom.window.HTMLElement,
12+
IS_REACT_ACT_ENVIRONMENT: true,
13+
window: dom.window,
14+
});
15+
});
16+
after(() => dom.window.close());
17+
18+
test("timeline intent keeps an ordinary reply out of the thread panel", async () => {
19+
const { cleanup, renderHook } = await import("@testing-library/react");
20+
const { useChannelRouteTarget } = await import("./useChannelRouteTarget.ts");
21+
const calls = [];
22+
const root = {
23+
id: "root",
24+
parentId: null,
25+
rootId: null,
26+
tags: [],
27+
};
28+
const reply = {
29+
id: "reply",
30+
parentId: "root",
31+
rootId: "root",
32+
tags: [
33+
["e", "root", "", "root"],
34+
["e", "root", "", "reply"],
35+
],
36+
};
37+
38+
const hook = renderHook(() =>
39+
useChannelRouteTarget({
40+
activeChannel: { id: "channel", channelType: "stream" },
41+
activeChannelId: "channel",
42+
closeAgentSession: () => calls.push("close-agent"),
43+
requireThreadEditResolution: () => {
44+
calls.push("resolve-edit");
45+
return true;
46+
},
47+
setEditTargetId: () => calls.push("edit"),
48+
setExpandedThreadReplyIds: () => calls.push("expand"),
49+
setOpenThreadHeadId: () => calls.push("open-thread"),
50+
setProfilePanelPubkey: () => calls.push("profile"),
51+
setThreadReplyTargetId: () => calls.push("reply-target"),
52+
setThreadScrollTargetId: () => calls.push("scroll"),
53+
targetMessageId: "reply",
54+
targetMessageView: "timeline",
55+
timelineMessages: [root, reply],
56+
}),
57+
);
58+
59+
assert.equal(hook.result.current, "root");
60+
assert.deepEqual(calls, []);
61+
hook.unmount();
62+
cleanup();
63+
});
64+
65+
test("timeline intent preserves the exact id of a broadcast reply row", async () => {
66+
const { cleanup, renderHook } = await import("@testing-library/react");
67+
const { useChannelRouteTarget } = await import("./useChannelRouteTarget.ts");
68+
const broadcast = {
69+
id: "broadcast-reply",
70+
parentId: "root",
71+
rootId: "root",
72+
tags: [
73+
["e", "root", "", "root"],
74+
["e", "root", "", "reply"],
75+
["broadcast", "1"],
76+
],
77+
};
78+
79+
const hook = renderHook(() =>
80+
useChannelRouteTarget({
81+
activeChannel: { id: "channel", channelType: "stream" },
82+
activeChannelId: "channel",
83+
closeAgentSession: () => {},
84+
requireThreadEditResolution: () => true,
85+
setEditTargetId: () => {},
86+
setExpandedThreadReplyIds: () => {},
87+
setOpenThreadHeadId: () => {},
88+
setProfilePanelPubkey: () => {},
89+
setThreadReplyTargetId: () => {},
90+
setThreadScrollTargetId: () => {},
91+
targetMessageId: "broadcast-reply",
92+
targetMessageView: "timeline",
93+
timelineMessages: [broadcast],
94+
}),
95+
);
96+
97+
assert.equal(hook.result.current, "broadcast-reply");
98+
hook.unmount();
99+
cleanup();
100+
});

‎desktop/src/features/channels/ui/useChannelRouteTarget.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ function getRouteMainTimelineTargetId(
4949
return targetMessageId;
5050
}
5151

52+
// Ordinary replies are intentionally absent from the main timeline. Keep
53+
// timeline navigation on the containing root; only broadcast replies have
54+
// a rendered row whose exact id can be targeted there.
5255
return targetMessage.rootId ?? targetMessage.parentId;
5356
}
5457

@@ -118,11 +121,13 @@ export function useChannelRouteTarget({
118121
return;
119122
}
120123

124+
// Explicit timeline intent outranks ancestry-based thread navigation.
125+
if (targetMessageView === "timeline") {
126+
handledThreadRouteTargetRef.current = targetKey;
127+
return;
128+
}
129+
121130
if (!targetMessage.parentId) {
122-
if (targetMessageView === "timeline") {
123-
handledThreadRouteTargetRef.current = targetKey;
124-
return;
125-
}
126131
if (!requireThreadEditResolution()) {
127132
return;
128133
}

‎desktop/src/features/channels/useLiveChannelUpdates.test.mjs‎

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
import assert from "node:assert/strict";
22
import { after, before, test } from "node:test";
33
import { JSDOM } from "jsdom";
4-
import { HOME_MENTION_EVENT_KINDS } from "@/shared/constants/kinds";
4+
import {
5+
HOME_MENTION_EVENT_KINDS,
6+
KIND_HUDDLE_STARTED,
7+
KIND_REACTION,
8+
KIND_STREAM_MESSAGE_DIFF,
9+
KIND_SYSTEM_MESSAGE,
10+
} from "@/shared/constants/kinds";
511

612
const dom = new JSDOM("<!doctype html><html><body></body></html>", {
713
url: "http://localhost",
@@ -356,6 +362,80 @@ test("untagged auxiliary event keeps its single-channel context in the timeline
356362
}
357363
});
358364

365+
test("live rows and auxiliary events survive authoritative window projection", async () => {
366+
const h = await mount(channels(1));
367+
try {
368+
const channelId = "channel-0";
369+
const parent = message("parent", {
370+
created_at: Math.floor(Date.now() / 1000) - 10,
371+
});
372+
h.queryClient.setQueryData(h.channelWindowKey(channelId), {
373+
pages: [
374+
{
375+
startCursor: null,
376+
rows: [{ event: parent, thread: null }],
377+
aux: [],
378+
nextCursor: null,
379+
hasMore: false,
380+
},
381+
],
382+
liveOverlay: [],
383+
liveAux: [],
384+
liveSummaries: {},
385+
});
386+
h.queryClient.setQueryData(h.channelMessagesKey(channelId), [parent]);
387+
388+
const sub = h.subscriptions[0];
389+
for (const [id, kind] of [
390+
["diff", KIND_STREAM_MESSAGE_DIFF],
391+
["system", KIND_SYSTEM_MESSAGE],
392+
["huddle", KIND_HUDDLE_STARTED],
393+
["reaction", KIND_REACTION],
394+
]) {
395+
await h.deliver(sub, message(id, { kind }));
396+
}
397+
await h.deliver(
398+
sub,
399+
message("thread-only", {
400+
tags: [
401+
["h", channelId],
402+
["e", "parent", "", "root"],
403+
["e", "parent", "", "reply"],
404+
],
405+
}),
406+
);
407+
await h.deliver(sub, message("next-row"));
408+
409+
const window = h.queryClient.getQueryData(h.channelWindowKey(channelId));
410+
assert.deepEqual(
411+
new Set(window.liveOverlay.map((event) => event.id)),
412+
new Set(["diff", "system", "huddle", "next-row"]),
413+
);
414+
assert.deepEqual(
415+
window.liveAux.map((event) => event.id),
416+
["reaction"],
417+
);
418+
assert.deepEqual(
419+
new Set(
420+
h.queryClient
421+
.getQueryData(h.channelMessagesKey(channelId))
422+
.map((event) => event.id),
423+
),
424+
new Set([
425+
"parent",
426+
"diff",
427+
"system",
428+
"huddle",
429+
"reaction",
430+
"thread-only",
431+
"next-row",
432+
]),
433+
);
434+
} finally {
435+
h.restore();
436+
}
437+
});
438+
359439
test("one failed setup does not abort other channel streams and retries only that channel", async () => {
360440
const originalSetTimeout = window.setTimeout;
361441
const originalClearTimeout = window.clearTimeout;

‎desktop/src/features/channels/useLiveChannelUpdates.ts‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@ import {
2323
} from "@/features/notifications/lib/shouldNotify";
2424
import { relayClient } from "@/shared/api/relayClient";
2525
import {
26+
CHANNEL_AUX_EVENT_KINDS,
2627
CHANNEL_EVENT_KINDS,
2728
CHANNEL_MESSAGE_EVENT_KINDS,
29+
CHANNEL_TIMELINE_CONTENT_KINDS,
2830
HOME_MENTION_EVENT_KINDS,
2931
} from "@/shared/constants/kinds";
3032
import type { Channel, RelayEvent } from "@/shared/api/types";
@@ -92,6 +94,8 @@ const CHANNELS_INVALIDATE_DEBOUNCE_MS = 500;
9294
// Only "new content" kinds should bump unread state. Shared with the
9395
// catch-up query in useUnreadChannels so the two paths stay in lockstep.
9496
const UNREAD_TRIGGER_KINDS = new Set<number>(CHANNEL_MESSAGE_EVENT_KINDS);
97+
const CHANNEL_TIMELINE_KINDS = new Set<number>(CHANNEL_TIMELINE_CONTENT_KINDS);
98+
const CHANNEL_AUX_KINDS = new Set<number>(CHANNEL_AUX_EVENT_KINDS);
9599

96100
export const EMPTY_SET: ReadonlySet<string> = new Set();
97101

@@ -358,15 +362,24 @@ export function useLiveChannelUpdates(
358362
},
359363
);
360364

365+
const isTimelineRow = CHANNEL_TIMELINE_KINDS.has(event.kind);
366+
const isAuxEvent = CHANNEL_AUX_KINDS.has(event.kind);
367+
// channelMessagesKey is derived from this store. Admission here follows
368+
// render semantics, not the deliberately narrower unread-kind policy, so
369+
// a later projection cannot discard a live row or structural overlay.
361370
if (
362-
isUnreadTriggerKind &&
363-
(!isThreadedReply || isBroadcastReply(event.tags))
371+
(isTimelineRow && (!isThreadedReply || isBroadcastReply(event.tags))) ||
372+
isAuxEvent
364373
) {
365374
const windowKey = channelWindowKey(channelId);
366375
const currentWindow =
367376
queryClient.getQueryData<ChannelWindowStore>(windowKey) ??
368377
emptyChannelWindowStore();
369-
const nextWindow = mergeLiveChannelWindowEvent(currentWindow, event);
378+
const nextWindow = mergeLiveChannelWindowEvent(
379+
currentWindow,
380+
event,
381+
isTimelineRow,
382+
);
370383
if (nextWindow !== currentWindow) {
371384
queryClient.setQueryData(windowKey, nextWindow);
372385
projectChannelWindowMessages(queryClient, channelId);

0 commit comments

Comments
 (0)