diff --git a/desktop/src/features/channels/ui/ChannelBrowserDialog.tsx b/desktop/src/features/channels/ui/ChannelBrowserDialog.tsx index 6d56e4dbfcc..6401086b8ce 100644 --- a/desktop/src/features/channels/ui/ChannelBrowserDialog.tsx +++ b/desktop/src/features/channels/ui/ChannelBrowserDialog.tsx @@ -61,6 +61,69 @@ const CHANNEL_SORT_OPTIONS: { label: string; value: ChannelSort }[] = [ { label: "Most members", value: "members" }, ]; +/** + * The browser's channel list for one lowercased query: filtered to the active + * tab and ordered by the chosen sort, then by match score while searching. + */ +function selectBrowserChannels( + channels: Channel[], + { + activeTab, + channelTypeFilter, + query, + sort, + }: { + activeTab: BrowserTab; + channelTypeFilter?: "stream" | "forum"; + query: string; + sort: ChannelSort; + }, +): Channel[] { + // Fuzzy match score per channel id, so filtering and relevance-ordering + // share one source of truth. Empty when no query. + const matchScoreById = new Map(); + if (query.length > 0) { + for (const channel of channels) { + const score = scoreChannelMatch(channel, query); + if (score !== null) matchScoreById.set(channel.id, score); + } + } + + const visibleChannels = channels.filter( + (channel) => + channel.channelType !== "dm" && + (channel.archivedAt + ? channel.isMember + : channel.visibility === "open" || channel.isMember) && + (channelTypeFilter ? channel.channelType === channelTypeFilter : true) && + (query.length === 0 || matchScoreById.has(channel.id)) && + (activeTab === "archived" + ? channel.archivedAt !== null + : activeTab === "joined" + ? channel.archivedAt === null && channel.isMember + : true), + ); + + const sorted = + sort === "members" + ? [...visibleChannels].sort( + (a, b) => + b.memberCount - a.memberCount || + a.name.localeCompare(b.name, undefined, { + sensitivity: "base", + }), + ) + : sortChannelsForSidebar(visibleChannels, sort); + + if (query.length === 0) return sorted; + + return sorted.sort( + (a, b) => + (matchScoreById.get(a.id) ?? Number.POSITIVE_INFINITY) - + (matchScoreById.get(b.id) ?? Number.POSITIVE_INFINITY), + ); +} + function BrowseState({ icon: Icon, title, @@ -162,80 +225,18 @@ export function ChannelBrowserDialog({ onCreated: () => onOpenChange(false), }); - // Fuzzy match score per channel id for the current query, so both filtering - // and relevance-ordering share one source of truth. Empty when no query. - const matchScoreById = React.useMemo(() => { - const scores = new Map(); - if (deferredQuery.length === 0) return scores; - for (const channel of channels) { - const score = scoreChannelMatch(channel, deferredQuery); - if (score !== null) scores.set(channel.id, score); - } - return scores; - }, [channels, deferredQuery]); - - const matchingChannels = React.useMemo(() => { - const filtered = channels.filter( - (channel) => - channel.channelType !== "dm" && - (channel.archivedAt - ? channel.isMember - : channel.visibility === "open" || channel.isMember) && - (channelTypeFilter ? channel.channelType === channelTypeFilter : true), - ); - - if (deferredQuery.length === 0) { - return filtered; - } - - return filtered.filter((channel) => matchScoreById.has(channel.id)); - }, [channels, channelTypeFilter, deferredQuery, matchScoreById]); - - const currentChannels = React.useMemo( - () => matchingChannels.filter((channel) => channel.archivedAt === null), - [matchingChannels], - ); - - const joinedChannels = React.useMemo( - () => currentChannels.filter((channel) => channel.isMember), - [currentChannels], - ); - - const archivedChannels = React.useMemo( - () => matchingChannels.filter((channel) => channel.archivedAt !== null), - [matchingChannels], + // The list renders from the deferred query so typing stays responsive. + const orderedVisibleChannels = React.useMemo( + () => + selectBrowserChannels(channels, { + activeTab, + channelTypeFilter, + query: deferredQuery, + sort, + }), + [activeTab, channelTypeFilter, channels, deferredQuery, sort], ); - const visibleChannels = - activeTab === "archived" - ? archivedChannels - : activeTab === "joined" - ? joinedChannels - : matchingChannels; - - const isSearching = deferredQuery.length > 0; - - const orderedVisibleChannels = React.useMemo(() => { - const sorted = - sort === "members" - ? [...visibleChannels].sort( - (a, b) => - b.memberCount - a.memberCount || - a.name.localeCompare(b.name, undefined, { - sensitivity: "base", - }), - ) - : sortChannelsForSidebar(visibleChannels, sort); - - if (!isSearching) return sorted; - - return sorted.sort( - (a, b) => - (matchScoreById.get(a.id) ?? Number.POSITIVE_INFINITY) - - (matchScoreById.get(b.id) ?? Number.POSITIVE_INFINITY), - ); - }, [isSearching, matchScoreById, sort, visibleChannels]); - const selectedSortLabel = CHANNEL_SORT_OPTIONS.find((option) => option.value === sort)?.label ?? "Alphabetical"; @@ -386,12 +387,6 @@ export function ChannelBrowserDialog({ }); } - // Map the flat nav index back to a channel, accounting for the create row - // occupying index 0 when present. - const selectedItem = - selectedIndex !== null && !isCreateRowSelected - ? orderedVisibleChannels[selectedIndex - channelNavOffset] - : undefined; const emptyTitle = deferredQuery.length > 0 ? `No ${entityLabel}s match your search` @@ -487,23 +482,38 @@ export function ChannelBrowserDialog({ event.key === "Enter" && !event.nativeEvent.isComposing ) { + // Enter acts on what was typed. A fast Enter can land + // before the deferred list renders, so filter the live + // query here rather than reading the stale list. + const results = + deferredQuery === normalizedQuery + ? orderedVisibleChannels + : selectBrowserChannels(channels, { + activeTab, + channelTypeFilter, + query: normalizedQuery, + sort, + }); // If the create row is highlighted — or it's the only // actionable item (no channel matches) — Enter creates. if ( showCreateRow && - (isCreateRowSelected || - orderedVisibleChannels.length === 0) + (isCreateRowSelected || results.length === 0) ) { event.preventDefault(); enterCreateMode(trimmedQuery); return; } - if (orderedVisibleChannels.length > 0) { + if (results.length > 0) { event.preventDefault(); - handleSelect( - selectedItem ?? orderedVisibleChannels[0], - ); + // Map the flat nav index back to a channel, + // accounting for the create row at index 0. + const selectedItem = + selectedIndex !== null && !isCreateRowSelected + ? results[selectedIndex - channelNavOffset] + : undefined; + handleSelect(selectedItem ?? results[0]); } } }} diff --git a/desktop/src/features/messages/lib/autocompleteTriggerClose.jsdom-test.mjs b/desktop/src/features/messages/lib/autocompleteTriggerClose.jsdom-test.mjs new file mode 100644 index 00000000000..11ad3dad688 --- /dev/null +++ b/desktop/src/features/messages/lib/autocompleteTriggerClose.jsdom-test.mjs @@ -0,0 +1,106 @@ +/** + * Clearing a composer draft must close its `#channel` or `:emoji` + * autocomplete at once. Closing used to wait out the 120 ms query debounce, so + * an Enter pressed right after clearing picked the stale suggestion instead of + * submitting (empty-edit-delete.spec.ts covers the `@mention` hook end to end). + */ + +import assert from "node:assert/strict"; +import { afterEach, test } from "node:test"; + +import { act, cleanup, renderHook, waitFor } from "@testing-library/react"; +import * as React from "react"; + +import { ChannelNavigationProvider } from "../../../shared/context/ChannelNavigationContext.tsx"; +import { useChannelLinks } from "./useChannelLinks.ts"; +import { useEmojiAutocomplete } from "./useEmojiAutocomplete.ts"; + +afterEach(cleanup); + +const channels = [ + { + id: "general-id", + name: "general", + channelType: "stream", + archivedAt: null, + }, +]; +const customEmoji = [ + { shortcode: "partyparrot", url: "https://example.test/partyparrot.png" }, +]; + +const cases = [ + { + trigger: "#channel", + draft: "Ship notes for #general", + renderAutocomplete: () => + renderHook(() => useChannelLinks(), { + wrapper: ({ children }) => + React.createElement( + ChannelNavigationProvider, + { channels }, + children, + ), + }), + isOpen: (hook) => hook.isChannelOpen, + update: (hook, value, cursor) => hook.updateChannelQuery(value, cursor), + keyDown: (hook, event) => hook.handleChannelKeyDown(event), + }, + { + trigger: ":emoji", + draft: "Ship notes :party", + renderAutocomplete: () => + renderHook(() => useEmojiAutocomplete(customEmoji)), + isOpen: (hook) => hook.isEmojiAutocompleteOpen, + update: (hook, value, cursor) => hook.updateEmojiQuery(value, cursor), + keyDown: (hook, event) => hook.handleEmojiKeyDown(event), + }, +]; + +for (const { + trigger, + draft, + renderAutocomplete, + isOpen, + update, + keyDown, +} of cases) { + test(`clearing a ${trigger} draft closes its autocomplete before Enter`, async () => { + const { result } = renderAutocomplete(); + + act(() => update(result.current, draft, draft.length)); + // Typing a query still debounces. + assert.equal(isOpen(result.current), false); + await waitFor(() => assert.equal(isOpen(result.current), true)); + + act(() => update(result.current, "", 0)); + assert.equal(isOpen(result.current), false); + let prevented = false; + const enter = { + key: "Enter", + altKey: false, + ctrlKey: false, + metaKey: false, + shiftKey: false, + preventDefault: () => { + prevented = true; + }, + }; + assert.deepEqual(keyDown(result.current, enter), { handled: false }); + assert.equal(prevented, false); + }); + + test(`dropping a ${trigger} trigger without clearing the draft still debounces`, async () => { + const { result } = renderAutocomplete(); + + act(() => update(result.current, draft, draft.length)); + await waitFor(() => assert.equal(isOpen(result.current), true)); + + // Selecting a suggestion with Enter splits the paragraph in the editor + // before the composer's keydown handler runs. That update carries no + // trigger, but the handler must still see the open autocomplete. + act(() => update(result.current, "Ship notes\n", 11)); + assert.equal(isOpen(result.current), true); + await waitFor(() => assert.equal(isOpen(result.current), false)); + }); +} diff --git a/desktop/src/features/messages/lib/channelWindowReconciliation.ts b/desktop/src/features/messages/lib/channelWindowReconciliation.ts index f6a7e4df21f..df1e33af3cb 100644 --- a/desktop/src/features/messages/lib/channelWindowReconciliation.ts +++ b/desktop/src/features/messages/lib/channelWindowReconciliation.ts @@ -1,7 +1,7 @@ import type { RelayEvent } from "@/shared/api/types"; import { CHANNEL_TIMELINE_CONTENT_KINDS } from "@/shared/constants/kinds"; +import { compareChronologicalOrder } from "@/shared/lib/eventOrder"; import { - compareRelayOrder, flattenChannelWindowEvents, type ChannelWindowStore, } from "./channelWindowStore"; @@ -19,6 +19,31 @@ function retainRefetchReconciliationEvents(events: RelayEvent[]) { }); } +/** + * An acknowledged send keeps its optimistic id as `localKey`, and its row + * renders under that key. A head refetch or a live echo of the same event + * arrives without one, so carry it over by event id. Otherwise the row + * remounts and a frozen unread tail treats it as deleted. + */ +function retainAcknowledgedLocalKeys( + windowEvents: RelayEvent[], + messages: RelayEvent[], +) { + const localKeys = new Map(); + for (const message of messages) { + if (message.localKey !== undefined) { + localKeys.set(message.id, message.localKey); + } + } + if (localKeys.size === 0) return windowEvents; + return windowEvents.map((event) => { + const localKey = localKeys.get(event.id); + return localKey === undefined || event.localKey !== undefined + ? event + : { ...event, localKey }; + }); +} + /** * Project the timeline from the authoritative window while retaining local * pending sends and non-broadcast thread replies the window does not contain. @@ -27,7 +52,10 @@ export function reconcileChannelWindowMessages( window: ChannelWindowStore, messages: RelayEvent[], ) { - const windowEvents = flattenChannelWindowEvents(window); + const windowEvents = retainAcknowledgedLocalKeys( + flattenChannelWindowEvents(window), + messages, + ); if (window.pages.length === 0) { // A pageless window is unresolved, not authoritative. This state can exist // briefly when the companion window query mounts beside an already-cached @@ -38,7 +66,7 @@ export function reconcileChannelWindowMessages( for (const event of windowEvents) { merged = reconcileIncomingMessage(merged, event); } - return [...merged].sort((left, right) => compareRelayOrder(right, left)); + return [...merged].sort(compareChronologicalOrder); } const authoritativeIds = new Set(windowEvents.map((event) => event.id)); const retained = retainRefetchReconciliationEvents(messages).filter( @@ -64,9 +92,7 @@ function mergeChronologicalMessages( cacheOnly: RelayEvent[], authoritative: RelayEvent[], ) { - const retained = [...cacheOnly].sort((left, right) => - compareRelayOrder(right, left), - ); + const retained = [...cacheOnly].sort(compareChronologicalOrder); const merged: RelayEvent[] = []; let retainedIndex = 0; let authoritativeIndex = 0; @@ -77,7 +103,7 @@ function mergeChronologicalMessages( ) { const retainedEvent = retained[retainedIndex]; const authoritativeEvent = authoritative[authoritativeIndex]; - if (compareRelayOrder(retainedEvent, authoritativeEvent) > 0) { + if (compareChronologicalOrder(retainedEvent, authoritativeEvent) < 0) { merged.push(retainedEvent); retainedIndex += 1; } else { diff --git a/desktop/src/features/messages/lib/channelWindowStore.ts b/desktop/src/features/messages/lib/channelWindowStore.ts index 1bd921aabf4..63d343d3a53 100644 --- a/desktop/src/features/messages/lib/channelWindowStore.ts +++ b/desktop/src/features/messages/lib/channelWindowStore.ts @@ -1,4 +1,8 @@ import type { RelayEvent } from "@/shared/api/types"; +import { + compareChronologicalOrder, + compareRelayOrder, +} from "@/shared/lib/eventOrder"; export type ChannelWindowCursor = { createdAt: number; eventId: string }; export type ChannelWindowThreadSummary = { @@ -56,17 +60,6 @@ function cursorsEqual( ); } -/** Relay order: newest timestamp first, then ascending id within a second. */ -export function compareRelayOrder(left: RelayEvent, right: RelayEvent) { - return left.created_at !== right.created_at - ? right.created_at - left.created_at - : left.id < right.id - ? -1 - : left.id > right.id - ? 1 - : 0; -} - function isStrictlyOlder(event: RelayEvent, cursor: ChannelWindowCursor) { return ( event.created_at < cursor.createdAt || @@ -271,9 +264,7 @@ export function flattenChannelWindowEvents(store: ChannelWindowStore) { } for (const event of store.liveOverlay) byId.set(event.id, event); for (const event of store.liveAux) byId.set(event.id, event); - return [...byId.values()].sort((left, right) => - compareRelayOrder(right, left), - ); + return [...byId.values()].sort(compareChronologicalOrder); } /** diff --git a/desktop/src/features/messages/lib/messageQueryKeys.test.mjs b/desktop/src/features/messages/lib/messageQueryKeys.test.mjs index 217b171f1ac..1e40d4d6350 100644 --- a/desktop/src/features/messages/lib/messageQueryKeys.test.mjs +++ b/desktop/src/features/messages/lib/messageQueryKeys.test.mjs @@ -135,9 +135,10 @@ test("timeline history merge preserves the older window despite auxiliary events test("sortMessages tiebreaks same-second events on id, order-independent", () => { // Three events sharing one created_at, fed in two different input orders. - // The (created_at, id) sort must produce the same sequence both ways, so a - // history-then-live merge and a live-then-history merge can't shuffle a - // same-second message to a different visible position. + // The sort must produce the same sequence both ways, so a history-then-live + // merge and a live-then-history merge can't shuffle a same-second message to + // a different visible position. Within a second the larger id is older + // (the reverse of relay order), matching the channel window. const a = event({ id: id("aaa", 1), createdAt: 5_000 }); const b = event({ id: id("bbb", 1), createdAt: 5_000 }); const c = event({ id: id("ccc", 1), createdAt: 5_000 }); @@ -146,5 +147,5 @@ test("sortMessages tiebreaks same-second events on id, order-independent", () => const reverse = normalizeTimelineMessages([c, b, a]).map((m) => m.id); assert.deepEqual(forward, reverse); - assert.deepEqual(forward, [a.id, b.id, c.id]); + assert.deepEqual(forward, [c.id, b.id, a.id]); }); diff --git a/desktop/src/features/messages/lib/messageQueryKeys.ts b/desktop/src/features/messages/lib/messageQueryKeys.ts index 67d8a8d7eca..690a410f0c8 100644 --- a/desktop/src/features/messages/lib/messageQueryKeys.ts +++ b/desktop/src/features/messages/lib/messageQueryKeys.ts @@ -1,4 +1,5 @@ import type { RelayEvent } from "@/shared/api/types"; +import { compareChronologicalOrder } from "@/shared/lib/eventOrder"; export function channelMessagesKey(channelId: string) { return ["channel-messages", channelId] as const; @@ -31,16 +32,9 @@ export function dedupeMessagesById(messages: RelayEvent[]) { } export function sortMessages(messages: RelayEvent[]) { - return dedupeMessagesById(messages).sort((left, right) => { - if (left.created_at !== right.created_at) { - return left.created_at - right.created_at; - } - // Tiebreak same-second events on id so the merge order is deterministic. - // Without this, two events sharing a created_at can land in a different - // position depending on which REQ (history vs live-sub) delivered them - // first — reading as a "missing"/shuffled message at a fixed scroll offset. - return left.id < right.id ? -1 : left.id > right.id ? 1 : 0; - }); + // The channel window projects the same timeline, so same-second events + // must tiebreak exactly as it does, whichever REQ delivered them first. + return dedupeMessagesById(messages).sort(compareChronologicalOrder); } export function normalizeTimelineMessages(messages: RelayEvent[]) { diff --git a/desktop/src/features/messages/lib/projectChannelWindow.test.mjs b/desktop/src/features/messages/lib/projectChannelWindow.test.mjs index 9594b937879..70815c5d91b 100644 --- a/desktop/src/features/messages/lib/projectChannelWindow.test.mjs +++ b/desktop/src/features/messages/lib/projectChannelWindow.test.mjs @@ -4,6 +4,7 @@ import { QueryClient, QueryObserver } from "@tanstack/react-query"; import { reconcileFetchedChannelWindow } from "../hooks.ts"; import { channelMessagesKey, channelWindowKey } from "./messageQueryKeys.ts"; +import { mergeTimelineCacheMessages } from "./messageMerge.ts"; import { appendOlderChannelWindow, emptyChannelWindowStore, @@ -210,6 +211,37 @@ test("test_reconciliation_preserves_dense_second_window_order", () => { ); }); +test("test_live_cache_merge_and_window_projection_agree_on_same_second_order", () => { + // useLiveChannelUpdates merges each live event straight into the rendered + // cache, and the subscription projects the same event through the window. + // If the two disagree on same-second order, the rows swap on every write. + for (const arrivals of [ + ["a", "b"], + ["b", "a"], + ]) { + const harness = createHarness(); + const rendered = []; + for (const content of arrivals) { + const live = event(content, 200); + harness.client.setQueryData(harness.messagesKey, (current) => + mergeTimelineCacheMessages(current, live), + ); + rendered.push( + harness.client.getQueryData(harness.messagesKey).map((m) => m.content), + ); + appendLiveEvent(harness, live); + rendered.push( + harness.client.getQueryData(harness.messagesKey).map((m) => m.content), + ); + } + + assert.deepEqual(rendered.slice(2), [ + ["initial", "b", "a"], + ["initial", "b", "a"], + ]); + } +}); + test("test_reconciliation_retains_identical_pending_sends", () => { const harness = createHarness(); const first = { diff --git a/desktop/src/features/messages/lib/useChannelLinks.ts b/desktop/src/features/messages/lib/useChannelLinks.ts index 4b703f53cd6..ddc8eb9df74 100644 --- a/desktop/src/features/messages/lib/useChannelLinks.ts +++ b/desktop/src/features/messages/lib/useChannelLinks.ts @@ -122,6 +122,16 @@ export function useChannelLinks() { if (debounceTimerRef.current !== null) { clearTimeout(debounceTimerRef.current); + debounceTimerRef.current = null; + } + + // A cleared draft closes now: an Enter inside the debounce would + // otherwise pick a stale suggestion instead of submitting. Any other + // update keeps the debounce, so an edit made while selecting a + // suggestion cannot close the menu under the keydown handler. + if (value.length === 0) { + setChannelQuery(null); + return; } debounceTimerRef.current = setTimeout(() => { diff --git a/desktop/src/features/messages/lib/useEmojiAutocomplete.ts b/desktop/src/features/messages/lib/useEmojiAutocomplete.ts index 21a0a102350..468492ded90 100644 --- a/desktop/src/features/messages/lib/useEmojiAutocomplete.ts +++ b/desktop/src/features/messages/lib/useEmojiAutocomplete.ts @@ -185,6 +185,13 @@ export function useEmojiAutocomplete(customEmoji: CustomEmoji[] = []) { if (debounceTimerRef.current !== null) { clearTimeout(debounceTimerRef.current); + debounceTimerRef.current = null; + } + + // A cleared draft closes now; see useChannelLinks. + if (value.length === 0) { + setEmojiQuery(null); + return; } debounceTimerRef.current = setTimeout(() => { diff --git a/desktop/src/features/messages/lib/useMentions.ts b/desktop/src/features/messages/lib/useMentions.ts index 63f8aaa12be..6b0a7686e6e 100644 --- a/desktop/src/features/messages/lib/useMentions.ts +++ b/desktop/src/features/messages/lib/useMentions.ts @@ -696,13 +696,24 @@ export function useMentions( cursorPosition, searchableNamesLowerRef.current, ); + if (debounceTimerRef.current !== null) { + clearTimeout(debounceTimerRef.current); + debounceTimerRef.current = null; + } if (activeInlineMention) { mentionPickerOriginRef.current = "inline"; } else if (mentionPickerOriginRef.current === "inline") { mentionPickerOriginRef.current = null; - } - if (debounceTimerRef.current !== null) { - clearTimeout(debounceTimerRef.current); + // A cleared draft closes now: an Enter inside the debounce would + // otherwise pick a stale suggestion instead of submitting. Any other + // trigger-less update keeps the debounce. Selecting a suggestion, or + // the Enter that selects it, edits the document before the keydown + // handler runs, and closing here would hide the open menu from that + // handler. A picker opened explicitly has no trigger to lose. + if (value.length === 0) { + setMentionQuery(null); + return; + } } debounceTimerRef.current = setTimeout(() => { debounceTimerRef.current = null; diff --git a/desktop/src/features/messages/ui/ChannelAutocomplete.tsx b/desktop/src/features/messages/ui/ChannelAutocomplete.tsx index 5c759a8a693..c0520e2994f 100644 --- a/desktop/src/features/messages/ui/ChannelAutocomplete.tsx +++ b/desktop/src/features/messages/ui/ChannelAutocomplete.tsx @@ -58,6 +58,7 @@ export const ChannelAutocomplete = React.memo(function ChannelAutocomplete({ : "origin-bottom slide-in-from-bottom-1", POPOVER_SURFACE_CLASS, )} + data-testid="channel-autocomplete" onMouseDown={(event) => event.preventDefault()} ref={listRef} style={POPOVER_SHADOW_STYLE} diff --git a/desktop/src/features/messages/ui/useBufferedTimelineMessages.jsdom-test.mjs b/desktop/src/features/messages/ui/useBufferedTimelineMessages.jsdom-test.mjs new file mode 100644 index 00000000000..1f05dd00912 --- /dev/null +++ b/desktop/src/features/messages/ui/useBufferedTimelineMessages.jsdom-test.mjs @@ -0,0 +1,121 @@ +/** + * An acknowledged send must stay in the frozen unread tail when its event + * comes back without the optimistic `localKey`, from a head refetch or from a + * live echo after the ack. Only arrivals newer than the snapshot are pending. + */ + +import assert from "node:assert/strict"; +import { afterEach, test } from "node:test"; + +import { cleanup, renderHook } from "@testing-library/react"; + +import { createOptimisticMessage } from "../hooks.ts"; +import { reconcileChannelWindowMessages } from "../lib/channelWindowReconciliation.ts"; +import { + emptyChannelWindowStore, + mergeLiveChannelWindowEvent, + replaceNewestChannelWindow, +} from "../lib/channelWindowStore.ts"; +import { formatTimelineMessages } from "../lib/formatTimelineMessages.ts"; +import { useBufferedTimelineMessages } from "./useBufferedTimelineMessages.ts"; + +afterEach(cleanup); + +const channelId = "channel"; +const pubkey = "a".repeat(64); + +function relayEvent(id, createdAt, content = id) { + return { + id: id.padEnd(64, "0"), + pubkey, + created_at: createdAt, + kind: 9, + tags: [["h", channelId]], + content, + sig: "b".repeat(128), + }; +} + +function newestPage(events) { + return { + startCursor: null, + rows: events.map((event) => ({ event, thread: null })), + aux: [], + nextCursor: null, + hasMore: false, + }; +} + +const cases = [ + { + path: "a head refetch", + returnWithoutLocalKey: (window, accepted, earlier) => + replaceNewestChannelWindow(window, newestPage([accepted, earlier])), + }, + { + path: "a live echo after the ack", + returnWithoutLocalKey: (window, accepted) => + mergeLiveChannelWindowEvent(window, accepted), + }, +]; + +for (const { path, returnWithoutLocalKey } of cases) { + test(`an acknowledged send stays frozen when ${path} drops its localKey`, () => { + const now = Math.floor(Date.now() / 1_000); + const earlier = relayEvent("earlier", now - 60); + let window = replaceNewestChannelWindow( + emptyChannelWindowStore(), + newestPage([earlier]), + ); + let messages = reconcileChannelWindowMessages(window, []); + const advance = (next) => { + window = next; + messages = reconcileChannelWindowMessages(window, messages); + return formatTimelineMessages(messages, null, pubkey, null); + }; + + // Send, then acknowledge the way useSendMessageMutation does. + const optimistic = createOptimisticMessage( + channelId, + "hello", + { pubkey }, + messages, + ); + advance(mergeLiveChannelWindowEvent(window, optimistic)); + const accepted = { + ...relayEvent("accepted", optimistic.created_at, optimistic.content), + tags: optimistic.tags, + }; + const acknowledged = advance( + mergeLiveChannelWindowEvent( + { + ...window, + liveOverlay: window.liveOverlay.filter( + (event) => event.id !== optimistic.id, + ), + }, + { ...accepted, localKey: optimistic.id }, + ), + ); + + // The reader scrolls up, freezing the tail with the acknowledged row. + const { result, rerender } = renderHook( + (props) => useBufferedTimelineMessages({ channelId, ...props }), + { initialProps: { isAtBottom: true, messages: acknowledged } }, + ); + rerender({ isAtBottom: false, messages: acknowledged }); + + advance(returnWithoutLocalKey(window, accepted, earlier)); + const newer = relayEvent("newer", optimistic.created_at + 1); + rerender({ + isAtBottom: false, + messages: advance(mergeLiveChannelWindowEvent(window, newer)), + }); + + assert.deepEqual( + result.current.messages.map(({ id }) => id), + [earlier.id, accepted.id], + ); + assert.equal(result.current.pendingCount, 1); + }); +} diff --git a/desktop/src/features/messages/ui/useBufferedTimelineMessages.test.mjs b/desktop/src/features/messages/ui/useBufferedTimelineMessages.test.mjs index 0d581506ae3..f401677ebff 100644 --- a/desktop/src/features/messages/ui/useBufferedTimelineMessages.test.mjs +++ b/desktop/src/features/messages/ui/useBufferedTimelineMessages.test.mjs @@ -8,7 +8,7 @@ const rows = (...ids) => ids.map((id) => ({ id })); test("freezes live arrivals after the semantic tail while scrolled up", () => { assert.deepEqual( selectBufferedTimelineMessages({ - frozenMessageIds: ["a", "b", "c"], + frozenRowKeys: ["a", "b", "c"], isAtBottom: false, messages: rows("a", "b", "c", "d", "e"), }).map(({ id }) => id), @@ -19,7 +19,7 @@ test("freezes live arrivals after the semantic tail while scrolled up", () => { test("admits older-history prepends without exposing buffered arrivals", () => { assert.deepEqual( selectBufferedTimelineMessages({ - frozenMessageIds: ["a", "b", "c"], + frozenRowKeys: ["a", "b", "c"], isAtBottom: false, messages: rows("older-a", "older-b", "a", "b", "c", "d"), }).map(({ id }) => id), @@ -31,7 +31,7 @@ test("releases the full logical dataset at bottom", () => { const messages = rows("a", "b", "c", "d"); assert.deepEqual( selectBufferedTimelineMessages({ - frozenMessageIds: ["a", "b"], + frozenRowKeys: ["a", "b"], isAtBottom: true, messages, }), @@ -43,10 +43,47 @@ test("accepts an authoritative replacement when its old tail disappeared", () => const messages = rows("x", "y"); assert.deepEqual( selectBufferedTimelineMessages({ - frozenMessageIds: ["old-tail"], + frozenRowKeys: ["old-tail"], isAtBottom: false, messages, }), messages, ); }); + +test("keeps the tail frozen when a frozen row is deleted", () => { + assert.deepEqual( + selectBufferedTimelineMessages({ + frozenRowKeys: ["a", "b", "c"], + isAtBottom: false, + messages: rows("a", "b", "d"), + }).map(({ id }) => id), + ["a", "b"], + ); +}); + +test("keeps frozen rows once each when they come back reordered", () => { + assert.deepEqual( + selectBufferedTimelineMessages({ + frozenRowKeys: ["a", "b", "c"], + isAtBottom: false, + messages: rows("older", "b", "a", "c", "d"), + }).map(({ id }) => id), + ["older", "a", "b", "c"], + ); +}); + +test("keeps the tail frozen when a frozen optimistic row is acknowledged", () => { + assert.deepEqual( + selectBufferedTimelineMessages({ + frozenRowKeys: ["a", "pending-b"], + isAtBottom: false, + messages: [ + ...rows("a"), + { id: "b", renderKey: "pending-b" }, + ...rows("c"), + ], + }).map(({ id }) => id), + ["a", "b"], + ); +}); diff --git a/desktop/src/features/messages/ui/useBufferedTimelineMessages.ts b/desktop/src/features/messages/ui/useBufferedTimelineMessages.ts index 46b2a5a577e..952e25ede29 100644 --- a/desktop/src/features/messages/ui/useBufferedTimelineMessages.ts +++ b/desktop/src/features/messages/ui/useBufferedTimelineMessages.ts @@ -1,5 +1,12 @@ import * as React from "react"; +type TimelineRow = { id: string; renderKey?: string }; + +/** A row's identity, stable across an optimistic send and its relay ack. */ +function rowKey(message: TimelineRow) { + return message.renderKey ?? message.id; +} + /** * Keeps the logical tail stable while a reader is away from the bottom. * @@ -8,30 +15,37 @@ import * as React from "react"; * history before the retained tail is admitted immediately, while newer output * is released atomically when the reader returns to the bottom. */ -export function selectBufferedTimelineMessages({ - frozenMessageIds, +export function selectBufferedTimelineMessages({ + frozenRowKeys, isAtBottom, messages, }: { - frozenMessageIds: readonly string[] | null; + frozenRowKeys: readonly string[] | null; isAtBottom: boolean; messages: T[]; }): T[] { - if (isAtBottom || frozenMessageIds === null) return messages; - if (frozenMessageIds.length === 0) return []; + if (isAtBottom || frozenRowKeys === null) return messages; + if (frozenRowKeys.length === 0) return []; - const currentById = new Map(messages.map((message) => [message.id, message])); - if (frozenMessageIds.some((id) => !currentById.has(id))) { - // A deletion or authoritative replacement removed part of the frozen - // snapshot. Keeping stale message objects would be worse than accepting it. - return messages; - } + const currentByKey = new Map( + messages.map((message) => [rowKey(message), message]), + ); + // A deletion drops only its own row: the rest stays frozen, so later + // arrivals still count as pending. Once the whole snapshot is gone (an + // authoritative replacement), accept the new list rather than keep stale + // message objects. + const survivingKeys = frozenRowKeys.filter((key) => currentByKey.has(key)); + if (survivingKeys.length === 0) return messages; - const firstFrozenIndex = messages.findIndex( - (message) => message.id === frozenMessageIds[0], + // Frozen rows can come back reordered: an acked send changes its id and so + // its place among same-second rows. Older history ends at the first frozen + // row of any key, so no frozen row is also prepended and counted twice. + const frozenKeys = new Set(survivingKeys); + const firstFrozenIndex = messages.findIndex((message) => + frozenKeys.has(rowKey(message)), ); const prepended = messages.slice(0, firstFrozenIndex); - const frozen = frozenMessageIds.map((id) => currentById.get(id) as T); + const frozen = survivingKeys.map((key) => currentByKey.get(key) as T); const buffered = [...prepended, ...frozen]; if ( buffered.length === messages.length && @@ -46,7 +60,7 @@ export function selectBufferedTimelineMessages({ return buffered; } -export function useBufferedTimelineMessages({ +export function useBufferedTimelineMessages({ channelId, isAtBottom, messages, @@ -55,22 +69,22 @@ export function useBufferedTimelineMessages({ isAtBottom: boolean; messages: T[]; }): { messages: T[]; pendingCount: number } { - const frozenMessageIdsRef = React.useRef(null); + const frozenRowKeysRef = React.useRef(null); const previousChannelIdRef = React.useRef(channelId); if (previousChannelIdRef.current !== channelId) { previousChannelIdRef.current = channelId; - frozenMessageIdsRef.current = null; + frozenRowKeysRef.current = null; } if (isAtBottom) { - frozenMessageIdsRef.current = messages.map((message) => message.id); - } else if (frozenMessageIdsRef.current === null) { - frozenMessageIdsRef.current = messages.map((message) => message.id); + frozenRowKeysRef.current = messages.map(rowKey); + } else if (frozenRowKeysRef.current === null) { + frozenRowKeysRef.current = messages.map(rowKey); } const buffered = selectBufferedTimelineMessages({ - frozenMessageIds: frozenMessageIdsRef.current, + frozenRowKeys: frozenRowKeysRef.current, isAtBottom, messages, }); diff --git a/desktop/src/features/messages/useThreadReplies.test.mjs b/desktop/src/features/messages/useThreadReplies.test.mjs index ff5408905d2..e2f1cde2ab6 100644 --- a/desktop/src/features/messages/useThreadReplies.test.mjs +++ b/desktop/src/features/messages/useThreadReplies.test.mjs @@ -250,6 +250,37 @@ test("loadThreadReplies aggregates events across multiple pages", async () => { } }); +// ── Test: fetch and live append agree on same-second order ─────────────────── +// +// A live reply lands through mergeMessages (hooks.ts appendMessage), which +// re-sorts the whole thread-replies cache. If the fetch stored the pages in +// another same-second order, the fetched replies would swap on screen the +// moment that reply arrived. Storing the pages' ascending-id order as-is fails +// this test. + +test("loadThreadReplies orders same-second replies as a live append does", async () => { + const { loadThreadReplies } = await import("./useThreadReplies.ts"); + const { mergeMessages } = await import("./lib/messageMerge.ts"); + + const at = (id, createdAt) => ({ ...fakeEvent(id), created_at: createdAt }); + const sameSecond = [at("1".repeat(64), 100), at("2".repeat(64), 100)]; + const fetched = await loadThreadReplies( + makeQueryClient(), + "chan-order", + "root-order", + null, + new Set(), + async () => singlePage(sameSecond), + ); + + const live = at("3".repeat(64), 101); + assert.deepEqual( + mergeMessages(fetched, live).map((e) => e.id), + [...fetched.map((e) => e.id), live.id], + "a later live reply must not reorder the fetched same-second replies", + ); +}); + // ── Test: query-fn writes exhaustion on the terminal attempt ────────────────── // // Load-bearing for the query-fn exhaustion write in useThreadReplies. diff --git a/desktop/src/features/projects/lib/projectAgentConversation.ts b/desktop/src/features/projects/lib/projectAgentConversation.ts index 80d1c721777..471819112ad 100644 --- a/desktop/src/features/projects/lib/projectAgentConversation.ts +++ b/desktop/src/features/projects/lib/projectAgentConversation.ts @@ -32,7 +32,7 @@ export function projectAgentMembershipInput({ /** * True when `event` is the conversation opener or comes after it in the * timeline's `(created_at, event_id)` ordering (`compareRelayOrder` in - * `channelWindowStore.ts`). A bare timestamp cannot make this call — every + * `shared/lib/eventOrder.ts`). A bare timestamp cannot make this call — every * unrelated event sharing the opener's second would pass — which is why the * opener's exact event id participates. Id equality is checked first so the * opener itself is admitted even against a legacy persisted `createdAt` that diff --git a/desktop/src/shared/api/relayClientShared.test.mjs b/desktop/src/shared/api/relayClientShared.test.mjs index d7b56529906..5087865731e 100644 --- a/desktop/src/shared/api/relayClientShared.test.mjs +++ b/desktop/src/shared/api/relayClientShared.test.mjs @@ -28,8 +28,8 @@ test("sortEvents — same-second events sort by id, order-independent", () => { const forward = sortEvents([a, b, c]).map((e) => e.id); const shuffled = sortEvents([c, b, a]).map((e) => e.id); - // Stable (created_at, id) order regardless of input order, matching the - // cache sort (sortMessages) and the relay's id-ASC same-second tiebreak. + // Stable (created_at, id) order regardless of input order, using the + // relay's id-ASC same-second tiebreak. assert.deepEqual(forward, ["aaa", "bbb", "ccc"]); assert.deepEqual(shuffled, ["aaa", "bbb", "ccc"]); }); diff --git a/desktop/src/shared/api/relayClientShared.ts b/desktop/src/shared/api/relayClientShared.ts index 351b779f376..6ca774cd3ec 100644 --- a/desktop/src/shared/api/relayClientShared.ts +++ b/desktop/src/shared/api/relayClientShared.ts @@ -146,10 +146,9 @@ export function sortEvents(events: RelayEvent[]) { if (left.created_at !== right.created_at) { return left.created_at - right.created_at; } - // Same (created_at, id) tiebreak as the cache sort (sortMessages) so a - // history REQ resolves same-second events in a stable, relay-matching - // order. Currently every consumer re-sorts downstream, but keeping the - // two sorts on one invariant avoids a latent ordering drift. + // Tiebreak same-second events on ascending id so a history REQ resolves + // in a stable order; project issues replay assignment operations in it. + // Timeline consumers re-sort with compareChronologicalOrder. return left.id < right.id ? -1 : left.id > right.id ? 1 : 0; }); } diff --git a/desktop/src/shared/lib/eventOrder.ts b/desktop/src/shared/lib/eventOrder.ts new file mode 100644 index 00000000000..63cc312ab48 --- /dev/null +++ b/desktop/src/shared/lib/eventOrder.ts @@ -0,0 +1,26 @@ +type OrderedEvent = { created_at: number; id: string }; + +/** Relay order: newest timestamp first, then ascending id within a second. */ +export function compareRelayOrder(left: OrderedEvent, right: OrderedEvent) { + return left.created_at !== right.created_at + ? right.created_at - left.created_at + : left.id < right.id + ? -1 + : left.id > right.id + ? 1 + : 0; +} + +/** + * Timeline order, oldest first: the exact reverse of relay order, so a larger + * id sorts first within a second. Every timeline sort uses this one order so + * the live cache and the channel window never swap same-second rows, and an + * older page fetched from a mid-second cursor lands contiguously above the + * rows already shown. + */ +export function compareChronologicalOrder( + left: OrderedEvent, + right: OrderedEvent, +) { + return compareRelayOrder(right, left); +} diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 9191b3f7ab7..9fa7b42b2a2 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -1632,6 +1632,11 @@ declare global { __BUZZ_E2E_HOLD_USERS_BATCH__?: (hold: boolean) => number; /** Number of `get_users_batch` calls currently held. */ __BUZZ_E2E_USERS_BATCH_PENDING__?: () => number; + /** Start or stop holding older-page `get_channel_window` responses (any + * call with a cursor). Turning the hold off flushes everything held; + * either call returns the number released, so a spec can prove the page + * stayed in flight until it let go. */ + __BUZZ_E2E_HOLD_CHANNEL_WINDOW_PAGES__?: (hold: boolean) => number; /** Release every `get_profile` response held by `deferProfileReads`. */ __BUZZ_E2E_RELEASE_PROFILE_READS__?: () => number; /** Number of `get_profile` responses currently held. */ @@ -1776,6 +1781,10 @@ let profileReadsReleased = false; // second paste of the same label is provably still deciding). let holdUsersBatch = false; let heldUsersBatchReleases: Array<() => void> = []; +// Older channel-window pages held by `__BUZZ_E2E_HOLD_CHANNEL_WINDOW_PAGES__`, +// so a spec can finish driving scroll input before the fetched page exists. +let holdChannelWindowPages = false; +let heldChannelWindowPageReleases: Array<() => void> = []; // Starts currently held behind `startManagedAgentDelayMs`, releasable early // via `__BUZZ_E2E_RELEASE_MANAGED_AGENT_STARTS__()`: a spec that holds a // start across a community round-trip needs the hold long enough to be @@ -6011,6 +6020,11 @@ async function handleGetChannelWindow( if (args.cursor !== null) { probe.__CHANNEL_WINDOW_FETCH_COUNT__ = (probe.__CHANNEL_WINDOW_FETCH_COUNT__ ?? 0) + 1; + if (holdChannelWindowPages) { + await new Promise((resolve) => { + heldChannelWindowPageReleases.push(resolve); + }); + } } const delayMs = @@ -11458,6 +11472,8 @@ export function maybeInstallE2eTauriMocks() { profileReadsReleased = false; holdUsersBatch = false; heldUsersBatchReleases = []; + holdChannelWindowPages = false; + heldChannelWindowPageReleases = []; cancelledMediaUploadIds = new Set(); for (const controller of mockMediaFetchControllers.values()) { controller.abort(); @@ -11503,6 +11519,12 @@ export function maybeInstallE2eTauriMocks() { return queued.length; }; window.__BUZZ_E2E_USERS_BATCH_PENDING__ = () => heldUsersBatchReleases.length; + window.__BUZZ_E2E_HOLD_CHANNEL_WINDOW_PAGES__ = (hold: boolean) => { + holdChannelWindowPages = hold; + const queued = hold ? [] : heldChannelWindowPageReleases.splice(0); + for (const release of queued) release(); + return queued.length; + }; mockGlobalAgentConfig = config.mock?.globalAgentConfig ? { ...config.mock.globalAgentConfig } : null; diff --git a/desktop/tests/e2e/channel-browser.spec.ts b/desktop/tests/e2e/channel-browser.spec.ts index b60453fc4f9..06ff426d365 100644 --- a/desktop/tests/e2e/channel-browser.spec.ts +++ b/desktop/tests/e2e/channel-browser.spec.ts @@ -413,6 +413,59 @@ test("Enter with no matches jumps to create", async ({ page }) => { ); }); +// Type and press Enter in one task. React commits the urgent render on the +// microtask queue, but the deferred list re-renders in a later task, so Enter +// lands while the list still shows the previous (unfiltered) results. +async function typeThenEnterBeforeDeferredRender(page: Page, value: string) { + await page + .getByTestId("channel-browser-search") + .evaluate(async (el, text) => { + const input = el as HTMLInputElement; + Object.getOwnPropertyDescriptor( + HTMLInputElement.prototype, + "value", + )?.set?.call(input, text); + input.dispatchEvent(new Event("input", { bubbles: true })); + for (let i = 0; i < 5; i += 1) await Promise.resolve(); + input.dispatchEvent( + new KeyboardEvent("keydown", { + bubbles: true, + cancelable: true, + key: "Enter", + }), + ); + }, value); +} + +test("Enter before the list catches up still creates a new name", async ({ + page, +}) => { + const channelName = `enter-fast-${Date.now()}`; + + await page.goto("/"); + + await openChannelBrowser(page); + await expect(page.getByTestId("browse-channel-agents")).toBeVisible(); + await typeThenEnterBeforeDeferredRender(page, channelName); + + await expect(page.getByTestId("create-channel-name")).toHaveValue( + channelName, + ); +}); + +test("Enter before the list catches up still opens the typed match", async ({ + page, +}) => { + await page.goto("/"); + + await openChannelBrowser(page); + await expect(page.getByTestId("browse-channel-agents")).toBeVisible(); + await typeThenEnterBeforeDeferredRender(page, "desig"); + + await expect(page.getByTestId("channel-browser-dialog")).not.toBeVisible(); + await expect(page.getByTestId("chat-title")).toHaveText("design"); +}); + test("arrow keys reach the pinned create row and Enter activates it", async ({ page, }) => { diff --git a/desktop/tests/e2e/channel-dense-second-reach.spec.ts b/desktop/tests/e2e/channel-dense-second-reach.spec.ts index f588c971fca..bc7e6d999c8 100644 --- a/desktop/tests/e2e/channel-dense-second-reach.spec.ts +++ b/desktop/tests/e2e/channel-dense-second-reach.spec.ts @@ -190,3 +190,88 @@ test("dense single second beyond one window page is fully reachable via composit Array.from({ length: DENSE_COUNT }, (_, index) => index), ); }); + +// The live cache merge and the channel-window projection both write the +// rendered timeline. They must tie-break a shared second identically, or the +// pair flips as each write lands (closing any menu open on either row). +test("same-second live messages render in one order and never swap", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + await expect + .poll(() => + page.evaluate( + () => + window.__BUZZ_E2E_HAS_MOCK_LIVE_SUBSCRIPTION__?.({ + channelName: "general", + kind: 9, + exactChannel: true, + }) ?? false, + ), + ) + .toBe(true); + + const low = "1".repeat(64); + const high = "f".repeat(64); + // Record every order the pair is ever committed in. The observer runs right + // after each DOM commit, before any later task, so the read below has seen + // the final commit. Sampling on animation frames missed it whenever the read + // beat the next frame. + await page.evaluate( + ({ low, high }) => { + const orders: string[] = []; + Object.assign(window, { __SAME_SECOND_ORDERS__: orders }); + new MutationObserver(() => { + const order = [...document.querySelectorAll("[data-message-id]")] + .map((row) => row.getAttribute("data-message-id")) + .filter((id) => id === low || id === high) + .join(","); + if (order.includes(",") && orders.at(-1) !== order) orders.push(order); + }).observe(document.body, { + attributeFilter: ["data-message-id"], + childList: true, + subtree: true, + }); + }, + { low, high }, + ); + const createdAt = Math.floor(Date.now() / 1000); + for (const id of [low, high]) { + await page.evaluate( + ({ createdAt, id }) => + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: `same second ${id[0]}`, + createdAt, + id, + }), + { createdAt, id }, + ); + } + // Live events apply in order, so once a later message renders, both writes + // for the pair have landed. + await page.evaluate( + (createdAt) => + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: "one second later", + createdAt: createdAt + 1, + }), + createdAt, + ); + await expect( + page.getByTestId("message-timeline").getByText("one second later"), + ).toBeVisible(); + + // Within a second the larger id is older: the reverse of relay order. + expect( + await page.evaluate( + () => + (window as Window & { __SAME_SECOND_ORDERS__?: string[] }) + .__SAME_SECOND_ORDERS__, + ), + ).toEqual([`${high},${low}`]); +}); diff --git a/desktop/tests/e2e/empty-edit-delete.spec.ts b/desktop/tests/e2e/empty-edit-delete.spec.ts index 246cba89a4d..67c710f8937 100644 --- a/desktop/tests/e2e/empty-edit-delete.spec.ts +++ b/desktop/tests/e2e/empty-edit-delete.spec.ts @@ -151,3 +151,42 @@ test("a non-empty edit still edits and never deletes", async ({ page }) => { RENDERED_ORIGINAL_CONTENT, ); }); + +// Clearing a draft must close its autocomplete at once. An Enter inside the +// 120 ms debounce used to pick the stale suggestion instead of submitting, so +// the delete prompt never appeared. The draft's trailing `#general` opens the +// channel list on its own; the other triggers are typed after it. +for (const { trigger, typed, popover } of [ + { trigger: "#channel", typed: "", popover: "channel-autocomplete" }, + { trigger: "@mention", typed: " @al", popover: "mention-autocomplete" }, + { trigger: ":emoji", typed: " :smi", popover: "emoji-autocomplete" }, +]) { + test(`an empty edit submits while the ${trigger} autocomplete is open`, async ({ + page, + }) => { + const row = page.locator(`[data-message-id="${OWN_MESSAGE_ID}"]`); + await expect(row).toBeVisible({ timeout: 10_000 }); + await openMoreActionsMenu(page, OWN_MESSAGE_ID); + await page.getByTestId(`edit-message-${OWN_MESSAGE_ID}`).click(); + await expect(page.getByTestId("edit-target")).toBeVisible({ + timeout: 5_000, + }); + // The channel list opens only once the caret has landed after `#general`. + await expect(page.getByTestId("channel-autocomplete")).toBeVisible({ + timeout: 5_000, + }); + await page.keyboard.type(typed); + await expect(page.getByTestId(popover)).toBeVisible({ timeout: 5_000 }); + + // Clear and submit back-to-back, well inside the debounce. + await page.keyboard.press("ControlOrMeta+A"); + await page.keyboard.press("Backspace"); + await page.keyboard.press("Enter"); + + await expect(page.getByRole("alertdialog")).toContainText( + "Delete message?", + { timeout: 5_000 }, + ); + await expect(page.getByTestId("message-input")).toBeEmpty(); + }); +} diff --git a/desktop/tests/e2e/video-attachment.spec.ts b/desktop/tests/e2e/video-attachment.spec.ts index f2741733794..92c6cae05e5 100644 --- a/desktop/tests/e2e/video-attachment.spec.ts +++ b/desktop/tests/e2e/video-attachment.spec.ts @@ -1,4 +1,4 @@ -import { expect, type Page, test } from "@playwright/test"; +import { expect, type Locator, type Page, test } from "@playwright/test"; import { installMockBridge } from "../helpers/bridge"; import { expectCornerRadiusPx, expectSmoothCorners } from "../helpers/css"; @@ -64,15 +64,20 @@ function emitMockMessage( page: Page, channelName: string, content: string, - options: { extraTags?: string[][]; parentEventId?: string } = {}, + options: { + createdAt?: number; + extraTags?: string[][]; + parentEventId?: string; + } = {}, ) { return page.evaluate( - ({ channelName, content, extraTags, parentEventId }) => { + ({ channelName, content, createdAt, extraTags, parentEventId }) => { const emit = ( window as Window & { __BUZZ_E2E_EMIT_MOCK_MESSAGE__?: (input: { channelName: string; content: string; + createdAt?: number; extraTags?: string[][]; parentEventId?: string; }) => unknown; @@ -81,11 +86,18 @@ function emitMockMessage( if (!emit) { throw new Error("Mock message emitter is unavailable."); } - return emit({ channelName, content, extraTags, parentEventId }); + return emit({ + channelName, + content, + createdAt, + extraTags, + parentEventId, + }); }, { channelName, content, + createdAt: options.createdAt, extraTags: options.extraTags, parentEventId: options.parentEventId, }, @@ -1453,9 +1465,15 @@ test("dark accent uses a contrast-safe review foreground", async ({ page }) => { // player and, for the relay case, a resolvable download URL. function emitVideoMessage( page: Page, - { url, sha, filename }: { url: string; sha: string; filename: string }, + { + url, + sha, + filename, + createdAt, + }: { url: string; sha: string; filename: string; createdAt?: number }, ) { return emitMockMessage(page, "general", `![video](${url})`, { + createdAt, extraTags: [ [ "imeta", @@ -1472,6 +1490,31 @@ function emitVideoMessage( }); } +// A live row reaches the timeline floor in two scroll steps: first to the +// virtualizer's estimated floor, which leaves a video row hanging under the +// composer overlay, then to its measured floor. Right-click only once the +// player clears the composer. Earlier, the click lands on the composer, and +// the second step's scroll dismisses any menu that did open. +async function rightClickSettledPlayer(player: Locator) { + await expect + .poll(() => + player.evaluate((element) => { + const composer = document.querySelector( + '[data-testid="message-composer"]', + ); + return ( + composer !== null && + element.getBoundingClientRect().bottom <= + composer.getBoundingClientRect().top + ); + }), + ) + .toBe(true); + // No `force`: the Play button is inside the player, so the hit-target check + // passes, and the contextmenu still reaches the surface's capture handler. + await player.click({ button: "right" }); +} + test("right-click menus expose distinct selectors for links, relay video, and off-relay video", async ({ page, }) => { @@ -1481,12 +1524,15 @@ test("right-click menus expose distinct selectors for links, relay video, and of await page.getByTestId("channel-general").click(); await expect(page.getByTestId("chat-title")).toHaveText("general"); await waitForMockLiveSubscription(page, "general"); + // One second per message keeps the three rows in a fixed order. + const createdAt = Math.floor(Date.now() / 1000); // ── Link menu: Open link + Copy link, never image/video attributes ──────── await emitMockMessage( page, "general", "Docs at https://example.com/handbook", + { createdAt }, ); const link = page.getByRole("link", { name: "https://example.com/handbook" }); await expect(link).toBeVisible(); @@ -1523,15 +1569,13 @@ test("right-click menus expose distinct selectors for links, relay video, and of url: MENU_RELAY_VIDEO_URL, sha: MENU_RELAY_VIDEO_SHA, filename: "relay-clip.mp4", + createdAt: createdAt + 1, })) as { id: string }; const relayPlayer = page .locator(`[data-message-id="${relayMessage.id}"]`) .getByTestId("video-player"); await expect(relayPlayer).toBeVisible(); - // Right-click the player surface. `force` skips the actionability guard: the - // Play-button overlay sits above the video, but the contextmenu event still - // capture-bubbles to the surface handler that opens the menu. - await relayPlayer.click({ button: "right", force: true }); + await rightClickSettledPlayer(relayPlayer); const videoMenu = page.locator("[data-video-context-menu]"); await expect(videoMenu).toBeVisible(); @@ -1571,12 +1615,13 @@ test("right-click menus expose distinct selectors for links, relay video, and of url: MENU_OFF_RELAY_VIDEO_URL, sha: MENU_OFF_RELAY_VIDEO_SHA, filename: "external-clip.mp4", + createdAt: createdAt + 2, })) as { id: string }; const offRelayPlayer = page .locator(`[data-message-id="${offRelayMessage.id}"]`) .getByTestId("video-player"); await expect(offRelayPlayer).toBeVisible(); - await offRelayPlayer.click({ button: "right", force: true }); + await rightClickSettledPlayer(offRelayPlayer); const offRelayMenu = page.locator("[data-video-context-menu]"); await expect(offRelayMenu).toBeVisible(); diff --git a/desktop/tests/e2e/virtualization.spec.ts b/desktop/tests/e2e/virtualization.spec.ts index 7a858d8ebe9..ffb9d7d1cd2 100644 --- a/desktop/tests/e2e/virtualization.spec.ts +++ b/desktop/tests/e2e/virtualization.spec.ts @@ -2,7 +2,10 @@ import { expect, test } from "@playwright/test"; import type { Locator, Page } from "@playwright/test"; import { installMockBridge } from "../helpers/bridge"; -import { waitForMockChannelHeadReady } from "../helpers/channelHeadReady"; +import { + openGeneral, + waitForMockChannelHeadReady, +} from "../helpers/channelHeadReady"; const WATERCOOLER_CHANNEL_ID = "a27e1ee9-76a6-5bdf-a5d5-1d85610dad11"; const FORUM_THREAD_ID = "mock-forum-release-thread"; @@ -268,13 +271,10 @@ test.describe("list virtualization", () => { // reproduce Chromium/WebKit's native wheel → scroll callback ordering. The // old boundary rollback moved the viewport back down before the fetch // committed; keep that pre-prepend reversal below the same 5px frame bar. - // A 300ms relay delay leaves the input boundary and prepend commit as two - // distinct phases so this assertion cannot accidentally measure only the - // later anchor correction. - await installMockBridge(page, { - deepHistoryMessageCount: 1_800, - channelWindowDelayMs: 300, - }); + // Each older page stays in flight until the burst has been traced, so the + // input boundary and prepend commit are two distinct phases and this + // assertion cannot accidentally measure only the later anchor correction. + await installMockBridge(page, { deepHistoryMessageCount: 1_800 }); await page.goto("/#/channels/feedf00d-0000-4000-8000-000000000007"); const timeline = page.getByTestId("message-timeline"); await expect(timeline.locator("[data-message-id]").first()).toBeVisible(); @@ -322,6 +322,12 @@ test.describe("list virtualization", () => { element.scrollTop = 4000; }); await page.waitForTimeout(300); + // Hold the page this crossing fetches. A slow runner can pause between + // wheels for longer than the settle gate's quiet window, and the gate + // would then rightly commit a page that had already arrived mid-burst. + await page.evaluate(() => + window.__BUZZ_E2E_HOLD_CHANNEL_WINDOW_PAGES__?.(true), + ); await timeline.evaluate((element) => { element.scrollTop = 180; }); @@ -353,8 +359,8 @@ test.describe("list virtualization", () => { // CDP can deliver a wheel burst late (Linux Chromium more than macOS), // so a window timed from before the burst can close before the input // lands. Time the 120 ms window from the first wheel the page receives - // and keep it open until every wheel has arrived. Stop early at the - // prepend commit: its anchor correction is not a boundary rollback. + // and keep it open until every wheel has arrived. The held page cannot + // commit meanwhile, so no anchor correction is read as a rollback. const upDeltas = [-60, -30, -20, -15]; const wheelTracePromise = timeline.evaluate( async (scroller, wheelCount) => { @@ -364,14 +370,12 @@ test.describe("list virtualization", () => { if (!event.ctrlKey) received += 1; }; s.addEventListener("wheel", countWheel, { passive: true }); - const startHeight = s.scrollHeight; let previousScrollTop = s.scrollTop; let maxBoundaryRollback = 0; let minScrollTop = s.scrollTop; let firstWheelAt: number | null = null; const giveUpAt = performance.now() + 5_000; while (performance.now() < giveUpAt) { - if (s.scrollHeight > startHeight + 800) break; if (firstWheelAt === null && received > 0) { firstWheelAt = performance.now(); } @@ -426,6 +430,10 @@ test.describe("list virtualization", () => { const motion = await timeline.evaluate( async (scroller, { anchorId, anchorTop, oldHeight }) => { const s = scroller as HTMLElement; + // Let the held page go only once this trace is running, so it sees + // every frame of the commit against a baseline taken before it. + const released = + window.__BUZZ_E2E_HOLD_CHANNEL_WINDOW_PAGES__?.(false) ?? 0; let maxDrift = 0; let sawPrepend = false; let sawAnchorAfterPrepend = false; @@ -453,7 +461,7 @@ test.describe("list virtualization", () => { if (sawAnchorAfterPrepend && stableFrames >= 8) break; await new Promise((resolve) => requestAnimationFrame(resolve)); } - return { maxDrift, sawPrepend }; + return { maxDrift, released, sawPrepend }; }, { anchorId: committedAnchor.id, @@ -461,6 +469,7 @@ test.describe("list virtualization", () => { oldHeight: before.scrollHeight, }, ); + expect(motion.released).toBe(1); expect(motion.sawPrepend).toBe(true); if (pageIndex === 0) expect(await ctrlWheelPromise).toBe(true); expect(motion.maxDrift).toBeLessThan(5); @@ -1004,6 +1013,70 @@ test("live tail arrivals stay buffered while reading and release on jump", async await expect(page.getByText("buffered live tail sentinel")).toBeVisible(); }); +test("deleting a row while reading keeps live arrivals counted", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + const newestSeedId = await page.evaluate(() => { + let newest = ""; + for (let index = 0; index < 20; index += 1) { + newest = + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: `delete-while-reading seed ${index}\nsecond line ${index}`, + createdAt: 1_700_000_000 + index, + })?.id ?? ""; + } + return newest; + }); + // The deletion and the arrival below are live events, so live admission and + // its head refresh must land before reading starts. + await openGeneral(page); + const timeline = page.getByTestId("message-timeline"); + await expect(timeline).toContainText("delete-while-reading seed 19"); + + await timeline.evaluate((element) => { + element.dispatchEvent(new WheelEvent("wheel", { deltaY: -1 })); + element.scrollTop -= 300; + }); + const pill = page.getByTestId("message-scroll-to-latest"); + await expect(pill).toBeVisible(); + + // Removing a frozen row used to drop the whole freeze, so the next arrival + // joined the tail uncounted and the pill read "Jump to latest". + await page.evaluate((target) => { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: "", + kind: 5, + extraTags: [["e", target]], + }); + }, newestSeedId); + await expect(page.locator(`[data-message-id="${newestSeedId}"]`)).toHaveCount( + 0, + ); + await page.evaluate(() => { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: "delete-while-reading live sentinel", + createdAt: 1_800_000_000, + }); + }); + + await expect(pill).toContainText("1 new message"); + await expect( + page.getByText("delete-while-reading live sentinel"), + ).toHaveCount(0); + await pill.click(); + await expect( + page.getByText("delete-while-reading live sentinel"), + ).toBeVisible(); +}); + test("a single scroll back to the bottom releases the live tail", async ({ page, }) => { diff --git a/desktop/tests/e2e/voice-note.spec.ts b/desktop/tests/e2e/voice-note.spec.ts index c9e6ee17755..814bbf3051f 100644 --- a/desktop/tests/e2e/voice-note.spec.ts +++ b/desktop/tests/e2e/voice-note.spec.ts @@ -910,35 +910,42 @@ test("starting a duplicate voice-note player pauses the other instance", async ( await page.getByTestId("channel-general").click(); await waitForMockLiveSubscription(page, "general"); - await page.evaluate( - ({ audioUrl }) => { + // Two messages share one audio URL. Distinct seconds fix their order, and + // each card is located through its own message row, never by index. + const [firstId, secondId] = await page.evaluate( + ({ audioUrl, createdAt }) => { const emit = ( window as Window & { __BUZZ_E2E_EMIT_MOCK_MESSAGE__?: (input: { channelName: string; content: string; + createdAt: number; extraTags: string[][]; - }) => unknown; + }) => { id: string } | undefined; } ).__BUZZ_E2E_EMIT_MOCK_MESSAGE__; if (!emit) throw new Error("Mock message emitter is unavailable."); - const input = { - channelName: "general", - content: `[voice-note-123.mp4](${audioUrl})`, - extraTags: [ - [ - "imeta", - `url ${audioUrl}`, - "m video/mp4", - "duration 9.4", - "filename voice-note-123.mp4", + return [0, 1].map((offset) => { + const filename = `voice-note-12${offset}.mp4`; + const emitted = emit({ + channelName: "general", + content: `[${filename}](${audioUrl})`, + createdAt: createdAt + offset, + extraTags: [ + [ + "imeta", + `url ${audioUrl}`, + "m video/mp4", + "duration 9.4", + `filename ${filename}`, + ], ], - ], - }; - emit(input); - emit(input); + }); + if (!emitted?.id) throw new Error("mock message emit returned no id"); + return emitted.id; + }); }, - { audioUrl: AUDIO_URL }, + { audioUrl: AUDIO_URL, createdAt: Math.floor(Date.now() / 1000) }, ); const cards = page.getByTestId("audio-message-attachment"); @@ -948,12 +955,18 @@ test("starting a duplicate voice-note player pauses the other instance", async ( await cards.locator("audio").evaluateAll((audios) => { for (const audio of audios) (audio as HTMLAudioElement).loop = true; }); - const firstAudio = cards.nth(0).locator("audio"); - const secondAudio = cards.nth(1).locator("audio"); - - await cards.nth(0).getByRole("button", { name: "Play voice note" }).click(); + const firstCard = page + .locator(`[data-message-id="${firstId}"]`) + .getByTestId("audio-message-attachment"); + const secondCard = page + .locator(`[data-message-id="${secondId}"]`) + .getByTestId("audio-message-attachment"); + const firstAudio = firstCard.locator("audio"); + const secondAudio = secondCard.locator("audio"); + + await firstCard.getByRole("button", { name: "Play voice note" }).click(); await expect(firstAudio).toHaveJSProperty("paused", false); - await cards.nth(1).getByRole("button", { name: "Play voice note" }).click(); + await secondCard.getByRole("button", { name: "Play voice note" }).click(); await expect(firstAudio).toHaveJSProperty("paused", true); await expect(secondAudio).toHaveJSProperty("paused", false); });