From 117949f9175aa2d235547b1238457d55c694a9c4 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Mon, 6 Apr 2026 12:46:46 -0400 Subject: [PATCH 01/17] feat(bookmarks): add message bookmarks (MSC4438) --- config.json | 9 + src/app/features/bookmarks/bookmarkDomain.ts | 171 ++++++ .../features/bookmarks/bookmarkRepository.ts | 147 +++++ src/app/features/bookmarks/useBookmarks.ts | 78 +++ .../features/bookmarks/useInitBookmarks.ts | 70 +++ src/app/features/room/message/Message.tsx | 55 ++ .../settings/experimental/Experimental.tsx | 2 + .../experimental/MSC4438MessageBookmarks.tsx | 56 ++ src/app/hooks/router/useHomeSelected.ts | 11 + src/app/hooks/router/useInbox.ts | 17 +- src/app/hooks/useClientConfig.ts | 67 +++ src/app/pages/Router.tsx | 4 + src/app/pages/client/ClientNonUIFeatures.tsx | 7 + .../pages/client/bookmarks/BookmarksList.tsx | 560 ++++++++++++++++++ src/app/pages/client/bookmarks/index.ts | 1 + src/app/pages/client/home/Home.tsx | 162 ++--- src/app/pages/client/inbox/Inbox.tsx | 44 +- src/app/pages/pathUtils.ts | 4 + src/app/pages/paths.ts | 3 + src/app/state/bookmarks.ts | 20 + src/app/state/settings.ts | 6 + src/types/matrix/accountData.ts | 5 + 22 files changed, 1429 insertions(+), 70 deletions(-) create mode 100644 src/app/features/bookmarks/bookmarkDomain.ts create mode 100644 src/app/features/bookmarks/bookmarkRepository.ts create mode 100644 src/app/features/bookmarks/useBookmarks.ts create mode 100644 src/app/features/bookmarks/useInitBookmarks.ts create mode 100644 src/app/features/settings/experimental/MSC4438MessageBookmarks.tsx create mode 100644 src/app/pages/client/bookmarks/BookmarksList.tsx create mode 100644 src/app/pages/client/bookmarks/index.ts create mode 100644 src/app/state/bookmarks.ts diff --git a/config.json b/config.json index f0c3c8b61..4f3dae570 100644 --- a/config.json +++ b/config.json @@ -19,6 +19,15 @@ "enabled": true }, + "experiments": { + "messageBookmarks": { + "enabled": false, + "rolloutPercentage": 0, + "controlVariant": "control", + "variants": ["enabled"] + } + }, + "featuredCommunities": { "openAsDefault": false, "spaces": [ diff --git a/src/app/features/bookmarks/bookmarkDomain.ts b/src/app/features/bookmarks/bookmarkDomain.ts new file mode 100644 index 000000000..15da412c9 --- /dev/null +++ b/src/app/features/bookmarks/bookmarkDomain.ts @@ -0,0 +1,171 @@ +/** + * MSC4438: Message bookmarks via account data + * https://github.com/matrix-org/matrix-spec-proposals/pull/4438 + * + * Unstable event type names in use (will migrate to stable names once MSC is accepted): + * m.bookmarks.index → org.matrix.msc4438.bookmarks.index + * m.bookmark. → org.matrix.msc4438.bookmark. + * + * Bookmark ID algorithm: djb2-like 32-bit hash over "|", prefixed with "bmk_". + * This matches the reference implementation in smokku/cinny commit 6363e441 and is used here for + * cross-client interoperability. If the algorithm ever changes, a migration must be provided so + * that existing bookmarks can have their IDs recomputed (the ID is stored in the item event, so + * old items remain accessible). + */ + +import { MatrixEvent, Room } from '$types/matrix-sdk'; +import { AccountDataEvent } from '$types/matrix/accountData'; + +export type BookmarkIndexContent = { + version: 1; + revision: number; + updated_ts: number; + bookmark_ids: string[]; +}; + +export type BookmarkItemContent = { + version: 1; + bookmark_id: string; + uri: string; + room_id: string; + event_id: string; + event_ts: number; + bookmarked_ts: number; + sender?: string; + room_name?: string; + body_preview?: string; + msgtype?: string; + deleted?: boolean; +}; + +/** + * Compute a bookmark ID for a (roomId, eventId) pair using the reference + * djb2-style algorithm agreed upon with the Cinny proof-of-concept. + * + * Input string: "|" + * Algorithm: For each UTF-16 code unit ch, hash = ((hash << 5) - hash + ch) | 0 + * Output: "bmk_" + unsigned 32-bit hex, zero-padded to 8 chars + * + * NOTE: If this algorithm is ever changed, a migration helper must be written + * so that existing bookmarked items (whose IDs are stored on the server as + * account data event-type suffixes) can still be resolved. The bookmark_id + * field inside each item event is the canonical reference. + */ +export function computeBookmarkId(roomId: string, eventId: string): string { + const input = `${roomId}|${eventId}`; + let hash = 0; + for (let i = 0; i < input.length; i += 1) { + const ch = input.charCodeAt(i); + // eslint-disable-next-line no-bitwise + hash = ((hash << 5) - hash + ch) | 0; + } + // Convert to unsigned 32-bit integer and encode as 8-char lowercase hex + // eslint-disable-next-line no-bitwise + const hex = (hash >>> 0).toString(16).padStart(8, '0'); + return `bmk_${hex}`; +} + +/** Construct the account data event type for a bookmark item. */ +export function bookmarkItemEventType(bookmarkId: string): string { + return `${AccountDataEvent.BookmarkItemPrefix}${bookmarkId}`; +} + +/** + * Build a matrix: URI for a room event. + * Canonical form: matrix:roomid//e/ + * (MSC4438 §Matrix URI) + */ +export function buildMatrixURI(roomId: string, eventId: string): string { + return `matrix:roomid/${encodeURIComponent(roomId)}/e/${encodeURIComponent(eventId)}`; +} + +const BODY_PREVIEW_MAX_LENGTH = 120; + +/** + * Extract a short preview of the event body for display in the bookmark list. + * Truncated to 120 chars with an ellipsis (MSC4438 §Body preview). + * + * Security: preview is only used as plain text in the UI, never parsed as HTML. + * Encrypted-room callers may choose to pass an empty string to avoid leaking + * plaintext into unencrypted account data (MSC4438 §Security considerations). + */ +export function extractBodyPreview( + mEvent: MatrixEvent, + maxLength = BODY_PREVIEW_MAX_LENGTH +): string { + const content = mEvent.getContent(); + const body = content?.body; + if (typeof body !== 'string' || body.length === 0) return ''; + if (body.length <= maxLength) return body; + return `${body.slice(0, maxLength)}\u2026`; +} + +/** + * Build a BookmarkItemContent from a room and event. + * + * Security: optional metadata (sender, room_name, body_preview) is copied into + * unencrypted account data. For encrypted rooms the caller may choose to omit + * these fields, storing only the required fields (room_id, event_id, uri). + * Currently we always populate them for usability; future work could honour a + * "privacy mode" setting. + */ +export function createBookmarkItem( + room: Room, + mEvent: MatrixEvent +): BookmarkItemContent | undefined { + const eventId = mEvent.getId(); + const { roomId } = room; + if (!eventId) return undefined; + + const bookmarkId = computeBookmarkId(roomId, eventId); + + return { + version: 1, + bookmark_id: bookmarkId, + uri: buildMatrixURI(roomId, eventId), + room_id: roomId, + event_id: eventId, + event_ts: mEvent.getTs(), + bookmarked_ts: Date.now(), + sender: mEvent.getSender() ?? undefined, + room_name: room.name, + body_preview: extractBodyPreview(mEvent), + msgtype: mEvent.getContent()?.msgtype, + }; +} + +// Validators (MSC4438: clients must validate before use) +export function isValidIndexContent(content: unknown): content is BookmarkIndexContent { + if (typeof content !== 'object' || content === null) return false; + const c = content as Record; + return ( + c.version === 1 && + typeof c.revision === 'number' && + typeof c.updated_ts === 'number' && + Array.isArray(c.bookmark_ids) && + (c.bookmark_ids as unknown[]).every((id) => typeof id === 'string') + ); +} + +export function isValidBookmarkItem(content: unknown): content is BookmarkItemContent { + if (typeof content !== 'object' || content === null) return false; + const c = content as Record; + return ( + c.version === 1 && + typeof c.bookmark_id === 'string' && + typeof c.uri === 'string' && + typeof c.room_id === 'string' && + typeof c.event_id === 'string' && + typeof c.event_ts === 'number' && + typeof c.bookmarked_ts === 'number' + ); +} + +export function emptyIndex(): BookmarkIndexContent { + return { + version: 1, + revision: 0, + updated_ts: Date.now(), + bookmark_ids: [], + }; +} diff --git a/src/app/features/bookmarks/bookmarkRepository.ts b/src/app/features/bookmarks/bookmarkRepository.ts new file mode 100644 index 000000000..d6703777f --- /dev/null +++ b/src/app/features/bookmarks/bookmarkRepository.ts @@ -0,0 +1,147 @@ +/** + * Bookmark repository: low-level read/write operations against Matrix account data. + * + * All writes follow the MSC4438 ordering guarantee: + * item is written first → index is updated second + * This ensures that when other devices receive the updated index via /sync, the + * referenced item event is already available. + */ + +import { MatrixClient } from '$types/matrix-sdk'; +import { AccountDataEvent } from '$types/matrix/accountData'; +import { + BookmarkIndexContent, + BookmarkItemContent, + bookmarkItemEventType, + emptyIndex, + isValidBookmarkItem, + isValidIndexContent, +} from './bookmarkDomain'; + +// Internal helpers +function readIndex(mx: MatrixClient): BookmarkIndexContent { + const evt = mx.getAccountData(AccountDataEvent.BookmarksIndex as any); + const content = evt?.getContent(); + if (isValidIndexContent(content)) return content; + return emptyIndex(); +} + +function readItem(mx: MatrixClient, bookmarkId: string): BookmarkItemContent | undefined { + const evt = mx.getAccountData(bookmarkItemEventType(bookmarkId) as any); + const content = evt?.getContent(); + // Must be valid and not tombstoned (MSC4438 §Listing bookmarks) + if (isValidBookmarkItem(content) && !content.deleted) return content; + return undefined; +} + +async function writeIndex(mx: MatrixClient, index: BookmarkIndexContent): Promise { + await mx.setAccountData(AccountDataEvent.BookmarksIndex as any, index as any); +} + +async function writeItem(mx: MatrixClient, item: BookmarkItemContent): Promise { + await mx.setAccountData(bookmarkItemEventType(item.bookmark_id) as any, item as any); +} + +// Public API +/** + * Add a bookmark. + * + * MSC4438 §Adding a bookmark: + * 1. Write the item event first. + * 2. Prepend the ID to bookmark_ids (if not already present). + * 3. Increment revision and update timestamp. + * 4. Write the updated index. + */ +export async function addBookmark(mx: MatrixClient, item: BookmarkItemContent): Promise { + // Write item before updating index (cross-device consistency) + await writeItem(mx, item); + + const index = readIndex(mx); + if (!index.bookmark_ids.includes(item.bookmark_id)) { + index.bookmark_ids.unshift(item.bookmark_id); + } + index.revision += 1; + index.updated_ts = Date.now(); + await writeIndex(mx, index); +} + +/** + * Remove a bookmark. + * + * MSC4438 §Removing a bookmark: + * 1. Remove the ID from the index. + * 2. Soft-delete the item (set deleted: true). + * + * Account data events cannot be deleted from the server, so soft-deletion is + * used. Other clients that encounter the item event can see it is explicitly + * removed. + */ +export async function removeBookmark(mx: MatrixClient, bookmarkId: string): Promise { + const index = readIndex(mx); + index.bookmark_ids = index.bookmark_ids.filter((id) => id !== bookmarkId); + index.revision += 1; + index.updated_ts = Date.now(); + await writeIndex(mx, index); + + // Soft-delete the item event + const existing = readItem(mx, bookmarkId); + if (existing) { + await writeItem(mx, { ...existing, deleted: true }); + } +} + +/** + * List all active bookmarks in index order, with orphan recovery. + * + * MSC4438 §Listing bookmarks: + * - Iterates bookmark_ids in order. + * - Skips missing, malformed, or tombstoned items. + * - Deduplicates by first occurrence. + * + * Orphan recovery: also scans the in-memory account data store for bookmark + * item events that exist but are absent from the index. These arise when two + * devices concurrently write the index (last-write-wins drops the other + * device's new bookmark_id while the item event itself persists). Orphaned + * items are appended after the index-ordered items. + */ +export function listBookmarks(mx: MatrixClient): BookmarkItemContent[] { + const index = readIndex(mx); + const seen = new Set(); + + const items = index.bookmark_ids + .filter((id) => { + if (seen.has(id)) return false; + seen.add(id); + return true; + }) + .map((id) => readItem(mx, id)) + .filter((item): item is BookmarkItemContent => item != null); + + // Walk the in-memory account data store for orphaned item events. + const prefix = AccountDataEvent.BookmarkItemPrefix as string; + Array.from(mx.store.accountData.keys()).forEach((key) => { + if (!key.startsWith(prefix)) return; + const bookmarkId = key.slice(prefix.length); + if (seen.has(bookmarkId)) return; + const item = readItem(mx, bookmarkId); + if (item) { + seen.add(bookmarkId); + items.push(item); + } + }); + + return items; +} + +/** + * Check whether a specific bookmark ID is in the index. + * + * NOTE: Do not rely on the bookmark ID being deterministically derivable from + * (roomId, eventId) for this check — different clients may use different + * algorithms. Use the bookmarkIdSet atom (derived from the live list) for + * O(1) per-message checks instead. + */ +export function isBookmarked(mx: MatrixClient, bookmarkId: string): boolean { + const index = readIndex(mx); + return index.bookmark_ids.includes(bookmarkId); +} diff --git a/src/app/features/bookmarks/useBookmarks.ts b/src/app/features/bookmarks/useBookmarks.ts new file mode 100644 index 000000000..aadd0bb8f --- /dev/null +++ b/src/app/features/bookmarks/useBookmarks.ts @@ -0,0 +1,78 @@ +import { useAtomValue, useSetAtom } from 'jotai'; +import { useCallback } from 'react'; +import { useMatrixClient } from '$hooks/useMatrixClient'; +import { bookmarkIdSetAtom, bookmarkListAtom, bookmarkLoadingAtom } from '$state/bookmarks'; +import { BookmarkItemContent, computeBookmarkId } from './bookmarkDomain'; +import { addBookmark, removeBookmark, listBookmarks, isBookmarked } from './bookmarkRepository'; + +/** Returns the current ordered bookmark list. */ +export function useBookmarkList(): BookmarkItemContent[] { + return useAtomValue(bookmarkListAtom); +} + +/** Returns true while a bookmark refresh is in progress. */ +export function useBookmarkLoading(): boolean { + return useAtomValue(bookmarkLoadingAtom); +} + +/** + * Returns true if the given (roomId, eventId) is currently bookmarked. + * + * Uses the locally cached bookmarkIdSetAtom for O(1) lookup. + * MSC4438 §Checking if a message is bookmarked. + */ +export function useIsBookmarked(roomId: string, eventId: string): boolean { + const idSet = useAtomValue(bookmarkIdSetAtom); + return idSet.has(computeBookmarkId(roomId, eventId)); +} + +/** + * Returns bookmark action callbacks: refresh, add, remove, checkIsBookmarked. + * + * `refresh` re-reads all bookmark items from the locally cached account data. + * `add` / `remove` optimistically update the local atom before writing to the server. + */ +export function useBookmarkActions() { + const mx = useMatrixClient(); + const setList = useSetAtom(bookmarkListAtom); + const setLoading = useSetAtom(bookmarkLoadingAtom); + + const refresh = useCallback(async () => { + setLoading(true); + try { + const items = listBookmarks(mx); + setList(items); + } finally { + setLoading(false); + } + }, [mx, setList, setLoading]); + + const add = useCallback( + async (item: BookmarkItemContent) => { + // Optimistic update + setList((prev) => { + if (prev.some((b) => b.bookmark_id === item.bookmark_id)) return prev; + return [item, ...prev]; + }); + await addBookmark(mx, item); + }, + [mx, setList] + ); + + const remove = useCallback( + async (bookmarkId: string) => { + // Optimistic update + setList((prev) => prev.filter((b) => b.bookmark_id !== bookmarkId)); + await removeBookmark(mx, bookmarkId); + }, + [mx, setList] + ); + + const checkIsBookmarked = useCallback( + (roomId: string, eventId: string): boolean => + isBookmarked(mx, computeBookmarkId(roomId, eventId)), + [mx] + ); + + return { refresh, add, remove, checkIsBookmarked }; +} diff --git a/src/app/features/bookmarks/useInitBookmarks.ts b/src/app/features/bookmarks/useInitBookmarks.ts new file mode 100644 index 000000000..40175ce58 --- /dev/null +++ b/src/app/features/bookmarks/useInitBookmarks.ts @@ -0,0 +1,70 @@ +import { MatrixEvent, SyncState } from '$types/matrix-sdk'; +import { useCallback, useEffect } from 'react'; +import { useSetAtom } from 'jotai'; +import { useMatrixClient } from '$hooks/useMatrixClient'; +import { useSyncState } from '$hooks/useSyncState'; +import { useAccountDataCallback } from '$hooks/useAccountDataCallback'; +import { bookmarkListAtom, bookmarkLoadingAtom } from '$state/bookmarks'; +import { AccountDataEvent } from '$types/matrix/accountData'; +import { listBookmarks } from './bookmarkRepository'; + +/** + * Top-level hook that keeps `bookmarkListAtom` in sync with account data. + * + * Must be called from an always-mounted component (e.g. ClientNonUIFeatures), + * NOT from a page component. Page components should simply read from the atom. + * + * Three triggers keep the atom current: + * 1. `useEffect` on mount — covers the case where `ClientNonUIFeatures` mounts + * after the initial sync transition has already fired (the common case). + * 2. `SyncState.Syncing` transition — refreshes on every reconnect. + * 3. `ClientEvent.AccountData` for the index event type — reacts immediately + * to index updates pushed by other devices mid-session. + */ +export function useInitBookmarks(): void { + const mx = useMatrixClient(); + const setList = useSetAtom(bookmarkListAtom); + const setLoading = useSetAtom(bookmarkLoadingAtom); + + const loadBookmarks = useCallback(() => { + setLoading(true); + try { + setList(listBookmarks(mx)); + } finally { + setLoading(false); + } + }, [mx, setList, setLoading]); + + // Immediate load: fires once on mount to cover the case where ClientNonUIFeatures + // mounts after the initial SyncState.Syncing transition has already fired. + // loadBookmarks is stable (memoized with stable deps), so this fires exactly once. + useEffect(() => { + loadBookmarks(); + }, [loadBookmarks]); + + // Trigger on reconnect (SyncState.Syncing transition after a disconnect). + useSyncState( + mx, + useCallback( + (state, prevState) => { + if (state === SyncState.Syncing && prevState !== SyncState.Syncing) { + loadBookmarks(); + } + }, + [loadBookmarks] + ) + ); + + // React to index updates pushed by other devices mid-session. + useAccountDataCallback( + mx, + useCallback( + (event: MatrixEvent) => { + if (event.getType() === (AccountDataEvent.BookmarksIndex as string)) { + loadBookmarks(); + } + }, + [loadBookmarks] + ) + ); +} diff --git a/src/app/features/room/message/Message.tsx b/src/app/features/room/message/Message.tsx index 98e7e927a..6652e1844 100644 --- a/src/app/features/room/message/Message.tsx +++ b/src/app/features/room/message/Message.tsx @@ -80,6 +80,9 @@ import { MessageEditHistoryItem } from '$components/message/modals/MessageEditHi import { MessageSourceCodeItem } from '$components/message/modals/MessageSource'; import { MessageForwardItem } from '$components/message/modals/MessageForward'; import { MessageDeleteItem } from '$components/message/modals/MessageDelete'; +import { computeBookmarkId, createBookmarkItem } from '$features/bookmarks/bookmarkDomain'; +import { useIsBookmarked, useBookmarkActions } from '$features/bookmarks/useBookmarks'; +import { useExperimentVariant } from '$hooks/useClientConfig'; import { MessageReportItem } from '$components/message/modals/MessageReport'; import { filterPronounsByLanguage, getParsedPronouns } from '$utils/pronouns'; import { useMentionClickHandler } from '$hooks/useMentionClickHandler'; @@ -209,6 +212,50 @@ export const MessagePinItem = as< ); }); +// message bookmarking +export const MessageBookmarkItem = as< + 'button', + { + room: Room; + mEvent: MatrixEvent; + onClose?: () => void; + } +>(({ room, mEvent, onClose, ...props }, ref) => { + const mx = useMatrixClient(); + const bookmarksExperiment = useExperimentVariant('messageBookmarks', mx.getUserId() ?? undefined); + const [enableMessageBookmarks] = useSetting(settingsAtom, 'enableMessageBookmarks'); + const eventId = mEvent.getId() ?? ''; + const isBookmarked = useIsBookmarked(room.roomId, eventId); + const { add, remove } = useBookmarkActions(); + + if (!bookmarksExperiment.inExperiment && !enableMessageBookmarks) return null; + + const handleClick = async () => { + if (isBookmarked) { + await remove(computeBookmarkId(room.roomId, eventId)); + } else { + const item = createBookmarkItem(room, mEvent); + if (item) await add(item); + } + onClose?.(); + }; + + return ( + } + radii="300" + onClick={handleClick} + {...props} + ref={ref} + > + + {isBookmarked ? 'Remove Bookmark' : 'Bookmark Message'} + + + ); +}); + export type ForwardedMessageProps = { originalTimestamp: number; isForwarded: boolean; @@ -1097,6 +1144,7 @@ function MessageInternal( )} + {canPinEvent && ( )} @@ -1430,6 +1478,13 @@ export const Event = as<'div', EventProps>( )} + {!stateEvent && ( + + )} {((!mEvent.isRedacted() && canDelete && !stateEvent) || (mEvent.getSender() !== mx.getUserId() && !stateEvent)) && ( diff --git a/src/app/features/settings/experimental/Experimental.tsx b/src/app/features/settings/experimental/Experimental.tsx index 330412185..e048ed281 100644 --- a/src/app/features/settings/experimental/Experimental.tsx +++ b/src/app/features/settings/experimental/Experimental.tsx @@ -10,6 +10,7 @@ import { Sync } from '../general'; import { SettingsSectionPage } from '../SettingsSectionPage'; import { BandwidthSavingEmojis } from './BandwithSavingEmojis'; import { MSC4268HistoryShare } from './MSC4268HistoryShare'; +import { MSC4438MessageBookmarks } from './MSC4438MessageBookmarks'; function PersonaToggle() { const [showPersonaSetting, setShowPersonaSetting] = useSetting( @@ -59,6 +60,7 @@ export function Experimental({ requestBack, requestClose }: Readonly + diff --git a/src/app/features/settings/experimental/MSC4438MessageBookmarks.tsx b/src/app/features/settings/experimental/MSC4438MessageBookmarks.tsx new file mode 100644 index 000000000..45e429b18 --- /dev/null +++ b/src/app/features/settings/experimental/MSC4438MessageBookmarks.tsx @@ -0,0 +1,56 @@ +import { SequenceCard } from '$components/sequence-card'; +import { SettingTile } from '$components/setting-tile'; +import { useSetting } from '$state/hooks/settings'; +import { settingsAtom } from '$state/settings'; +import { Box, Switch, Text } from 'folds'; +import { SequenceCardStyle } from '../styles.css'; + +export function MSC4438MessageBookmarks() { + const [enableMessageBookmarks, setEnableMessageBookmarks] = useSetting( + settingsAtom, + 'enableMessageBookmarks' + ); + + return ( + + Message Bookmarks + + + Save individual messages for later. Bookmarks are synced across all your devices via + account data.{' '} + + MSC4438 + + .{' '} + + Known issues (Sable GitHub) + + . + + } + after={ + + } + /> + + + ); +} diff --git a/src/app/hooks/router/useHomeSelected.ts b/src/app/hooks/router/useHomeSelected.ts index 2a16511aa..fcc439196 100644 --- a/src/app/hooks/router/useHomeSelected.ts +++ b/src/app/hooks/router/useHomeSelected.ts @@ -4,6 +4,7 @@ import { getHomeJoinPath, getHomePath, getHomeSearchPath, + getHomeBookmarksPath, } from '$pages/pathUtils'; export const useHomeSelected = (): boolean => { @@ -45,3 +46,13 @@ export const useHomeSearchSelected = (): boolean => { return !!match; }; + +export const useHomeBookmarksSelected = (): boolean => { + const match = useMatch({ + path: getHomeBookmarksPath(), + caseSensitive: true, + end: false, + }); + + return !!match; +}; diff --git a/src/app/hooks/router/useInbox.ts b/src/app/hooks/router/useInbox.ts index 639e16dd4..c19c0cc4b 100644 --- a/src/app/hooks/router/useInbox.ts +++ b/src/app/hooks/router/useInbox.ts @@ -1,5 +1,10 @@ import { useMatch } from 'react-router-dom'; -import { getInboxInvitesPath, getInboxNotificationsPath, getInboxPath } from '$pages/pathUtils'; +import { + getInboxBookmarksPath, + getInboxInvitesPath, + getInboxNotificationsPath, + getInboxPath, +} from '$pages/pathUtils'; export const useInboxSelected = (): boolean => { const match = useMatch({ @@ -30,3 +35,13 @@ export const useInboxInvitesSelected = (): boolean => { return !!match; }; + +export const useInboxBookmarksSelected = (): boolean => { + const match = useMatch({ + path: getInboxBookmarksPath(), + caseSensitive: true, + end: false, + }); + + return !!match; +}; diff --git a/src/app/hooks/useClientConfig.ts b/src/app/hooks/useClientConfig.ts index e523f15a7..d3e85df09 100644 --- a/src/app/hooks/useClientConfig.ts +++ b/src/app/hooks/useClientConfig.ts @@ -5,6 +5,21 @@ export type HashRouterConfig = { basename?: string; }; +export type ExperimentConfig = { + enabled?: boolean; + rolloutPercentage?: number; + variants?: string[]; + controlVariant?: string; +}; + +export type ExperimentSelection = { + key: string; + enabled: boolean; + rolloutPercentage: number; + variant: string; + inExperiment: boolean; +}; + export type ClientConfig = { defaultHomeserver?: number; homeserverList?: string[]; @@ -14,6 +29,8 @@ export type ClientConfig = { disableAccountSwitcher?: boolean; hideUsernamePasswordFields?: boolean; + experiments?: Record; + pushNotificationDetails?: { pushNotifyUrl?: string; vapidPublicKey?: string; @@ -55,6 +72,56 @@ export function useClientConfig(): ClientConfig { return config; } +const DEFAULT_CONTROL_VARIANT = 'control'; + +const normalizeRolloutPercentage = (value?: number): number => { + if (typeof value !== 'number' || Number.isNaN(value)) return 100; + if (value < 0) return 0; + if (value > 100) return 100; + return value; +}; + +const hashToUInt32 = (input: string): number => { + let hash = 0; + for (let index = 0; index < input.length; index += 1) { + hash = (hash * 131 + input.charCodeAt(index)) % 4294967291; + } + return hash; +}; + +export const selectExperimentVariant = ( + key: string, + experiment: ExperimentConfig | undefined, + subjectId: string | undefined +): ExperimentSelection => { + const controlVariant = experiment?.controlVariant ?? DEFAULT_CONTROL_VARIANT; + const variants = (experiment?.variants?.filter((v) => v.length > 0) ?? []).filter( + (v) => v !== controlVariant + ); + + const enabled = Boolean(experiment?.enabled); + const rolloutPercentage = normalizeRolloutPercentage(experiment?.rolloutPercentage); + + if (!enabled || !subjectId || variants.length === 0 || rolloutPercentage === 0) { + return { key, enabled, rolloutPercentage, variant: controlVariant, inExperiment: false }; + } + + // Two independent hashes keep rollout and variant assignment stable but decorrelated. + const rolloutBucket = hashToUInt32(`${key}:rollout:${subjectId}`) % 10000; + const rolloutCutoff = Math.floor(rolloutPercentage * 100); + if (rolloutBucket >= rolloutCutoff) { + return { key, enabled, rolloutPercentage, variant: controlVariant, inExperiment: false }; + } + + const variantIndex = hashToUInt32(`${key}:variant:${subjectId}`) % variants.length; + return { key, enabled, rolloutPercentage, variant: variants[variantIndex], inExperiment: true }; +}; + +export const useExperimentVariant = (key: string, subjectId?: string): ExperimentSelection => { + const clientConfig = useClientConfig(); + return selectExperimentVariant(key, clientConfig.experiments?.[key], subjectId); +}; + export const clientDefaultServer = (clientConfig: ClientConfig): string => clientConfig.homeserverList?.[clientConfig.defaultHomeserver ?? 0] ?? 'matrix.org'; diff --git a/src/app/pages/Router.tsx b/src/app/pages/Router.tsx index 28f8c7efb..fd8bc462e 100644 --- a/src/app/pages/Router.tsx +++ b/src/app/pages/Router.tsx @@ -48,6 +48,7 @@ import { NOTIFICATIONS_PATH_SEGMENT, ROOM_PATH_SEGMENT, SEARCH_PATH_SEGMENT, + BOOKMARKS_PATH_SEGMENT, SERVER_PATH_SEGMENT, CREATE_PATH, TO_ROOM_EVENT_PATH, @@ -65,6 +66,7 @@ import { import { ClientBindAtoms, ClientLayout, ClientRoot, ClientRouteOutlet } from './client'; import { HandleNotificationClick, ClientNonUIFeatures } from './client/ClientNonUIFeatures'; import { Home, HomeRouteRoomProvider, HomeSearch } from './client/home'; +import { BookmarksList } from './client/bookmarks'; import { Direct, DirectCreate, DirectRouteRoomProvider } from './client/direct'; import { RouteSpaceProvider, Space, SpaceRouteRoomProvider, SpaceSearch } from './client/space'; import { Explore, FeaturedRooms, PublicRooms } from './client/explore'; @@ -242,6 +244,7 @@ export const createRouter = (clientConfig: ClientConfig, screenSize: ScreenSize) } /> join

} /> } /> + } /> } /> } /> + } /> } /> diff --git a/src/app/pages/client/ClientNonUIFeatures.tsx b/src/app/pages/client/ClientNonUIFeatures.tsx index 26ac2f431..caebe459a 100644 --- a/src/app/pages/client/ClientNonUIFeatures.tsx +++ b/src/app/pages/client/ClientNonUIFeatures.tsx @@ -56,6 +56,7 @@ import { useCallSignaling } from '$hooks/useCallSignaling'; import { getBlobCacheStats } from '$hooks/useBlobCache'; import { lastVisitedRoomIdAtom } from '$state/room/lastRoom'; import { useSettingsSyncEffect } from '$hooks/useSettingsSync'; +import { useInitBookmarks } from '$features/bookmarks/useInitBookmarks'; import { getInboxInvitesPath } from '../pathUtils'; import { BackgroundNotifications } from './BackgroundNotifications'; @@ -845,11 +846,17 @@ function SettingsSyncFeature() { return null; } +function BookmarksFeature() { + useInitBookmarks(); + return null; +} + export function ClientNonUIFeatures({ children }: ClientNonUIFeaturesProps) { useCallSignaling(); return ( <> + diff --git a/src/app/pages/client/bookmarks/BookmarksList.tsx b/src/app/pages/client/bookmarks/BookmarksList.tsx new file mode 100644 index 000000000..6dae4fb5c --- /dev/null +++ b/src/app/pages/client/bookmarks/BookmarksList.tsx @@ -0,0 +1,560 @@ +import { FormEventHandler, useCallback, useMemo, useRef, useState } from 'react'; +import { + Avatar, + Box, + Button, + Dialog, + Header, + Icon, + IconButton, + Icons, + Input, + Line, + Overlay, + OverlayBackdrop, + OverlayCenter, + Scroll, + Spinner, + Text, + Chip, + config, + color, +} from 'folds'; +import FocusTrap from 'focus-trap-react'; +import { JoinRule } from '$types/matrix-sdk'; +import { + Page, + PageContent, + PageContentCenter, + PageHeader, + PageHero, + PageHeroSection, +} from '$components/page'; +import { SequenceCard } from '$components/sequence-card'; +import { AvatarBase, ModernLayout, Time, Username, UsernameBold } from '$components/message'; +import { RoomAvatar, RoomIcon } from '$components/room-avatar'; +import { UserAvatar } from '$components/user-avatar'; +import { BackRouteHandler } from '$components/BackRouteHandler'; +import { useMatrixClient } from '$hooks/useMatrixClient'; +import { useMediaAuthentication } from '$hooks/useMediaAuthentication'; +import { useRoomNavigate } from '$hooks/useRoomNavigate'; +import { ScreenSize, useScreenSizeContext } from '$hooks/useScreenSize'; +import { useSetting } from '$state/hooks/settings'; +import { settingsAtom } from '$state/settings'; +import { getMemberAvatarMxc, getMemberDisplayName, getRoomAvatarUrl } from '$utils/room'; +import { getMxIdLocalPart, mxcUrlToHttp } from '$utils/matrix'; +import colorMXID from '$utils/colorMXID'; +import { stopPropagation } from '$utils/keyboard'; +import { BookmarkItemContent } from '$features/bookmarks/bookmarkDomain'; +import { + useBookmarkActions, + useBookmarkList, + useBookmarkLoading, +} from '$features/bookmarks/useBookmarks'; + +// --------------------------------------------------------------------------- +// RemoveBookmarkDialog +// --------------------------------------------------------------------------- + +type RemoveBookmarkDialogProps = { + item: BookmarkItemContent; + onConfirm: () => void; + onClose: () => void; +}; + +function RemoveBookmarkDialog({ item, onConfirm, onClose }: RemoveBookmarkDialogProps) { + return ( + +
+ + Remove Bookmark + + + + +
+ + {item.body_preview && ( + + + {item.body_preview} + + + )} + Remove this bookmark? You can always re-bookmark the message. + + + + + + + ); +} + +// --------------------------------------------------------------------------- +// BookmarkItemRow +// --------------------------------------------------------------------------- + +type BookmarkItemRowProps = { + item: BookmarkItemContent; + highlight?: string; + onJump: (roomId: string, eventId: string) => void; + onRemove: (item: BookmarkItemContent) => void; + hour24Clock: boolean; + dateFormatString: string; +}; + +function BookmarkItemRow({ + item, + highlight, + onJump, + onRemove, + hour24Clock, + dateFormatString, +}: BookmarkItemRowProps) { + const mx = useMatrixClient(); + const useAuthentication = useMediaAuthentication(); + + // Try to resolve live room/member data; fall back to stored metadata + const room = mx.getRoom(item.room_id) ?? undefined; + const senderId = item.sender ?? ''; + + const displayName = room + ? (getMemberDisplayName(room, senderId) ?? getMxIdLocalPart(senderId) ?? senderId) + : (getMxIdLocalPart(senderId) ?? senderId); + + const senderAvatarMxc = room ? getMemberAvatarMxc(room, senderId) : undefined; + const avatarUrl = senderAvatarMxc + ? (mxcUrlToHttp(mx, senderAvatarMxc, useAuthentication, 48, 48, 'crop') ?? undefined) + : undefined; + + const usernameColor = colorMXID(senderId); + + // Highlight matching substring in body_preview + const preview = item.body_preview ?? ''; + const highlightedPreview = useMemo(() => { + if (!highlight || !preview) return <>{preview}; + const idx = preview.toLowerCase().indexOf(highlight.toLowerCase()); + if (idx === -1) return <>{preview}; + return ( + <> + {preview.slice(0, idx)} + + {preview.slice(idx, idx + highlight.length)} + + {preview.slice(idx + highlight.length)} + + ); + }, [preview, highlight]); + + return ( + + + + } + /> + + + } + > + + + + + {displayName} + + + + + onJump(item.room_id, item.event_id)} + variant="Secondary" + radii="400" + > + Jump + + onRemove(item)} + aria-label="Remove bookmark" + > + + + + + {preview && ( + + {highlightedPreview} + + )} + + + ); +} + +// --------------------------------------------------------------------------- +// BookmarkResultGroup +// --------------------------------------------------------------------------- + +type BookmarkResultGroupProps = { + roomId: string; + roomName: string; + items: BookmarkItemContent[]; + highlight?: string; + onJump: (roomId: string, eventId: string) => void; + onRemove: (item: BookmarkItemContent) => void; + hour24Clock: boolean; + dateFormatString: string; +}; + +function BookmarkResultGroup({ + roomId, + roomName, + items, + highlight, + onJump, + onRemove, + hour24Clock, + dateFormatString, +}: BookmarkResultGroupProps) { + const mx = useMatrixClient(); + const useAuthentication = useMediaAuthentication(); + const room = mx.getRoom(roomId) ?? undefined; + const avatarUrl = room ? getRoomAvatarUrl(mx, room, 96, useAuthentication) : undefined; + const displayRoomName = room?.name ?? roomName; + + return ( + +
+ + + ( + + )} + /> + + + {displayRoomName} + + +
+ + {items.map((item) => ( + + ))} + + + ); +} + +// --------------------------------------------------------------------------- +// BookmarkFilterInput +// --------------------------------------------------------------------------- + +type BookmarkFilterInputProps = { + inputRef: React.RefObject; + active?: boolean; + loading?: boolean; + onFilter: (term: string) => void; + onReset: () => void; +}; + +function BookmarkFilterInput({ + inputRef, + active, + loading, + onFilter, + onReset, +}: BookmarkFilterInputProps) { + const handleSubmit: FormEventHandler = (evt) => { + evt.preventDefault(); + const { filterInput } = evt.target as HTMLFormElement & { + filterInput: HTMLInputElement; + }; + const term = filterInput.value.trim(); + if (term) onFilter(term); + }; + + return ( + + + Filter + + ) : ( + + {active && ( + + + Clear + + )} + + Filter + + + ) + } + /> + + ); +} + +// --------------------------------------------------------------------------- +// BookmarksList (main export) +// --------------------------------------------------------------------------- + +export function BookmarksList() { + const mx = useMatrixClient(); + const screenSize = useScreenSizeContext(); + const scrollRef = useRef(null); + const filterInputRef = useRef(null); + const { navigateRoom } = useRoomNavigate(); + + const [hour24Clock] = useSetting(settingsAtom, 'hour24Clock'); + const [dateFormatString] = useSetting(settingsAtom, 'dateFormatString'); + + const bookmarks = useBookmarkList(); + const loading = useBookmarkLoading(); + const { remove } = useBookmarkActions(); + + const [filterTerm, setFilterTerm] = useState(); + const [removingItem, setRemovingItem] = useState(); + + // Filter and group bookmarks + const filteredBookmarks = useMemo(() => { + if (!filterTerm) return bookmarks; + const lower = filterTerm.toLowerCase(); + return bookmarks.filter( + (b) => + b.body_preview?.toLowerCase().includes(lower) || + b.room_name?.toLowerCase().includes(lower) || + (b.sender && getMxIdLocalPart(b.sender)?.toLowerCase().includes(lower)) + ); + }, [bookmarks, filterTerm]); + + // Group by room_id, preserving order + const groupedByRoom = useMemo(() => { + const map = new Map< + string, + { roomId: string; roomName: string; items: BookmarkItemContent[] } + >(); + filteredBookmarks.forEach((item) => { + let group = map.get(item.room_id); + if (!group) { + const room = mx.getRoom(item.room_id); + group = { + roomId: item.room_id, + roomName: room?.name ?? item.room_name ?? item.room_id, + items: [], + }; + map.set(item.room_id, group); + } + group.items.push(item); + }); + return Array.from(map.values()); + }, [filteredBookmarks, mx]); + + const handleJump = useCallback( + (roomId: string, eventId: string) => { + navigateRoom(roomId, eventId); + }, + [navigateRoom] + ); + + const handleRemoveConfirm = useCallback(async () => { + if (!removingItem) return; + await remove(removingItem.bookmark_id); + setRemovingItem(undefined); + }, [removingItem, remove]); + + const handleFilter = useCallback((term: string) => { + setFilterTerm(term); + }, []); + + const handleReset = useCallback(() => { + setFilterTerm(undefined); + if (filterInputRef.current) { + filterInputRef.current.value = ''; + } + }, []); + + return ( + + + + + {screenSize === ScreenSize.Mobile && ( + + {(onBack) => ( + + + + )} + + )} + + + {screenSize !== ScreenSize.Mobile && } + + Bookmarks + + + + + + + + + + + + + {loading && bookmarks.length === 0 && ( + + + + )} + + {!loading && bookmarks.length === 0 && ( + + } + title="No Bookmarks Yet" + subTitle="Bookmark messages to find them again easily. Right-click a message and choose Bookmark." + /> + + )} + + {!loading && bookmarks.length > 0 && filteredBookmarks.length === 0 && ( + + + + No bookmarks match your filter. + + + )} + + {groupedByRoom.length > 0 && ( + + {groupedByRoom.map((group, i) => ( + <> + {i > 0 && } + + + ))} + + )} + + + + + + {removingItem && ( + }> + + setRemovingItem(undefined), + clickOutsideDeactivates: true, + escapeDeactivates: stopPropagation, + }} + > + setRemovingItem(undefined)} + /> + + + + )} + + ); +} diff --git a/src/app/pages/client/bookmarks/index.ts b/src/app/pages/client/bookmarks/index.ts new file mode 100644 index 000000000..cdd211f71 --- /dev/null +++ b/src/app/pages/client/bookmarks/index.ts @@ -0,0 +1 @@ +export * from './BookmarksList'; diff --git a/src/app/pages/client/home/Home.tsx b/src/app/pages/client/home/Home.tsx index c25d99e30..175b48f9d 100644 --- a/src/app/pages/client/home/Home.tsx +++ b/src/app/pages/client/home/Home.tsx @@ -35,11 +35,16 @@ import { getHomeCreatePath, getHomeRoomPath, getHomeSearchPath, + getHomeBookmarksPath, withSearchParam, } from '$pages/pathUtils'; import { getCanonicalAliasOrRoomId } from '$utils/matrix'; import { useSelectedRoom } from '$hooks/router/useSelectedRoom'; -import { useHomeCreateSelected, useHomeSearchSelected } from '$hooks/router/useHomeSelected'; +import { + useHomeCreateSelected, + useHomeSearchSelected, + useHomeBookmarksSelected, +} from '$hooks/router/useHomeSelected'; import { useMatrixClient } from '$hooks/useMatrixClient'; import { VirtualTile } from '$components/virtualizer'; import { RoomNavCategoryButton, RoomNavItem } from '$features/room-nav'; @@ -54,6 +59,7 @@ import { useClosedNavCategoriesAtom } from '$state/hooks/closedNavCategories'; import { stopPropagation } from '$utils/keyboard'; import { useSetting } from '$state/hooks/settings'; import { settingsAtom } from '$state/settings'; +import { useExperimentVariant } from '$hooks/useClientConfig'; import { getRoomNotificationMode, useRoomsNotificationPreferencesContext, @@ -203,6 +209,10 @@ export function Home() { const selectedRoomId = useSelectedRoom(); const createRoomSelected = useHomeCreateSelected(); const searchSelected = useHomeSearchSelected(); + const bookmarksSelected = useHomeBookmarksSelected(); + const bookmarksExperiment = useExperimentVariant('messageBookmarks', mx.getUserId() ?? undefined); + const [enableMessageBookmarks] = useSetting(settingsAtom, 'enableMessageBookmarks'); + const showBookmarks = bookmarksExperiment.inExperiment || enableMessageBookmarks; const noRoomToDisplay = rooms.length === 0; const [closedCategories, setClosedCategories] = useAtom(useClosedNavCategoriesAtom()); @@ -236,83 +246,101 @@ export function Home() { return ( - {noRoomToDisplay ? ( - - ) : ( - - - - - navigate(getHomeCreatePath())}> - - - - - - - - Create Room - - + + + + + navigate(getHomeCreatePath())}> + + + + + + + + Create Room + - - - - - {(open, setOpen) => ( - <> - - setOpen(true)}> - - - - - - - - Join with Address - - + + + + + + {(open, setOpen) => ( + <> + + setOpen(true)}> + + + + + + + + Join with Address + - - - - {open && ( - setOpen(false)} - onOpen={(roomIdOrAlias, viaServers, eventId) => { - setOpen(false); - const path = getHomeRoomPath(roomIdOrAlias, eventId); - navigate( - viaServers - ? withSearchParam(path, { - viaServers: encodeSearchParamValueArray(viaServers), - }) - : path - ); - }} - /> - )} - - )} - - - + + + + + {open && ( + setOpen(false)} + onOpen={(roomIdOrAlias, viaServers, eventId) => { + setOpen(false); + const path = getHomeRoomPath(roomIdOrAlias, eventId); + navigate( + viaServers + ? withSearchParam(path, { + viaServers: encodeSearchParamValueArray(viaServers), + }) + : path + ); + }} + /> + )} + + )} + + + + + + + + + + + Message Search + + + + + + + {showBookmarks && ( + + - + - Message Search + Bookmarks - + )} + + {noRoomToDisplay ? ( + + ) : ( - - - )} + )} + + ); } diff --git a/src/app/pages/client/inbox/Inbox.tsx b/src/app/pages/client/inbox/Inbox.tsx index 661435513..d594a928e 100644 --- a/src/app/pages/client/inbox/Inbox.tsx +++ b/src/app/pages/client/inbox/Inbox.tsx @@ -1,12 +1,24 @@ import { Avatar, Box, Icon, Icons, Text } from 'folds'; import { useAtomValue } from 'jotai'; import { NavCategory, NavItem, NavItemContent, NavLink } from '$components/nav'; -import { getInboxInvitesPath, getInboxNotificationsPath } from '$pages/pathUtils'; -import { useInboxInvitesSelected, useInboxNotificationsSelected } from '$hooks/router/useInbox'; +import { + getInboxBookmarksPath, + getInboxInvitesPath, + getInboxNotificationsPath, +} from '$pages/pathUtils'; +import { + useInboxBookmarksSelected, + useInboxInvitesSelected, + useInboxNotificationsSelected, +} from '$hooks/router/useInbox'; import { UnreadBadge } from '$components/unread-badge'; import { allInvitesAtom } from '$state/room-list/inviteList'; import { useNavToActivePathMapper } from '$hooks/useNavToActivePathMapper'; import { PageNav, PageNavContent, PageNavHeader } from '$components/page'; +import { useMatrixClient } from '$hooks/useMatrixClient'; +import { useSetting } from '$state/hooks/settings'; +import { settingsAtom } from '$state/settings'; +import { useExperimentVariant } from '$hooks/useClientConfig'; function InvitesNavItem() { const invitesSelected = useInboxInvitesSelected(); @@ -39,9 +51,36 @@ function InvitesNavItem() { ); } +function BookmarksNavItem() { + const bookmarksSelected = useInboxBookmarksSelected(); + + return ( + + + + + + + + + + Bookmarks + + + + + + + ); +} + export function Inbox() { useNavToActivePathMapper('inbox'); + const mx = useMatrixClient(); const notificationsSelected = useInboxNotificationsSelected(); + const bookmarksExperiment = useExperimentVariant('messageBookmarks', mx.getUserId() ?? undefined); + const [enableMessageBookmarks] = useSetting(settingsAtom, 'enableMessageBookmarks'); + const showBookmarks = bookmarksExperiment.inExperiment || enableMessageBookmarks; return ( @@ -75,6 +114,7 @@ export function Inbox() { + {showBookmarks && } diff --git a/src/app/pages/pathUtils.ts b/src/app/pages/pathUtils.ts index 120a1a70c..7e5cb8d84 100644 --- a/src/app/pages/pathUtils.ts +++ b/src/app/pages/pathUtils.ts @@ -13,7 +13,9 @@ import { HOME_PATH, HOME_ROOM_PATH, HOME_SEARCH_PATH, + HOME_BOOKMARKS_PATH, LOGIN_PATH, + INBOX_BOOKMARKS_PATH, INBOX_INVITES_PATH, INBOX_NOTIFICATIONS_PATH, INBOX_PATH, @@ -93,6 +95,7 @@ export const getHomePath = (): string => HOME_PATH; export const getHomeCreatePath = (): string => HOME_CREATE_PATH; export const getHomeJoinPath = (): string => HOME_JOIN_PATH; export const getHomeSearchPath = (): string => HOME_SEARCH_PATH; +export const getHomeBookmarksPath = (): string => HOME_BOOKMARKS_PATH; export const getHomeRoomPath = (roomIdOrAlias: string, eventId?: string): string => { const params = { roomIdOrAlias: encodeURIComponent(roomIdOrAlias), @@ -160,6 +163,7 @@ export const getCreatePath = (): string => CREATE_PATH; export const getInboxPath = (): string => INBOX_PATH; export const getInboxNotificationsPath = (): string => INBOX_NOTIFICATIONS_PATH; export const getInboxInvitesPath = (): string => INBOX_INVITES_PATH; +export const getInboxBookmarksPath = (): string => INBOX_BOOKMARKS_PATH; export const getSettingsPath = (section?: string, focus?: string): string => { const path = trimTrailingSlash(generatePath(SETTINGS_PATH, { section: section ?? null })); diff --git a/src/app/pages/paths.ts b/src/app/pages/paths.ts index 1ac57b756..2e686d109 100644 --- a/src/app/pages/paths.ts +++ b/src/app/pages/paths.ts @@ -39,6 +39,7 @@ export type SearchPathSearchParams = { senders?: string; }; export const SEARCH_PATH_SEGMENT = 'search/'; +export const BOOKMARKS_PATH_SEGMENT = 'bookmarks/'; export type RoomSearchParams = { /* comma separated string of servers */ @@ -50,6 +51,7 @@ export const HOME_PATH = '/home/'; export const HOME_CREATE_PATH = `/home/${CREATE_PATH_SEGMENT}`; export const HOME_JOIN_PATH = `/home/${JOIN_PATH_SEGMENT}`; export const HOME_SEARCH_PATH = `/home/${SEARCH_PATH_SEGMENT}`; +export const HOME_BOOKMARKS_PATH = `/home/${BOOKMARKS_PATH_SEGMENT}`; export const HOME_ROOM_PATH = `/home/${ROOM_PATH_SEGMENT}`; export const DIRECT_PATH = '/direct/'; @@ -88,6 +90,7 @@ export type InboxNotificationsPathSearchParams = { }; export const INBOX_NOTIFICATIONS_PATH = `/inbox/${NOTIFICATIONS_PATH_SEGMENT}`; export const INBOX_INVITES_PATH = `/inbox/${INVITES_PATH_SEGMENT}`; +export const INBOX_BOOKMARKS_PATH = `/inbox/${BOOKMARKS_PATH_SEGMENT}`; export const TO_PATH = '/to'; // Deep-link route used by push notification click-back URLs. diff --git a/src/app/state/bookmarks.ts b/src/app/state/bookmarks.ts new file mode 100644 index 000000000..7a2375691 --- /dev/null +++ b/src/app/state/bookmarks.ts @@ -0,0 +1,20 @@ +import { atom } from 'jotai'; +import { BookmarkItemContent } from '../features/bookmarks/bookmarkDomain'; + +/** Ordered list of active bookmark items (mirrors the server index order). */ +export const bookmarkListAtom = atom([]); + +/** True while a refresh from account data is in progress. */ +export const bookmarkLoadingAtom = atom(false); + +/** + * Derived set of active bookmark IDs — used for O(1) per-message + * "is this message bookmarked?" checks. + * + * MSC4438 §Checking if a message is bookmarked: use a local reverse lookup + * rather than issuing server requests. + */ +export const bookmarkIdSetAtom = atom>((get) => { + const list = get(bookmarkListAtom); + return new Set(list.map((b) => b.bookmark_id)); +}); diff --git a/src/app/state/settings.ts b/src/app/state/settings.ts index 3dcf1b1fb..68318ae5c 100644 --- a/src/app/state/settings.ts +++ b/src/app/state/settings.ts @@ -117,6 +117,9 @@ export interface Settings { showPersonaSetting: boolean; closeFoldersByDefault: boolean; + // experimental + enableMessageBookmarks: boolean; + // furry stuff renderAnimals: boolean; } @@ -216,6 +219,9 @@ const defaultSettings: Settings = { showPersonaSetting: false, closeFoldersByDefault: false, + // experimental + enableMessageBookmarks: false, + // furry stuff renderAnimals: true, }; diff --git a/src/types/matrix/accountData.ts b/src/types/matrix/accountData.ts index 25b13cbc4..f89958d9f 100644 --- a/src/types/matrix/accountData.ts +++ b/src/types/matrix/accountData.ts @@ -18,6 +18,11 @@ export enum AccountDataEvent { CrossSigningUser = 'm.cross_signing.user', MegolmBackupV1 = 'm.megolm_backup.v1', + // MSC4438 Message Bookmarks (unstable prefix) + BookmarksIndex = 'org.matrix.msc4438.bookmarks.index', + /** Prefix for per-bookmark item events; append the bookmark ID to get the full event type. */ + BookmarkItemPrefix = 'org.matrix.msc4438.bookmark.', + // Sable account data SableNicknames = 'moe.sable.app.nicknames', SablePinStatus = 'moe.sable.app.pins_read_marker', From f7268334108cacaa2b9ec47493f7fde922aecf32 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Mon, 6 Apr 2026 12:46:47 -0400 Subject: [PATCH 02/17] test(bookmarks): add unit tests for MSC4438 bookmark domain and repository --- .../features/bookmarks/bookmarkDomain.test.ts | 375 ++++++++++++++++++ .../bookmarks/bookmarkRepository.test.ts | 334 ++++++++++++++++ src/app/state/bookmarks.test.ts | 69 ++++ 3 files changed, 778 insertions(+) create mode 100644 src/app/features/bookmarks/bookmarkDomain.test.ts create mode 100644 src/app/features/bookmarks/bookmarkRepository.test.ts create mode 100644 src/app/state/bookmarks.test.ts diff --git a/src/app/features/bookmarks/bookmarkDomain.test.ts b/src/app/features/bookmarks/bookmarkDomain.test.ts new file mode 100644 index 000000000..2f70879da --- /dev/null +++ b/src/app/features/bookmarks/bookmarkDomain.test.ts @@ -0,0 +1,375 @@ +/** + * Unit tests for MSC4438 bookmark domain logic. + * All functions in bookmarkDomain.ts are pure / side-effect-free. + */ +import { describe, it, expect } from 'vitest'; +import type { MatrixEvent, Room } from '$types/matrix-sdk'; +import { AccountDataEvent } from '$types/matrix/accountData'; +import { + bookmarkItemEventType, + buildMatrixURI, + computeBookmarkId, + createBookmarkItem, + emptyIndex, + extractBodyPreview, + isValidBookmarkItem, + isValidIndexContent, +} from './bookmarkDomain'; + +// --------------------------------------------------------------------------- +// Helpers: minimal Matrix object stubs +// --------------------------------------------------------------------------- + +function makeEvent( + opts: { + id?: string | null; + body?: unknown; + msgtype?: string; + sender?: string; + ts?: number; + } = {} +): MatrixEvent { + return { + getId: () => (opts.id === null ? undefined : (opts.id ?? '$event:server.tld')), + getTs: () => opts.ts ?? 1_000_000, + getSender: () => opts.sender ?? '@alice:server.tld', + getContent: () => ({ + body: opts.body, + msgtype: opts.msgtype ?? 'm.text', + }), + } as unknown as MatrixEvent; +} + +function makeRoom(opts: { roomId?: string; name?: string } = {}): Room { + return { + roomId: opts.roomId ?? '!room:server.tld', + name: opts.name ?? 'Test Room', + } as unknown as Room; +} + +// --------------------------------------------------------------------------- +// computeBookmarkId +// --------------------------------------------------------------------------- + +describe('computeBookmarkId', () => { + it('returns a string prefixed with "bmk_"', () => { + expect(computeBookmarkId('!room:s', '$event:s')).toMatch(/^bmk_/); + }); + + it('is exactly 12 characters long ("bmk_" + 8 hex digits)', () => { + expect(computeBookmarkId('!room:s', '$event:s')).toHaveLength(12); + }); + + it('only contains hex digits after the prefix', () => { + const id = computeBookmarkId('!room:server.tld', '$event:server.tld'); + expect(id.slice(4)).toMatch(/^[0-9a-f]{8}$/); + }); + + it('is deterministic — same inputs always yield the same ID', () => { + const a = computeBookmarkId('!room:server.tld', '$event:server.tld'); + const b = computeBookmarkId('!room:server.tld', '$event:server.tld'); + expect(a).toBe(b); + }); + + it('differs when roomId changes', () => { + const a = computeBookmarkId('!roomA:s', '$event:s'); + const b = computeBookmarkId('!roomB:s', '$event:s'); + expect(a).not.toBe(b); + }); + + it('differs when eventId changes', () => { + const a = computeBookmarkId('!room:s', '$eventA:s'); + const b = computeBookmarkId('!room:s', '$eventB:s'); + expect(a).not.toBe(b); + }); + + it('separator prevents (roomId + eventId) collisions', () => { + // Without "|" separator, ("ab", "c") and ("a", "bc") would hash the same + const a = computeBookmarkId('ab', 'c'); + const b = computeBookmarkId('a', 'bc'); + expect(a).not.toBe(b); + }); + + // Known vector — computed from the reference djb2-like algorithm: + // input = "a|b", each char's code units: 97, 124, 98 + // hash trace: 0 → 97 → 3131 → 97159 (0x17b87) + it('produces the known reference vector for ("a", "b")', () => { + expect(computeBookmarkId('a', 'b')).toBe('bmk_00017b87'); + }); +}); + +// --------------------------------------------------------------------------- +// bookmarkItemEventType +// --------------------------------------------------------------------------- + +describe('bookmarkItemEventType', () => { + it('returns the MSC4438 unstable event type for a given bookmark ID', () => { + expect(bookmarkItemEventType('bmk_abcd1234')).toBe( + `${AccountDataEvent.BookmarkItemPrefix}bmk_abcd1234` + ); + }); + + it('uses BookmarkItemPrefix as the base', () => { + const id = 'bmk_00000001'; + expect(bookmarkItemEventType(id)).toContain(AccountDataEvent.BookmarkItemPrefix); + }); + + it('has BookmarksIndex enum value defined correctly', () => { + expect(AccountDataEvent.BookmarksIndex).toBe('org.matrix.msc4438.bookmarks.index'); + }); +}); + +// --------------------------------------------------------------------------- +// buildMatrixURI +// --------------------------------------------------------------------------- + +describe('buildMatrixURI', () => { + it.each([ + [ + '!room:server.tld', + '$event:server.tld', + // encodeURIComponent does not encode '!' — only ':' and '$' are encoded here + 'matrix:roomid/!room%3Aserver.tld/e/%24event%3Aserver.tld', + ], + ['simple', 'id', 'matrix:roomid/simple/e/id'], + ['a b', 'c d', 'matrix:roomid/a%20b/e/c%20d'], + ])('buildMatrixURI(%s, %s) → %s', (roomId, eventId, expected) => { + expect(buildMatrixURI(roomId, eventId)).toBe(expected); + }); + + it('starts with "matrix:roomid/"', () => { + expect(buildMatrixURI('!r:s', '$e:s')).toMatch(/^matrix:roomid\//); + }); + + it('contains "/e/" separator between roomId and eventId', () => { + expect(buildMatrixURI('!r:s', '$e:s')).toContain('/e/'); + }); +}); + +// --------------------------------------------------------------------------- +// extractBodyPreview +// --------------------------------------------------------------------------- + +describe('extractBodyPreview', () => { + it('returns the body unchanged when it is within the default limit', () => { + const event = makeEvent({ body: 'Hello, world!' }); + expect(extractBodyPreview(event)).toBe('Hello, world!'); + }); + + it('returns an empty string when body is undefined', () => { + const event = makeEvent({ body: undefined }); + expect(extractBodyPreview(event)).toBe(''); + }); + + it('returns an empty string when body is a non-string type', () => { + const event = makeEvent({ body: 42 }); + expect(extractBodyPreview(event)).toBe(''); + }); + + it('returns an empty string when body is an empty string', () => { + const event = makeEvent({ body: '' }); + expect(extractBodyPreview(event)).toBe(''); + }); + + it('truncates to 120 chars and appends "…" when body exceeds the default limit', () => { + const long = 'x'.repeat(200); + const result = extractBodyPreview(makeEvent({ body: long })); + expect(result).toHaveLength(121); // 120 + ellipsis char + expect(result.endsWith('\u2026')).toBe(true); + expect(result.slice(0, 120)).toBe('x'.repeat(120)); + }); + + it('does not truncate when body is exactly 120 chars', () => { + const exact = 'y'.repeat(120); + expect(extractBodyPreview(makeEvent({ body: exact }))).toBe(exact); + }); + + it('respects a custom maxLength', () => { + const event = makeEvent({ body: 'abcdefghij' }); + const result = extractBodyPreview(event, 5); + expect(result).toBe('abcde\u2026'); + }); +}); + +// --------------------------------------------------------------------------- +// isValidIndexContent +// --------------------------------------------------------------------------- + +describe('isValidIndexContent', () => { + const valid = { + version: 1 as const, + revision: 0, + updated_ts: Date.now(), + bookmark_ids: [], + }; + + it('accepts a well-formed index', () => { + expect(isValidIndexContent(valid)).toBe(true); + }); + + it('accepts an index with string IDs in bookmark_ids', () => { + expect(isValidIndexContent({ ...valid, bookmark_ids: ['bmk_aabbccdd'] })).toBe(true); + }); + + it('rejects null', () => { + expect(isValidIndexContent(null)).toBe(false); + }); + + it('rejects a non-object', () => { + expect(isValidIndexContent('string')).toBe(false); + expect(isValidIndexContent(42)).toBe(false); + }); + + it('rejects version !== 1', () => { + expect(isValidIndexContent({ ...valid, version: 2 })).toBe(false); + }); + + it('rejects missing revision', () => { + const { revision, ...rest } = valid; + expect(isValidIndexContent(rest)).toBe(false); + }); + + it('rejects missing updated_ts', () => { + const { updated_ts: updatedTs, ...rest } = valid; + expect(isValidIndexContent(rest)).toBe(false); + }); + + it('rejects missing bookmark_ids', () => { + const { bookmark_ids: bookmarkIds, ...rest } = valid; + expect(isValidIndexContent(rest)).toBe(false); + }); + + it('rejects bookmark_ids containing a non-string', () => { + expect(isValidIndexContent({ ...valid, bookmark_ids: [1, 2, 3] })).toBe(false); + }); +}); + +// --------------------------------------------------------------------------- +// isValidBookmarkItem +// --------------------------------------------------------------------------- + +describe('isValidBookmarkItem', () => { + const valid = { + version: 1 as const, + bookmark_id: 'bmk_abcd1234', + uri: 'matrix:roomid/foo/e/bar', + room_id: '!room:s', + event_id: '$event:s', + event_ts: 1_000_000, + bookmarked_ts: 2_000_000, + }; + + it('accepts a complete, well-formed item', () => { + expect(isValidBookmarkItem(valid)).toBe(true); + }); + + it('accepts an item with optional fields set', () => { + expect( + isValidBookmarkItem({ ...valid, sender: '@alice:s', room_name: 'Room', deleted: false }) + ).toBe(true); + }); + + it('rejects null', () => { + expect(isValidBookmarkItem(null)).toBe(false); + }); + + it('rejects a non-object', () => { + expect(isValidBookmarkItem('string')).toBe(false); + }); + + it('rejects version !== 1', () => { + expect(isValidBookmarkItem({ ...valid, version: 2 })).toBe(false); + }); + + it.each(['bookmark_id', 'uri', 'room_id', 'event_id'] as const)( + 'rejects item missing string field "%s"', + (field) => { + const copy = { ...valid } as Record; + delete copy[field]; + expect(isValidBookmarkItem(copy)).toBe(false); + } + ); + + it.each(['event_ts', 'bookmarked_ts'] as const)( + 'rejects item missing numeric field "%s"', + (field) => { + const copy = { ...valid } as Record; + delete copy[field]; + expect(isValidBookmarkItem(copy)).toBe(false); + } + ); +}); + +// --------------------------------------------------------------------------- +// createBookmarkItem +// --------------------------------------------------------------------------- + +describe('createBookmarkItem', () => { + it('returns undefined when the event has no ID', () => { + const room = makeRoom(); + const event = makeEvent({ id: null }); + expect(createBookmarkItem(room, event)).toBeUndefined(); + }); + + it('returns a valid BookmarkItemContent for a normal event', () => { + const room = makeRoom({ roomId: '!r:s', name: 'My Room' }); + const event = makeEvent({ + id: '$e:s', + body: 'Hello', + msgtype: 'm.text', + sender: '@bob:s', + ts: 123456, + }); + const item = createBookmarkItem(room, event); + expect(item).toBeDefined(); + expect(item!.version).toBe(1); + expect(item!.room_id).toBe('!r:s'); + expect(item!.event_id).toBe('$e:s'); + expect(item!.bookmark_id).toBe(computeBookmarkId('!r:s', '$e:s')); + expect(item!.uri).toBe(buildMatrixURI('!r:s', '$e:s')); + expect(item!.event_ts).toBe(123456); + expect(item!.sender).toBe('@bob:s'); + expect(item!.room_name).toBe('My Room'); + expect(item!.body_preview).toBe('Hello'); + expect(item!.msgtype).toBe('m.text'); + }); + + it('omits body_preview when body is missing', () => { + const room = makeRoom(); + const event = makeEvent({ body: undefined }); + const item = createBookmarkItem(room, event); + expect(item!.body_preview).toBe(''); + }); + + it('passes isValidBookmarkItem on the returned content', () => { + const room = makeRoom(); + const event = makeEvent(); + const item = createBookmarkItem(room, event); + expect(isValidBookmarkItem(item)).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// emptyIndex +// --------------------------------------------------------------------------- + +describe('emptyIndex', () => { + it('returns a valid index with version 1', () => { + const idx = emptyIndex(); + expect(isValidIndexContent(idx)).toBe(true); + expect(idx.version).toBe(1); + }); + + it('starts with revision 0 and empty bookmark_ids', () => { + const idx = emptyIndex(); + expect(idx.revision).toBe(0); + expect(idx.bookmark_ids).toEqual([]); + }); + + it('returns a fresh object on each call (no shared reference)', () => { + const a = emptyIndex(); + const b = emptyIndex(); + a.bookmark_ids.push('bmk_aabbccdd'); + expect(b.bookmark_ids).toHaveLength(0); + }); +}); diff --git a/src/app/features/bookmarks/bookmarkRepository.test.ts b/src/app/features/bookmarks/bookmarkRepository.test.ts new file mode 100644 index 000000000..7b928b50e --- /dev/null +++ b/src/app/features/bookmarks/bookmarkRepository.test.ts @@ -0,0 +1,334 @@ +/** + * Unit tests for MSC4438 bookmark repository layer. + * + * The repository functions are pure in the sense that they read and write + * synchronously from a MatrixClient mock that returns predictable account data. + * No network calls are made. + */ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import type { MatrixClient } from '$types/matrix-sdk'; +import { AccountDataEvent } from '$types/matrix/accountData'; +import { addBookmark, removeBookmark, listBookmarks, isBookmarked } from './bookmarkRepository'; +import { + bookmarkItemEventType, + emptyIndex, + type BookmarkIndexContent, + type BookmarkItemContent, +} from './bookmarkDomain'; + +// --------------------------------------------------------------------------- +// Stub MatrixClient +// --------------------------------------------------------------------------- + +/** + * Build a minimal MatrixClient stub backed by an in-memory store. + * `getAccountData` returns a fake MatrixEvent whose `getContent()` reads + * from the store; `setAccountData` writes to the store. + */ +function makeClient(initialData: Record = {}): MatrixClient { + const store: Record = { ...initialData }; + const accountData = new Map(Object.entries(store)); + + return { + getAccountData: vi.fn((eventType: string) => { + const content = store[eventType]; + if (content === undefined) return undefined; + return { getContent: () => content }; + }), + setAccountData: vi.fn(async (eventType: string, content: unknown) => { + store[eventType] = content; + accountData.set(eventType, content); + }), + store: { accountData }, + _store: store, // exposed for inspection in tests + } as unknown as MatrixClient; +} + +// --------------------------------------------------------------------------- +// Test data helpers +// --------------------------------------------------------------------------- + +function makeItem(overrides: Partial = {}): BookmarkItemContent { + return { + version: 1, + bookmark_id: 'bmk_aabbccdd', + uri: 'matrix:roomid/foo/e/bar', + room_id: '!room:s', + event_id: '$event:s', + event_ts: 1_000_000, + bookmarked_ts: 2_000_000, + ...overrides, + }; +} + +function makeIndex(overrides: Partial = {}): BookmarkIndexContent { + return { + ...emptyIndex(), + ...overrides, + }; +} + +// --------------------------------------------------------------------------- +// addBookmark +// --------------------------------------------------------------------------- + +describe('addBookmark', () => { + let mx: MatrixClient; + + beforeEach(() => { + mx = makeClient(); + }); + + it('writes the item event before writing the index', async () => { + const item = makeItem(); + const callOrder: string[] = []; + + (mx.setAccountData as ReturnType).mockImplementation( + async (type: string, content: unknown) => { + callOrder.push(type); + // keep default in-memory behaviour + (mx as any)._store[type] = content; + } + ); + + await addBookmark(mx, item); + + expect(callOrder[0]).toBe(bookmarkItemEventType(item.bookmark_id)); + expect(callOrder[1]).toBe(AccountDataEvent.BookmarksIndex); + }); + + it('prepends the bookmark ID to bookmark_ids in the index', async () => { + const existing = makeItem({ bookmark_id: 'bmk_11111111' }); + const mx2 = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: [existing.bookmark_id] }), + [bookmarkItemEventType(existing.bookmark_id)]: existing, + }); + + const newItem = makeItem({ bookmark_id: 'bmk_22222222' }); + await addBookmark(mx2, newItem); + + const store = (mx2 as any)._store; + const idx = store[AccountDataEvent.BookmarksIndex] as BookmarkIndexContent; + expect(idx.bookmark_ids[0]).toBe('bmk_22222222'); + expect(idx.bookmark_ids[1]).toBe('bmk_11111111'); + }); + + it('does not duplicate an ID already in the index', async () => { + const item = makeItem(); + const mx2 = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: [item.bookmark_id] }), + [bookmarkItemEventType(item.bookmark_id)]: item, + }); + + await addBookmark(mx2, item); + + const idx = (mx2 as any)._store[AccountDataEvent.BookmarksIndex] as BookmarkIndexContent; + expect(idx.bookmark_ids.filter((id) => id === item.bookmark_id)).toHaveLength(1); + }); + + it('increments the index revision', async () => { + const item = makeItem(); + await addBookmark(mx, item); + + const idx = (mx as any)._store[AccountDataEvent.BookmarksIndex] as BookmarkIndexContent; + expect(idx.revision).toBe(1); + }); + + it('works when no index exists yet (creates an empty one)', async () => { + const item = makeItem(); + await addBookmark(mx, item); + + const idx = (mx as any)._store[AccountDataEvent.BookmarksIndex] as BookmarkIndexContent; + expect(idx.bookmark_ids).toContain(item.bookmark_id); + }); +}); + +// --------------------------------------------------------------------------- +// removeBookmark +// --------------------------------------------------------------------------- + +describe('removeBookmark', () => { + it('removes the bookmark ID from the index', async () => { + const item = makeItem(); + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: [item.bookmark_id] }), + [bookmarkItemEventType(item.bookmark_id)]: item, + }); + + await removeBookmark(mx, item.bookmark_id); + + const idx = (mx as any)._store[AccountDataEvent.BookmarksIndex] as BookmarkIndexContent; + expect(idx.bookmark_ids).not.toContain(item.bookmark_id); + }); + + it('soft-deletes the item event (sets deleted: true)', async () => { + const item = makeItem(); + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: [item.bookmark_id] }), + [bookmarkItemEventType(item.bookmark_id)]: item, + }); + + await removeBookmark(mx, item.bookmark_id); + + const stored = (mx as any)._store[ + bookmarkItemEventType(item.bookmark_id) + ] as BookmarkItemContent; + expect(stored.deleted).toBe(true); + }); + + it('increments the index revision', async () => { + const item = makeItem(); + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ + bookmark_ids: [item.bookmark_id], + revision: 3, + }), + [bookmarkItemEventType(item.bookmark_id)]: item, + }); + + await removeBookmark(mx, item.bookmark_id); + + const idx = (mx as any)._store[AccountDataEvent.BookmarksIndex] as BookmarkIndexContent; + expect(idx.revision).toBe(4); + }); + + it('succeeds without error when the item event does not exist', async () => { + const item = makeItem(); + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: [item.bookmark_id] }), + // No item event stored + }); + + await expect(removeBookmark(mx, item.bookmark_id)).resolves.not.toThrow(); + }); + + it('leaves the index unchanged when the ID was not present', async () => { + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: ['bmk_aaaabbbb'] }), + }); + + await removeBookmark(mx, 'bmk_nonexistent'); + + const idx = (mx as any)._store[AccountDataEvent.BookmarksIndex] as BookmarkIndexContent; + expect(idx.bookmark_ids).toEqual(['bmk_aaaabbbb']); + }); +}); + +// --------------------------------------------------------------------------- +// listBookmarks +// --------------------------------------------------------------------------- + +describe('listBookmarks', () => { + it('returns an empty array when there is no index', () => { + const mx = makeClient(); + expect(listBookmarks(mx)).toEqual([]); + }); + + it('returns active items in index order', () => { + const a = makeItem({ bookmark_id: 'bmk_aaaaaaaa' }); + const b = makeItem({ bookmark_id: 'bmk_bbbbbbbb' }); + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ + bookmark_ids: [a.bookmark_id, b.bookmark_id], + }), + [bookmarkItemEventType(a.bookmark_id)]: a, + [bookmarkItemEventType(b.bookmark_id)]: b, + }); + + const result = listBookmarks(mx); + expect(result.map((i) => i.bookmark_id)).toEqual([a.bookmark_id, b.bookmark_id]); + }); + + it('skips items that are soft-deleted (deleted: true)', () => { + const item = makeItem({ deleted: true }); + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: [item.bookmark_id] }), + [bookmarkItemEventType(item.bookmark_id)]: item, + }); + + expect(listBookmarks(mx)).toEqual([]); + }); + + it('skips item IDs whose event is missing from account data', () => { + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: ['bmk_orphaned'] }), + // No item event + }); + + expect(listBookmarks(mx)).toEqual([]); + }); + + it('deduplicates IDs that appear more than once in bookmark_ids', () => { + const item = makeItem(); + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ + bookmark_ids: [item.bookmark_id, item.bookmark_id], + }), + [bookmarkItemEventType(item.bookmark_id)]: item, + }); + + expect(listBookmarks(mx)).toHaveLength(1); + }); + + it('skips malformed item events', () => { + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: ['bmk_bad'] }), + [bookmarkItemEventType('bmk_bad')]: { not_a_valid: 'item' }, + }); + + expect(listBookmarks(mx)).toEqual([]); + }); + + it('recovers orphaned items whose event exists but ID is absent from the index', () => { + // Simulate a concurrent-write race: device A's bookmark_id was dropped from the + // index by a last-write-wins overwrite, but the item event still exists. + const orphan = makeItem({ bookmark_id: 'bmk_orphan1' }); + const indexed = makeItem({ bookmark_id: 'bmk_indexed' }); + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: [indexed.bookmark_id] }), + [bookmarkItemEventType(indexed.bookmark_id)]: indexed, + [bookmarkItemEventType(orphan.bookmark_id)]: orphan, + }); + + const result = listBookmarks(mx); + expect(result.map((i) => i.bookmark_id)).toContain(orphan.bookmark_id); + expect(result.map((i) => i.bookmark_id)).toContain(indexed.bookmark_id); + // Indexed item should appear before the orphan + expect(result[0].bookmark_id).toBe(indexed.bookmark_id); + }); + + it('does not return soft-deleted orphaned items', () => { + const orphan = makeItem({ bookmark_id: 'bmk_orphan2', deleted: true }); + const mx = makeClient({ + // No index entry for the orphan — deleted orphan should still be skipped + [bookmarkItemEventType(orphan.bookmark_id)]: orphan, + }); + + expect(listBookmarks(mx)).toEqual([]); + }); +}); + +// --------------------------------------------------------------------------- +// isBookmarked +// --------------------------------------------------------------------------- + +describe('isBookmarked', () => { + it('returns true when the ID is in the index', () => { + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: ['bmk_aabbccdd'] }), + }); + expect(isBookmarked(mx, 'bmk_aabbccdd')).toBe(true); + }); + + it('returns false when the ID is not in the index', () => { + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: ['bmk_aabbccdd'] }), + }); + expect(isBookmarked(mx, 'bmk_ffffffff')).toBe(false); + }); + + it('returns false when there is no index', () => { + const mx = makeClient(); + expect(isBookmarked(mx, 'bmk_aabbccdd')).toBe(false); + }); +}); diff --git a/src/app/state/bookmarks.test.ts b/src/app/state/bookmarks.test.ts new file mode 100644 index 000000000..1145b9d1e --- /dev/null +++ b/src/app/state/bookmarks.test.ts @@ -0,0 +1,69 @@ +/** + * Unit tests for the Jotai bookmark atoms in src/app/state/bookmarks.ts. + * + * The derived `bookmarkIdSetAtom` is the only atom with non-trivial logic — + * it builds an O(1) lookup Set from the bookmark list. The primitive atoms + * (`bookmarkListAtom`, `bookmarkLoadingAtom`) are default Jotai atoms whose + * read/write semantics are provided by the library itself and do not need + * additional testing. + */ +import { describe, it, expect } from 'vitest'; +import { createStore } from 'jotai'; +import { bookmarkIdSetAtom, bookmarkListAtom } from './bookmarks'; +import type { BookmarkItemContent } from '../features/bookmarks/bookmarkDomain'; + +// Helper: minimal valid bookmark item +function makeItem(id: string): BookmarkItemContent { + return { + version: 1, + bookmark_id: id, + uri: `matrix:roomid/foo/e/${id}`, + room_id: '!room:s', + event_id: `$${id}:s`, + event_ts: 1_000_000, + bookmarked_ts: 2_000_000, + }; +} + +describe('bookmarkIdSetAtom (derived)', () => { + it('returns an empty Set when the list is empty', () => { + const store = createStore(); + const set = store.get(bookmarkIdSetAtom); + expect(set.size).toBe(0); + }); + + it('contains the IDs of every item in bookmarkListAtom', () => { + const store = createStore(); + store.set(bookmarkListAtom, [makeItem('bmk_aaaaaaaa'), makeItem('bmk_bbbbbbbb')]); + + const set = store.get(bookmarkIdSetAtom); + expect(set.has('bmk_aaaaaaaa')).toBe(true); + expect(set.has('bmk_bbbbbbbb')).toBe(true); + }); + + it('does not contain IDs not in the list', () => { + const store = createStore(); + store.set(bookmarkListAtom, [makeItem('bmk_aaaaaaaa')]); + + const set = store.get(bookmarkIdSetAtom); + expect(set.has('bmk_ffffffff')).toBe(false); + }); + + it('updates reactively when the list changes', () => { + const store = createStore(); + store.set(bookmarkListAtom, [makeItem('bmk_11111111')]); + + expect(store.get(bookmarkIdSetAtom).has('bmk_11111111')).toBe(true); + + store.set(bookmarkListAtom, []); + expect(store.get(bookmarkIdSetAtom).has('bmk_11111111')).toBe(false); + }); + + it('returns a Set whose size equals the number of unique items', () => { + const store = createStore(); + const items = [makeItem('bmk_aaaaaaaa'), makeItem('bmk_bbbbbbbb'), makeItem('bmk_cccccccc')]; + store.set(bookmarkListAtom, items); + + expect(store.get(bookmarkIdSetAtom).size).toBe(3); + }); +}); From 2a9fa5206c580bbd793ca06a71cee4781da30209 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Mon, 6 Apr 2026 12:46:48 -0400 Subject: [PATCH 03/17] chore: add changeset for message-bookmarks --- .changeset/message-bookmarks.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/message-bookmarks.md diff --git a/.changeset/message-bookmarks.md b/.changeset/message-bookmarks.md new file mode 100644 index 000000000..9ca1cf6c3 --- /dev/null +++ b/.changeset/message-bookmarks.md @@ -0,0 +1,5 @@ +--- +default: minor +--- + +Add message bookmarks (MSC4438). Users can bookmark messages for easy retrieval via a new Bookmarks section in the home sidebar. Gated by an operator `config.json` experiment flag (`experiments.messageBookmarks`) and a per-user experimental settings toggle. From 1ad6b038c56bfedff0381fe78974ebf160a62f47 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Wed, 8 Apr 2026 16:42:01 -0400 Subject: [PATCH 04/17] fix(bookmarks): add missing focusId to MSC4438 settings SettingTile --- .../features/settings/experimental/MSC4438MessageBookmarks.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/app/features/settings/experimental/MSC4438MessageBookmarks.tsx b/src/app/features/settings/experimental/MSC4438MessageBookmarks.tsx index 45e429b18..0751a5578 100644 --- a/src/app/features/settings/experimental/MSC4438MessageBookmarks.tsx +++ b/src/app/features/settings/experimental/MSC4438MessageBookmarks.tsx @@ -16,6 +16,7 @@ export function MSC4438MessageBookmarks() { Message Bookmarks From 755588b737a3fb8fef3694e35f2d445637bb9a58 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Thu, 9 Apr 2026 09:23:25 -0400 Subject: [PATCH 05/17] fix(bookmarks): soft-delete item before updating index in removeBookmark --- src/app/features/bookmarks/bookmarkRepository.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/app/features/bookmarks/bookmarkRepository.ts b/src/app/features/bookmarks/bookmarkRepository.ts index d6703777f..e6d712dbd 100644 --- a/src/app/features/bookmarks/bookmarkRepository.ts +++ b/src/app/features/bookmarks/bookmarkRepository.ts @@ -77,17 +77,19 @@ export async function addBookmark(mx: MatrixClient, item: BookmarkItemContent): * removed. */ export async function removeBookmark(mx: MatrixClient, bookmarkId: string): Promise { + // Soft-delete the item FIRST — mirrors the item-before-index ordering of addBookmark. + // If writeIndex ran first, orphan recovery in listBookmarks() would transiently resurface the + // bookmark (item not yet deleted, but ID also not in index) between the two writes. + const existing = readItem(mx, bookmarkId); + if (existing) { + await writeItem(mx, { ...existing, deleted: true }); + } + const index = readIndex(mx); index.bookmark_ids = index.bookmark_ids.filter((id) => id !== bookmarkId); index.revision += 1; index.updated_ts = Date.now(); await writeIndex(mx, index); - - // Soft-delete the item event - const existing = readItem(mx, bookmarkId); - if (existing) { - await writeItem(mx, { ...existing, deleted: true }); - } } /** From 1ba25f8d18e5d4c9ead232e7b697dbf1e62c3c13 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Sun, 12 Apr 2026 10:53:57 -0400 Subject: [PATCH 06/17] fix(bookmarks): wire useInitBookmarks and fix orphan tombstoning - Mount BookmarksFeature in ClientNonUIFeatures so useInitBookmarks() is called; bookmarkListAtom was never populated, causing the viewer to always show "No Bookmarks Yet" even with data on the server. - Fix removeBookmark to tombstone any existing item event regardless of whether it passes validation, so malformed/orphan bmk_ items cannot be resurrected by orphan recovery in listBookmarks. - Add regression tests: tombstoning malformed item, idempotent tombstoning. --- .../bookmarks/bookmarkRepository.test.ts | 29 +++++++++++++++++++ .../features/bookmarks/bookmarkRepository.ts | 18 ++++++++---- src/app/pages/client/ClientNonUIFeatures.tsx | 8 +++-- 3 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/app/features/bookmarks/bookmarkRepository.test.ts b/src/app/features/bookmarks/bookmarkRepository.test.ts index 7b928b50e..3eeaaca65 100644 --- a/src/app/features/bookmarks/bookmarkRepository.test.ts +++ b/src/app/features/bookmarks/bookmarkRepository.test.ts @@ -202,6 +202,35 @@ describe('removeBookmark', () => { await expect(removeBookmark(mx, item.bookmark_id)).resolves.not.toThrow(); }); + it('tombstones a malformed item event (sets deleted: true even when validation fails)', async () => { + // A malformed item exists in account data (e.g. written by a buggy client). + // removeBookmark must still tombstone it so orphan recovery does not resurrect it. + const badContent = { not_a_valid: 'item' }; + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: ['bmk_bad'] }), + [bookmarkItemEventType('bmk_bad')]: badContent, + }); + + await removeBookmark(mx, 'bmk_bad'); + + const stored = (mx as any)._store[bookmarkItemEventType('bmk_bad')]; + expect(stored.deleted).toBe(true); + }); + + it('tombstones an already-deleted item event (idempotent)', async () => { + // If for any reason the same bookmark is removed twice, the tombstone write + // should still succeed and the item should remain deleted. + const item = makeItem({ deleted: true }); + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: [item.bookmark_id] }), + [bookmarkItemEventType(item.bookmark_id)]: item, + }); + + await expect(removeBookmark(mx, item.bookmark_id)).resolves.not.toThrow(); + const stored = (mx as any)._store[bookmarkItemEventType(item.bookmark_id)] as BookmarkItemContent; + expect(stored.deleted).toBe(true); + }); + it('leaves the index unchanged when the ID was not present', async () => { const mx = makeClient({ [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: ['bmk_aaaabbbb'] }), diff --git a/src/app/features/bookmarks/bookmarkRepository.ts b/src/app/features/bookmarks/bookmarkRepository.ts index e6d712dbd..d1a30370d 100644 --- a/src/app/features/bookmarks/bookmarkRepository.ts +++ b/src/app/features/bookmarks/bookmarkRepository.ts @@ -77,12 +77,18 @@ export async function addBookmark(mx: MatrixClient, item: BookmarkItemContent): * removed. */ export async function removeBookmark(mx: MatrixClient, bookmarkId: string): Promise { - // Soft-delete the item FIRST — mirrors the item-before-index ordering of addBookmark. - // If writeIndex ran first, orphan recovery in listBookmarks() would transiently resurface the - // bookmark (item not yet deleted, but ID also not in index) between the two writes. - const existing = readItem(mx, bookmarkId); - if (existing) { - await writeItem(mx, { ...existing, deleted: true }); + // Tombstone the item event directly — bypass readItem()'s validation so that + // malformed or already-deleted items still get marked deleted: true. Without + // this, orphan recovery can resurrect items whose deletion write failed halfway. + const evt = mx.getAccountData(bookmarkItemEventType(bookmarkId) as any); + const raw = evt?.getContent(); + if (raw != null) { + // Write using the bookmarkId param as the canonical type key, not item.bookmark_id, + // so malformed items (missing bookmark_id field) still get the right event type. + await mx.setAccountData( + bookmarkItemEventType(bookmarkId) as any, + { ...(raw as object), deleted: true } as any + ); } const index = readIndex(mx); diff --git a/src/app/pages/client/ClientNonUIFeatures.tsx b/src/app/pages/client/ClientNonUIFeatures.tsx index caebe459a..ca0b82b81 100644 --- a/src/app/pages/client/ClientNonUIFeatures.tsx +++ b/src/app/pages/client/ClientNonUIFeatures.tsx @@ -514,8 +514,12 @@ function MessageNotifications() { }); } - // In-app audio: play when notification sounds are enabled AND this notification is loud. - if (notificationSound && isLoud) { + // In-app audio: play when the app is in the foreground (has focus) and + // notification sounds are enabled for this notification type. + // Gating on hasFocus() rather than just visibilityState prevents a race + // where the page is still 'visible' for a brief window after the user + // backgrounds the app on mobile — hasFocus() flips false first. + if (notificationSound && isLoud && document.hasFocus()) { playSound(); } }; From 594fda08df6860d9909cc4ea218ab6419c92512f Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Sun, 12 Apr 2026 14:52:58 -0400 Subject: [PATCH 07/17] fix(bookmarks): strip deleted flag on re-add to guarantee re-activation --- .../bookmarks/bookmarkRepository.test.ts | 29 +++++++++++++++++++ .../features/bookmarks/bookmarkRepository.ts | 11 +++++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/app/features/bookmarks/bookmarkRepository.test.ts b/src/app/features/bookmarks/bookmarkRepository.test.ts index 3eeaaca65..54e3d85b8 100644 --- a/src/app/features/bookmarks/bookmarkRepository.test.ts +++ b/src/app/features/bookmarks/bookmarkRepository.test.ts @@ -141,6 +141,35 @@ describe('addBookmark', () => { const idx = (mx as any)._store[AccountDataEvent.BookmarksIndex] as BookmarkIndexContent; expect(idx.bookmark_ids).toContain(item.bookmark_id); }); + + it('re-activates a tombstoned bookmark (strips deleted: true)', async () => { + const tombstoned = makeItem({ deleted: true }); + const mx2 = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: [] }), + [bookmarkItemEventType(tombstoned.bookmark_id)]: tombstoned, + }); + + // Re-add with a fresh item (same bookmark_id, no deleted flag) + const freshItem = makeItem(); + await addBookmark(mx2, freshItem); + + const stored = (mx2 as any)._store[ + bookmarkItemEventType(freshItem.bookmark_id) + ] as BookmarkItemContent; + expect(stored.deleted).toBeUndefined(); + const idx = (mx2 as any)._store[AccountDataEvent.BookmarksIndex] as BookmarkIndexContent; + expect(idx.bookmark_ids).toContain(freshItem.bookmark_id); + }); + + it('strips deleted: true even when the item passed in carries the flag', async () => { + const item = makeItem({ deleted: true }); + await addBookmark(mx, item); + + const stored = (mx as any)._store[ + bookmarkItemEventType(item.bookmark_id) + ] as BookmarkItemContent; + expect(stored.deleted).toBeUndefined(); + }); }); // --------------------------------------------------------------------------- diff --git a/src/app/features/bookmarks/bookmarkRepository.ts b/src/app/features/bookmarks/bookmarkRepository.ts index d1a30370d..241430e76 100644 --- a/src/app/features/bookmarks/bookmarkRepository.ts +++ b/src/app/features/bookmarks/bookmarkRepository.ts @@ -44,17 +44,22 @@ async function writeItem(mx: MatrixClient, item: BookmarkItemContent): Promise { + // Strip deleted so that re-bookmarking a previously removed message always + // produces an active item, even if a stale tombstoned item is passed in. + const { deleted, ...activeItem } = item; // Write item before updating index (cross-device consistency) - await writeItem(mx, item); + await writeItem(mx, activeItem as BookmarkItemContent); const index = readIndex(mx); if (!index.bookmark_ids.includes(item.bookmark_id)) { From 31441add1d6a7ce0d2e553be855a8e787f3eb88a Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Sun, 12 Apr 2026 15:15:01 -0400 Subject: [PATCH 08/17] feat(bookmarks): show Recently Removed section with Restore button --- .../bookmarks/bookmarkRepository.test.ts | 77 ++++++++++++++++++- .../features/bookmarks/bookmarkRepository.ts | 39 ++++++++++ src/app/features/bookmarks/useBookmarks.ts | 47 +++++++++-- .../features/bookmarks/useInitBookmarks.ts | 8 +- .../pages/client/bookmarks/BookmarksList.tsx | 70 ++++++++++++++++- src/app/state/bookmarks.ts | 7 ++ 6 files changed, 235 insertions(+), 13 deletions(-) diff --git a/src/app/features/bookmarks/bookmarkRepository.test.ts b/src/app/features/bookmarks/bookmarkRepository.test.ts index 54e3d85b8..d9724cf67 100644 --- a/src/app/features/bookmarks/bookmarkRepository.test.ts +++ b/src/app/features/bookmarks/bookmarkRepository.test.ts @@ -8,7 +8,13 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import type { MatrixClient } from '$types/matrix-sdk'; import { AccountDataEvent } from '$types/matrix/accountData'; -import { addBookmark, removeBookmark, listBookmarks, isBookmarked } from './bookmarkRepository'; +import { + addBookmark, + removeBookmark, + listBookmarks, + listDeletedBookmarks, + isBookmarked, +} from './bookmarkRepository'; import { bookmarkItemEventType, emptyIndex, @@ -366,6 +372,75 @@ describe('listBookmarks', () => { }); }); +// --------------------------------------------------------------------------- +// listDeletedBookmarks +// --------------------------------------------------------------------------- + +describe('listDeletedBookmarks', () => { + it('returns an empty array when there are no tombstoned items', () => { + const item = makeItem(); + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: [item.bookmark_id] }), + [bookmarkItemEventType(item.bookmark_id)]: item, + }); + expect(listDeletedBookmarks(mx)).toEqual([]); + }); + + it('returns index-referenced items that are tombstoned (partial remove failure)', () => { + const item = makeItem({ deleted: true }); + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: [item.bookmark_id] }), + [bookmarkItemEventType(item.bookmark_id)]: item, + }); + const result = listDeletedBookmarks(mx); + expect(result).toHaveLength(1); + expect(result[0].bookmark_id).toBe(item.bookmark_id); + }); + + it('returns orphan tombstones not in the index (normal remove path)', () => { + const item = makeItem({ bookmark_id: 'bmk_orphan99', deleted: true }); + const mx = makeClient({ + // ID intentionally absent from the index + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: [] }), + [bookmarkItemEventType(item.bookmark_id)]: item, + }); + const result = listDeletedBookmarks(mx); + expect(result).toHaveLength(1); + expect(result[0].bookmark_id).toBe(item.bookmark_id); + }); + + it('does not return active (non-deleted) items', () => { + const active = makeItem(); + const deleted = makeItem({ bookmark_id: 'bmk_deleted1', deleted: true }); + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: [active.bookmark_id] }), + [bookmarkItemEventType(active.bookmark_id)]: active, + [bookmarkItemEventType(deleted.bookmark_id)]: deleted, + }); + const result = listDeletedBookmarks(mx); + expect(result.map((i) => i.bookmark_id)).not.toContain(active.bookmark_id); + expect(result.map((i) => i.bookmark_id)).toContain(deleted.bookmark_id); + }); + + it('deduplicates when the same ID appears in both index and orphan scan', () => { + const item = makeItem({ deleted: true }); + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: [item.bookmark_id] }), + [bookmarkItemEventType(item.bookmark_id)]: item, + }); + const result = listDeletedBookmarks(mx); + expect(result).toHaveLength(1); + }); + + it('skips malformed item events even if deleted: true', () => { + const mx = makeClient({ + [AccountDataEvent.BookmarksIndex]: makeIndex({ bookmark_ids: [] }), + [bookmarkItemEventType('bmk_bad')]: { deleted: true, not_valid: 'junk' }, + }); + expect(listDeletedBookmarks(mx)).toEqual([]); + }); +}); + // --------------------------------------------------------------------------- // isBookmarked // --------------------------------------------------------------------------- diff --git a/src/app/features/bookmarks/bookmarkRepository.ts b/src/app/features/bookmarks/bookmarkRepository.ts index 241430e76..bd9cda928 100644 --- a/src/app/features/bookmarks/bookmarkRepository.ts +++ b/src/app/features/bookmarks/bookmarkRepository.ts @@ -146,6 +146,45 @@ export function listBookmarks(mx: MatrixClient): BookmarkItemContent[] { return items; } +/** + * List all deleted (tombstoned) bookmark items. + * + * Includes both: + * - Items still referenced in the index whose item event carries deleted: true + * (arises when the index write fails after a soft-delete). + * - Orphaned tombstones whose ID has already been removed from the index + * (the normal case after a successful remove). + * + * Results are deduplicated and include only items that pass isValidBookmarkItem + * (ensuring enough stored metadata is available to display and restore them). + */ +export function listDeletedBookmarks(mx: MatrixClient): BookmarkItemContent[] { + const index = readIndex(mx); + const results: BookmarkItemContent[] = []; + const seen = new Set(); + + // 1. Index-referenced items that are tombstoned (partial remove failure) + index.bookmark_ids.forEach((id) => { + if (seen.has(id)) return; + seen.add(id); + const content = mx.getAccountData(bookmarkItemEventType(id) as any)?.getContent(); + if (isValidBookmarkItem(content) && content.deleted === true) results.push(content); + }); + + // 2. Orphan tombstones (properly removed from index but item event persists) + const prefix = AccountDataEvent.BookmarkItemPrefix as string; + Array.from(mx.store.accountData.keys()).forEach((key) => { + if (!key.startsWith(prefix)) return; + const bookmarkId = key.slice(prefix.length); + if (seen.has(bookmarkId)) return; + seen.add(bookmarkId); + const content = mx.getAccountData(key as any)?.getContent(); + if (isValidBookmarkItem(content) && content.deleted === true) results.push(content); + }); + + return results; +} + /** * Check whether a specific bookmark ID is in the index. * diff --git a/src/app/features/bookmarks/useBookmarks.ts b/src/app/features/bookmarks/useBookmarks.ts index aadd0bb8f..df9346dfd 100644 --- a/src/app/features/bookmarks/useBookmarks.ts +++ b/src/app/features/bookmarks/useBookmarks.ts @@ -1,15 +1,31 @@ import { useAtomValue, useSetAtom } from 'jotai'; import { useCallback } from 'react'; import { useMatrixClient } from '$hooks/useMatrixClient'; -import { bookmarkIdSetAtom, bookmarkListAtom, bookmarkLoadingAtom } from '$state/bookmarks'; +import { + bookmarkDeletedListAtom, + bookmarkIdSetAtom, + bookmarkListAtom, + bookmarkLoadingAtom, +} from '$state/bookmarks'; import { BookmarkItemContent, computeBookmarkId } from './bookmarkDomain'; -import { addBookmark, removeBookmark, listBookmarks, isBookmarked } from './bookmarkRepository'; +import { + addBookmark, + listBookmarks, + listDeletedBookmarks, + removeBookmark, + isBookmarked, +} from './bookmarkRepository'; /** Returns the current ordered bookmark list. */ export function useBookmarkList(): BookmarkItemContent[] { return useAtomValue(bookmarkListAtom); } +/** Returns deleted (tombstoned) bookmarks that can be restored. */ +export function useBookmarkDeletedList(): BookmarkItemContent[] { + return useAtomValue(bookmarkDeletedListAtom); +} + /** Returns true while a bookmark refresh is in progress. */ export function useBookmarkLoading(): boolean { return useAtomValue(bookmarkLoadingAtom); @@ -35,28 +51,30 @@ export function useIsBookmarked(roomId: string, eventId: string): boolean { export function useBookmarkActions() { const mx = useMatrixClient(); const setList = useSetAtom(bookmarkListAtom); + const setDeletedList = useSetAtom(bookmarkDeletedListAtom); const setLoading = useSetAtom(bookmarkLoadingAtom); const refresh = useCallback(async () => { setLoading(true); try { - const items = listBookmarks(mx); - setList(items); + setList(listBookmarks(mx)); + setDeletedList(listDeletedBookmarks(mx)); } finally { setLoading(false); } - }, [mx, setList, setLoading]); + }, [mx, setList, setDeletedList, setLoading]); const add = useCallback( async (item: BookmarkItemContent) => { - // Optimistic update + // Optimistic update: add to active list, remove from deleted list setList((prev) => { if (prev.some((b) => b.bookmark_id === item.bookmark_id)) return prev; return [item, ...prev]; }); + setDeletedList((prev) => prev.filter((b) => b.bookmark_id !== item.bookmark_id)); await addBookmark(mx, item); }, - [mx, setList] + [mx, setList, setDeletedList] ); const remove = useCallback( @@ -68,11 +86,24 @@ export function useBookmarkActions() { [mx, setList] ); + const restore = useCallback( + async (item: BookmarkItemContent) => { + // Optimistic update: move from deleted list to active list + setDeletedList((prev) => prev.filter((b) => b.bookmark_id !== item.bookmark_id)); + setList((prev) => { + if (prev.some((b) => b.bookmark_id === item.bookmark_id)) return prev; + return [item, ...prev]; + }); + await addBookmark(mx, item); // strips deleted flag + }, + [mx, setList, setDeletedList] + ); + const checkIsBookmarked = useCallback( (roomId: string, eventId: string): boolean => isBookmarked(mx, computeBookmarkId(roomId, eventId)), [mx] ); - return { refresh, add, remove, checkIsBookmarked }; + return { refresh, add, remove, restore, checkIsBookmarked }; } diff --git a/src/app/features/bookmarks/useInitBookmarks.ts b/src/app/features/bookmarks/useInitBookmarks.ts index 40175ce58..3b6cb2247 100644 --- a/src/app/features/bookmarks/useInitBookmarks.ts +++ b/src/app/features/bookmarks/useInitBookmarks.ts @@ -4,9 +4,9 @@ import { useSetAtom } from 'jotai'; import { useMatrixClient } from '$hooks/useMatrixClient'; import { useSyncState } from '$hooks/useSyncState'; import { useAccountDataCallback } from '$hooks/useAccountDataCallback'; -import { bookmarkListAtom, bookmarkLoadingAtom } from '$state/bookmarks'; +import { bookmarkDeletedListAtom, bookmarkListAtom, bookmarkLoadingAtom } from '$state/bookmarks'; import { AccountDataEvent } from '$types/matrix/accountData'; -import { listBookmarks } from './bookmarkRepository'; +import { listBookmarks, listDeletedBookmarks } from './bookmarkRepository'; /** * Top-level hook that keeps `bookmarkListAtom` in sync with account data. @@ -24,16 +24,18 @@ import { listBookmarks } from './bookmarkRepository'; export function useInitBookmarks(): void { const mx = useMatrixClient(); const setList = useSetAtom(bookmarkListAtom); + const setDeletedList = useSetAtom(bookmarkDeletedListAtom); const setLoading = useSetAtom(bookmarkLoadingAtom); const loadBookmarks = useCallback(() => { setLoading(true); try { setList(listBookmarks(mx)); + setDeletedList(listDeletedBookmarks(mx)); } finally { setLoading(false); } - }, [mx, setList, setLoading]); + }, [mx, setList, setDeletedList, setLoading]); // Immediate load: fires once on mount to cover the case where ClientNonUIFeatures // mounts after the initial SyncState.Syncing transition has already fired. diff --git a/src/app/pages/client/bookmarks/BookmarksList.tsx b/src/app/pages/client/bookmarks/BookmarksList.tsx index 6dae4fb5c..562a4920c 100644 --- a/src/app/pages/client/bookmarks/BookmarksList.tsx +++ b/src/app/pages/client/bookmarks/BookmarksList.tsx @@ -48,6 +48,7 @@ import { stopPropagation } from '$utils/keyboard'; import { BookmarkItemContent } from '$features/bookmarks/bookmarkDomain'; import { useBookmarkActions, + useBookmarkDeletedList, useBookmarkList, useBookmarkLoading, } from '$features/bookmarks/useBookmarks'; @@ -298,6 +299,45 @@ function BookmarkResultGroup({ ); } +// --------------------------------------------------------------------------- +// RemovedBookmarkRow +// --------------------------------------------------------------------------- + +type RemovedBookmarkRowProps = { + item: BookmarkItemContent; + onRestore: (item: BookmarkItemContent) => void; +}; + +function RemovedBookmarkRow({ item, onRestore }: RemovedBookmarkRowProps) { + const mx = useMatrixClient(); + const room = mx.getRoom(item.room_id) ?? undefined; + const roomName = room?.name ?? item.room_name ?? item.room_id; + + return ( + + + + + {roomName} + + {item.body_preview && ( + + {item.body_preview} + + )} + + onRestore(item)} variant="Secondary" radii="400"> + Restore + + + + ); +} + // --------------------------------------------------------------------------- // BookmarkFilterInput // --------------------------------------------------------------------------- @@ -379,8 +419,9 @@ export function BookmarksList() { const [dateFormatString] = useSetting(settingsAtom, 'dateFormatString'); const bookmarks = useBookmarkList(); + const deletedBookmarks = useBookmarkDeletedList(); const loading = useBookmarkLoading(); - const { remove } = useBookmarkActions(); + const { remove, restore } = useBookmarkActions(); const [filterTerm, setFilterTerm] = useState(); const [removingItem, setRemovingItem] = useState(); @@ -432,6 +473,13 @@ export function BookmarksList() { setRemovingItem(undefined); }, [removingItem, remove]); + const handleRestore = useCallback( + async (item: BookmarkItemContent) => { + await restore(item); + }, + [restore] + ); + const handleFilter = useCallback((term: string) => { setFilterTerm(term); }, []); @@ -530,6 +578,26 @@ export function BookmarksList() { ))} )} + + {deletedBookmarks.length > 0 && !filterTerm && ( + + +
+ + Recently Removed + +
+ + {deletedBookmarks.map((item) => ( + + ))} + + + )} diff --git a/src/app/state/bookmarks.ts b/src/app/state/bookmarks.ts index 7a2375691..4d6c2b19f 100644 --- a/src/app/state/bookmarks.ts +++ b/src/app/state/bookmarks.ts @@ -4,6 +4,13 @@ import { BookmarkItemContent } from '../features/bookmarks/bookmarkDomain'; /** Ordered list of active bookmark items (mirrors the server index order). */ export const bookmarkListAtom = atom([]); +/** + * Ordered list of deleted (tombstoned) bookmark items that are recoverable. + * Populated alongside bookmarkListAtom so the UI can show a "Recently Removed" + * section with a Restore button for each entry. + */ +export const bookmarkDeletedListAtom = atom([]); + /** True while a refresh from account data is in progress. */ export const bookmarkLoadingAtom = atom(false); From e4d24b2df6ec91e374f3636105f66d6280827bc5 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Mon, 13 Apr 2026 01:48:18 -0400 Subject: [PATCH 09/17] test(bookmarks): format repository tests for prettier compliance --- src/app/features/bookmarks/bookmarkRepository.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/app/features/bookmarks/bookmarkRepository.test.ts b/src/app/features/bookmarks/bookmarkRepository.test.ts index d9724cf67..0489fe692 100644 --- a/src/app/features/bookmarks/bookmarkRepository.test.ts +++ b/src/app/features/bookmarks/bookmarkRepository.test.ts @@ -262,7 +262,9 @@ describe('removeBookmark', () => { }); await expect(removeBookmark(mx, item.bookmark_id)).resolves.not.toThrow(); - const stored = (mx as any)._store[bookmarkItemEventType(item.bookmark_id)] as BookmarkItemContent; + const stored = (mx as any)._store[ + bookmarkItemEventType(item.bookmark_id) + ] as BookmarkItemContent; expect(stored.deleted).toBe(true); }); From a2d568350c03af53c64360d7947d2023ad09f73f Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Mon, 13 Apr 2026 16:27:43 -0400 Subject: [PATCH 10/17] fix(bookmarks): react to item-level account data events and fix remove() optimistic update - useInitBookmarks: also trigger loadBookmarks on bookmark item events (prefix org.matrix.msc4438.bookmark.*), not just the index event. Fixes cross-device sync not updating the UI when sliding sync delivers individual item events. - useBookmarks: remove() now moves the item to bookmarkDeletedListAtom optimistically so the Recently Removed section updates immediately. - Added regression tests for both fixes. --- .../features/bookmarks/useBookmarks.test.tsx | 132 +++++++++++++++ src/app/features/bookmarks/useBookmarks.ts | 15 +- .../bookmarks/useInitBookmarks.test.tsx | 155 ++++++++++++++++++ .../features/bookmarks/useInitBookmarks.ts | 10 +- 4 files changed, 307 insertions(+), 5 deletions(-) create mode 100644 src/app/features/bookmarks/useBookmarks.test.tsx create mode 100644 src/app/features/bookmarks/useInitBookmarks.test.tsx diff --git a/src/app/features/bookmarks/useBookmarks.test.tsx b/src/app/features/bookmarks/useBookmarks.test.tsx new file mode 100644 index 000000000..e09f446c2 --- /dev/null +++ b/src/app/features/bookmarks/useBookmarks.test.tsx @@ -0,0 +1,132 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { renderHook, act } from '@testing-library/react'; +import { createStore, Provider } from 'jotai'; +import { createElement, type ReactNode } from 'react'; +import { bookmarkListAtom, bookmarkDeletedListAtom } from '$state/bookmarks'; +import { useBookmarkActions } from './useBookmarks'; +import type { BookmarkItemContent } from './bookmarkDomain'; + +// --------------------------------------------------------------------------- +// Mocks +// --------------------------------------------------------------------------- + +const { mockMx } = vi.hoisted(() => { + const store: Record = {}; + return { + mockMx: { + getAccountData: vi.fn((type: string) => { + const content = store[type]; + if (!content) return undefined; + return { getContent: () => content }; + }), + setAccountData: vi.fn(async (type: string, content: unknown) => { + store[type] = content; + }), + store: { accountData: new Map() }, + }, + }; +}); + +vi.mock('$hooks/useMatrixClient', () => ({ + useMatrixClient: () => mockMx, +})); + +// Mock the repository so removeBookmark doesn't try to read real account data +vi.mock('./bookmarkRepository', async (importOriginal) => { + const orig = await importOriginal(); + return { + ...orig, + removeBookmark: vi.fn(async () => {}), + addBookmark: vi.fn(async () => {}), + }; +}); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeItem(id: string): BookmarkItemContent { + return { + version: 1, + bookmark_id: id, + uri: `matrix:roomid/foo/e/${id}`, + room_id: '!room:s', + event_id: `$${id}:s`, + event_ts: 1_000, + bookmarked_ts: 2_000, + }; +} + +function makeWrapper(store: ReturnType) { + return function Wrapper({ children }: { children: ReactNode }) { + return createElement(Provider, { store }, children); + }; +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('useBookmarkActions.remove', () => { + let store: ReturnType; + + beforeEach(() => { + store = createStore(); + }); + + it('moves item from active list to deleted list optimistically', async () => { + const item = makeItem('bmk_1111'); + store.set(bookmarkListAtom, [item]); + store.set(bookmarkDeletedListAtom, []); + + const { result } = renderHook(() => useBookmarkActions(), { + wrapper: makeWrapper(store), + }); + + await act(async () => { + await result.current.remove('bmk_1111'); + }); + + expect(store.get(bookmarkListAtom)).toHaveLength(0); + + const deleted = store.get(bookmarkDeletedListAtom); + expect(deleted).toHaveLength(1); + expect(deleted[0].bookmark_id).toBe('bmk_1111'); + expect(deleted[0].deleted).toBe(true); + }); + + it('does not duplicate item in deleted list if already present', async () => { + const item = makeItem('bmk_2222'); + const deletedItem = { ...item, deleted: true as const }; + store.set(bookmarkListAtom, [item]); + store.set(bookmarkDeletedListAtom, [deletedItem]); + + const { result } = renderHook(() => useBookmarkActions(), { + wrapper: makeWrapper(store), + }); + + await act(async () => { + await result.current.remove('bmk_2222'); + }); + + expect(store.get(bookmarkDeletedListAtom)).toHaveLength(1); + }); + + it('handles removing a non-existent item gracefully', async () => { + store.set(bookmarkListAtom, [makeItem('bmk_3333')]); + store.set(bookmarkDeletedListAtom, []); + + const { result } = renderHook(() => useBookmarkActions(), { + wrapper: makeWrapper(store), + }); + + await act(async () => { + await result.current.remove('bmk_nonexistent'); + }); + + // Original item untouched + expect(store.get(bookmarkListAtom)).toHaveLength(1); + // Nothing added to deleted list since the item wasn't found + expect(store.get(bookmarkDeletedListAtom)).toHaveLength(0); + }); +}); diff --git a/src/app/features/bookmarks/useBookmarks.ts b/src/app/features/bookmarks/useBookmarks.ts index df9346dfd..c6fac5746 100644 --- a/src/app/features/bookmarks/useBookmarks.ts +++ b/src/app/features/bookmarks/useBookmarks.ts @@ -79,11 +79,20 @@ export function useBookmarkActions() { const remove = useCallback( async (bookmarkId: string) => { - // Optimistic update - setList((prev) => prev.filter((b) => b.bookmark_id !== bookmarkId)); + // Optimistic update: move from active list to deleted list + setList((prev) => { + const removed = prev.find((b) => b.bookmark_id === bookmarkId); + if (removed) { + setDeletedList((del) => { + if (del.some((b) => b.bookmark_id === bookmarkId)) return del; + return [{ ...removed, deleted: true }, ...del]; + }); + } + return prev.filter((b) => b.bookmark_id !== bookmarkId); + }); await removeBookmark(mx, bookmarkId); }, - [mx, setList] + [mx, setList, setDeletedList] ); const restore = useCallback( diff --git a/src/app/features/bookmarks/useInitBookmarks.test.tsx b/src/app/features/bookmarks/useInitBookmarks.test.tsx new file mode 100644 index 000000000..893b5d64c --- /dev/null +++ b/src/app/features/bookmarks/useInitBookmarks.test.tsx @@ -0,0 +1,155 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { renderHook } from '@testing-library/react'; +import { createStore, Provider } from 'jotai'; +import { createElement, type ReactNode } from 'react'; +import { bookmarkListAtom, bookmarkDeletedListAtom } from '$state/bookmarks'; +import { useInitBookmarks } from './useInitBookmarks'; +import type { BookmarkItemContent, BookmarkIndexContent } from './bookmarkDomain'; + +// --------------------------------------------------------------------------- +// Mocks +// --------------------------------------------------------------------------- + +const BOOKMARKS_INDEX = 'org.matrix.msc4438.bookmarks.index'; +const BOOKMARK_PREFIX = 'org.matrix.msc4438.bookmark.'; + +const { accountDataCB, syncStateCB, mockMx } = vi.hoisted(() => { + const adCB: { current: ((event: { getType: () => string }) => void) | null } = { current: null }; + const ssCB: { current: ((state: string, prev: string) => void) | null } = { current: null }; + + const item: BookmarkItemContent = { + version: 1, + bookmark_id: 'bmk_aabb', + uri: 'matrix:roomid/foo/e/bar', + room_id: '!room:s', + event_id: '$ev:s', + event_ts: 1_000, + bookmarked_ts: 2_000, + }; + const deletedItem: BookmarkItemContent = { + version: 1, + bookmark_id: 'bmk_ccdd', + uri: 'matrix:roomid/baz/e/qux', + room_id: '!room2:s', + event_id: '$ev2:s', + event_ts: 3_000, + bookmarked_ts: 4_000, + deleted: true, + }; + const index: BookmarkIndexContent = { + version: 1, + revision: 1, + updated_ts: 5_000, + bookmark_ids: ['bmk_aabb', 'bmk_ccdd'], + }; + + const store: Record = { + ['org.matrix.msc4438.bookmarks.index']: index, + ['org.matrix.msc4438.bookmark.bmk_aabb']: item, + ['org.matrix.msc4438.bookmark.bmk_ccdd']: deletedItem, + }; + + const mx = { + getAccountData: vi.fn((type: string) => { + const content = store[type]; + if (!content) return undefined; + return { getContent: () => content }; + }), + setAccountData: vi.fn(), + store: { accountData: new Map(Object.entries(store)) }, + }; + + return { accountDataCB: adCB, syncStateCB: ssCB, mockMx: mx }; +}); + +vi.mock('$hooks/useMatrixClient', () => ({ + useMatrixClient: () => mockMx, +})); + +vi.mock('$hooks/useAccountDataCallback', () => ({ + useAccountDataCallback: ( + _mx: unknown, + cb: (event: { getType: () => string }) => void + ) => { + accountDataCB.current = cb; + }, +})); + +vi.mock('$hooks/useSyncState', () => ({ + useSyncState: (_mx: unknown, cb: (state: string, prev: string) => void) => { + syncStateCB.current = cb; + }, +})); + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeStore() { + return createStore(); +} + +function makeWrapper(store: ReturnType) { + return function Wrapper({ children }: { children: ReactNode }) { + return createElement(Provider, { store }, children); + }; +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +describe('useInitBookmarks', () => { + let store: ReturnType; + + beforeEach(() => { + store = makeStore(); + accountDataCB.current = null; + syncStateCB.current = null; + }); + + it('loads bookmarks on mount', () => { + renderHook(() => useInitBookmarks(), { wrapper: makeWrapper(store) }); + + const list = store.get(bookmarkListAtom); + const deleted = store.get(bookmarkDeletedListAtom); + expect(list).toHaveLength(1); + expect(list[0].bookmark_id).toBe('bmk_aabb'); + expect(deleted).toHaveLength(1); + expect(deleted[0].bookmark_id).toBe('bmk_ccdd'); + }); + + it('reloads when BookmarksIndex account data event fires', () => { + renderHook(() => useInitBookmarks(), { wrapper: makeWrapper(store) }); + + // Clear the atom to prove the callback re-populates it + store.set(bookmarkListAtom, []); + + accountDataCB.current!({ getType: () => BOOKMARKS_INDEX }); + + expect(store.get(bookmarkListAtom)).toHaveLength(1); + }); + + it('reloads when a bookmark item account data event fires', () => { + renderHook(() => useInitBookmarks(), { wrapper: makeWrapper(store) }); + + store.set(bookmarkListAtom, []); + + accountDataCB.current!({ + getType: () => `${BOOKMARK_PREFIX}bmk_aabb`, + }); + + expect(store.get(bookmarkListAtom)).toHaveLength(1); + }); + + it('ignores unrelated account data events', () => { + renderHook(() => useInitBookmarks(), { wrapper: makeWrapper(store) }); + + store.set(bookmarkListAtom, []); + + accountDataCB.current!({ getType: () => 'm.room.message' }); + + // Should still be empty — callback should not have triggered a reload + expect(store.get(bookmarkListAtom)).toHaveLength(0); + }); +}); diff --git a/src/app/features/bookmarks/useInitBookmarks.ts b/src/app/features/bookmarks/useInitBookmarks.ts index 3b6cb2247..480e20083 100644 --- a/src/app/features/bookmarks/useInitBookmarks.ts +++ b/src/app/features/bookmarks/useInitBookmarks.ts @@ -57,12 +57,18 @@ export function useInitBookmarks(): void { ) ); - // React to index updates pushed by other devices mid-session. + // React to bookmark account data changes pushed by other devices mid-session. + // The index event fires when the bookmark list changes; individual item events + // fire when a bookmark is added, removed, or soft-deleted. useAccountDataCallback( mx, useCallback( (event: MatrixEvent) => { - if (event.getType() === (AccountDataEvent.BookmarksIndex as string)) { + const type = event.getType(); + if ( + type === (AccountDataEvent.BookmarksIndex as string) || + type.startsWith(AccountDataEvent.BookmarkItemPrefix as string) + ) { loadBookmarks(); } }, From 2202bec58b871386f23e3cfbd54fd521b39dbb10 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Tue, 14 Apr 2026 22:53:32 -0400 Subject: [PATCH 11/17] fix(bookmarks): add Fragment key, guard missing eventId, fix removeBookmark doc comment - Wrap grouped bookmark list items in instead of un-keyed <> - Return null from MessageBookmarkItem when mEvent.getId() is undefined - Update removeBookmark doc comment to match item-first deletion ordering --- src/app/features/bookmarks/bookmarkRepository.ts | 11 +++++++---- src/app/features/room/message/Message.tsx | 5 +++-- src/app/pages/client/bookmarks/BookmarksList.tsx | 7 +++---- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/app/features/bookmarks/bookmarkRepository.ts b/src/app/features/bookmarks/bookmarkRepository.ts index bd9cda928..1b6cf0208 100644 --- a/src/app/features/bookmarks/bookmarkRepository.ts +++ b/src/app/features/bookmarks/bookmarkRepository.ts @@ -74,12 +74,15 @@ export async function addBookmark(mx: MatrixClient, item: BookmarkItemContent): * Remove a bookmark. * * MSC4438 §Removing a bookmark: - * 1. Remove the ID from the index. - * 2. Soft-delete the item (set deleted: true). + * 1. Soft-delete the item first (set deleted: true). + * 2. Remove the ID from the index. + * 3. Increment revision and update timestamp. + * 4. Write the updated index. * * Account data events cannot be deleted from the server, so soft-deletion is - * used. Other clients that encounter the item event can see it is explicitly - * removed. + * used. This implementation intentionally tombstones the item before updating + * the index to mirror addBookmark()'s item-first ordering and avoid transient + * orphan recovery/resurrection if a removal only partially completes. */ export async function removeBookmark(mx: MatrixClient, bookmarkId: string): Promise { // Tombstone the item event directly — bypass readItem()'s validation so that diff --git a/src/app/features/room/message/Message.tsx b/src/app/features/room/message/Message.tsx index 6652e1844..ec17635fe 100644 --- a/src/app/features/room/message/Message.tsx +++ b/src/app/features/room/message/Message.tsx @@ -224,10 +224,11 @@ export const MessageBookmarkItem = as< const mx = useMatrixClient(); const bookmarksExperiment = useExperimentVariant('messageBookmarks', mx.getUserId() ?? undefined); const [enableMessageBookmarks] = useSetting(settingsAtom, 'enableMessageBookmarks'); - const eventId = mEvent.getId() ?? ''; - const isBookmarked = useIsBookmarked(room.roomId, eventId); + const eventId = mEvent.getId(); + const isBookmarked = useIsBookmarked(room.roomId, eventId ?? ''); const { add, remove } = useBookmarkActions(); + if (!eventId) return null; if (!bookmarksExperiment.inExperiment && !enableMessageBookmarks) return null; const handleClick = async () => { diff --git a/src/app/pages/client/bookmarks/BookmarksList.tsx b/src/app/pages/client/bookmarks/BookmarksList.tsx index 562a4920c..a24b661c8 100644 --- a/src/app/pages/client/bookmarks/BookmarksList.tsx +++ b/src/app/pages/client/bookmarks/BookmarksList.tsx @@ -1,4 +1,4 @@ -import { FormEventHandler, useCallback, useMemo, useRef, useState } from 'react'; +import { FormEventHandler, Fragment, useCallback, useMemo, useRef, useState } from 'react'; import { Avatar, Box, @@ -561,10 +561,9 @@ export function BookmarksList() { {groupedByRoom.length > 0 && ( {groupedByRoom.map((group, i) => ( - <> + {i > 0 && } - + ))} )} From 7bb22d20e932e1a403adbf357642e396c609a4d6 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Wed, 15 Apr 2026 17:52:48 -0400 Subject: [PATCH 12/17] fix(nav): check DM membership before space parents in useRoomNavigate Mirrors the fix already applied in useNotificationJumper: when a room belongs to both the direct-message list and a space, prefer the /direct route over the space route. Previously useRoomNavigate checked orphan space parents first, which caused bookmark jumps and room-nav clicks on DMs-in-spaces to open the room via the space path instead of the direct path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/app/hooks/useRoomNavigate.ts | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/app/hooks/useRoomNavigate.ts b/src/app/hooks/useRoomNavigate.ts index 51555125c..c2918d5ca 100644 --- a/src/app/hooks/useRoomNavigate.ts +++ b/src/app/hooks/useRoomNavigate.ts @@ -37,7 +37,20 @@ export const useRoomNavigate = () => { const roomIdOrAlias = getCanonicalAliasOrRoomId(mx, roomId); const openSpaceTimeline = developerTools && spaceSelectedId === roomId; - const orphanParents = openSpaceTimeline ? [roomId] : getOrphanParents(roomToParents, roomId); + // Developer-mode: view the space's own timeline (must be checked first). + if (openSpaceTimeline) { + navigate(getSpaceRoomPath(roomIdOrAlias, roomId, eventId), opts); + return; + } + + // DMs take priority over space membership so direct chats always open + // via the direct route, even when the room also belongs to a space. + if (mDirects.has(roomId)) { + navigate(getDirectRoomPath(roomIdOrAlias, eventId), opts); + return; + } + + const orphanParents = getOrphanParents(roomToParents, roomId); if (orphanParents.length > 0) { let parentSpace: string; if (spaceSelectedId && orphanParents.includes(spaceSelectedId)) { @@ -48,15 +61,7 @@ export const useRoomNavigate = () => { const pSpaceIdOrAlias = getCanonicalAliasOrRoomId(mx, parentSpace); - navigate( - getSpaceRoomPath(pSpaceIdOrAlias, openSpaceTimeline ? roomId : roomIdOrAlias, eventId), - opts - ); - return; - } - - if (mDirects.has(roomId)) { - navigate(getDirectRoomPath(roomIdOrAlias, eventId), opts); + navigate(getSpaceRoomPath(pSpaceIdOrAlias, roomIdOrAlias, eventId), opts); return; } From ad50ff1b1d2d924b1c9531b935601d62ed8beb3f Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Thu, 16 Apr 2026 16:20:04 -0400 Subject: [PATCH 13/17] fix(timeline): restore useLayoutEffect auto-scroll, fix new-message scroll, fix eventId drag-to-bottom, increase list timeline limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - useTimelineSync: change auto-scroll recovery useEffect → useLayoutEffect to prevent one-frame flash after timeline reset - useTimelineSync: remove premature scrollToBottom from useLiveTimelineRefresh (operated on pre-commit DOM with stale scrollSize) - useTimelineSync: remove scrollToBottom + eventsLengthRef suppression from useLiveEventArrive; let useLayoutEffect handle scroll after React commits - RoomTimeline: init atBottomState to false when eventId is set, and reset it in the eventId useEffect, so auto-scroll doesn't drag to bottom on bookmark nav - RoomTimeline: change instant scrollToBottom to use scrollToIndex instead of scrollTo(scrollSize) — works correctly regardless of VList measurement state - slidingSync: increase DEFAULT_LIST_TIMELINE_LIMIT 1→3 to reduce empty previews when recent events are reactions/edits/state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/app/features/room/RoomTimeline.tsx | 23 ++++++++++++++--- src/app/hooks/timeline/useTimelineSync.ts | 31 +++++++++++++---------- src/client/slidingSync.ts | 6 ++--- 3 files changed, 41 insertions(+), 19 deletions(-) diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index e703d0206..99590f27a 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -198,7 +198,14 @@ export function RoomTimeline({ const setOpenThread = useSetAtom(openThreadAtom); const vListRef = useRef(null); - const [atBottomState, setAtBottomState] = useState(true); + // Load any cached scroll state for this room on mount. A fresh RoomTimeline is + // mounted per room (via key={roomId} in RoomView) so this is the only place we + // need to read the cache — the render-phase room-change block below only fires + // in the (hypothetical) case where the room prop changes without a remount. + const scrollCacheForRoomRef = useRef( + roomScrollCache.load(mxUserId, room.roomId) + ); + const [atBottomState, setAtBottomState] = useState(!eventId); const atBottomRef = useRef(atBottomState); const setAtBottom = useCallback((val: boolean) => { setAtBottomState(val); @@ -242,7 +249,14 @@ export function RoomTimeline({ if (!vListRef.current) return; const lastIndex = processedEventsRef.current.length - 1; if (lastIndex < 0) return; - vListRef.current.scrollTo(vListRef.current.scrollSize); + if (behavior === 'smooth') { + vListRef.current.scrollToIndex(lastIndex, { align: 'end', smooth: true }); + } else { + // scrollToIndex works reliably regardless of VList measurement state. + // The auto-scroll useLayoutEffect fires after React commits new items, + // so lastIndex is always valid when this is called. + vListRef.current.scrollToIndex(lastIndex, { align: 'end' }); + } }, []); const timelineSync = useTimelineSync({ @@ -394,8 +408,11 @@ export function RoomTimeline({ useEffect(() => { if (!eventId) return; setIsReady(false); + // Ensure auto-scroll to bottom doesn't fire while we're navigating to a + // specific event — atBottom will be updated correctly once the user scrolls. + setAtBottom(false); timelineSyncRef.current.loadEventTimeline(eventId); - }, [eventId, room.roomId]); + }, [eventId, room.roomId, setAtBottom]); useEffect(() => { if (eventId) return; diff --git a/src/app/hooks/timeline/useTimelineSync.ts b/src/app/hooks/timeline/useTimelineSync.ts index 51c85dda8..948630887 100644 --- a/src/app/hooks/timeline/useTimelineSync.ts +++ b/src/app/hooks/timeline/useTimelineSync.ts @@ -1,4 +1,13 @@ -import { useState, useMemo, useCallback, useRef, useEffect, Dispatch, SetStateAction } from 'react'; +import { + useState, + useMemo, + useCallback, + useRef, + useEffect, + useLayoutEffect, + Dispatch, + SetStateAction, +} from 'react'; import to from 'await-to-js'; import * as Sentry from '@sentry/react'; import { @@ -466,9 +475,6 @@ export function useTimelineSync({ const lastScrolledAtEventsLengthRef = useRef(eventsLength); - const eventsLengthRef = useRef(eventsLength); - eventsLengthRef.current = eventsLength; - useLiveEventArrive( room, useCallback( @@ -490,9 +496,6 @@ export function useTimelineSync({ setUnreadInfo(getRoomUnreadInfo(room)); } - scrollToBottom(getSender.call(mEvt) === mx.getUserId() ? 'instant' : 'smooth'); - lastScrolledAtEventsLengthRef.current = eventsLengthRef.current + 1; - setTimeline((ct) => ({ ...ct })); return; } @@ -502,7 +505,7 @@ export function useTimelineSync({ setUnreadInfo(getRoomUnreadInfo(room)); } }, - [mx, room, isAtBottomRef, unreadInfo, scrollToBottom, setUnreadInfo, hideReadsRef] + [mx, room, isAtBottomRef, unreadInfo, setUnreadInfo, hideReadsRef] ) ); @@ -527,10 +530,10 @@ export function useTimelineSync({ const wasAtBottom = isAtBottomRef.current; resetAutoScrollPendingRef.current = wasAtBottom; setTimeline({ linkedTimelines: getInitialTimeline(room).linkedTimelines }); - if (wasAtBottom) { - scrollToBottom('instant'); - } - }, [room, isAtBottomRef, scrollToBottom]) + // Scroll is handled by the useLayoutEffect auto-scroll recovery which + // fires after React commits the new timeline state — scrolling here + // would operate on the pre-commit DOM with a stale scrollSize. + }, [room, isAtBottomRef]) ); useRelationUpdate( @@ -547,7 +550,9 @@ export function useTimelineSync({ }, []) ); - useEffect(() => { + // useLayoutEffect so scroll fires before paint — prevents the one-frame flash + // where new VList content is briefly visible at the wrong position. + useLayoutEffect(() => { const resetAutoScrollPending = resetAutoScrollPendingRef.current; if (resetAutoScrollPending) resetAutoScrollPendingRef.current = false; diff --git a/src/client/slidingSync.ts b/src/client/slidingSync.ts index 43fdf39ea..5c147179c 100644 --- a/src/client/slidingSync.ts +++ b/src/client/slidingSync.ts @@ -34,9 +34,9 @@ export const LIST_SEARCH = 'search'; export const LIST_ROOM_SEARCH = 'room_search'; // Dynamic list key used for space-scoped room views. export const LIST_SPACE = 'space'; -// One event of timeline per list room is enough to compute unread counts; -// the full history is loaded when the user opens the room. -const LIST_TIMELINE_LIMIT = 1; +// Higher limit avoids empty previews when the most-recent events are +// reactions/edits/state that useRoomLatestRenderedEvent skips over. +const LIST_TIMELINE_LIMIT = 3; const DEFAULT_LIST_PAGE_SIZE = 250; const DEFAULT_POLL_TIMEOUT_MS = 20000; const DEFAULT_MAX_ROOMS = 5000; From bd31c97dcfe5b3fb7b864fddf21f11bf2021f418 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Thu, 16 Apr 2026 19:33:47 -0400 Subject: [PATCH 14/17] fix(timeline): restore upstream scroll pattern for new messages Restore scrollToBottom call in useLiveEventArrive with instant/smooth based on sender, add back eventsLengthRef and lastScrolledAt suppression, restore scrollToBottom in useLiveTimelineRefresh when wasAtBottom, and revert instant scrollToBottom to scrollTo(scrollSize) matching upstream. The previous changes removed all scroll calls from event arrival handlers and relied solely on the useLayoutEffect auto-scroll recovery, which has timing issues with VList measurement. Upstream's pattern of scrolling in the event handler and suppressing the effect works reliably. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/app/features/room/RoomTimeline.tsx | 5 +---- src/app/hooks/timeline/useTimelineSync.ts | 16 +++++++++++----- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index 99590f27a..d842f2a28 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -252,10 +252,7 @@ export function RoomTimeline({ if (behavior === 'smooth') { vListRef.current.scrollToIndex(lastIndex, { align: 'end', smooth: true }); } else { - // scrollToIndex works reliably regardless of VList measurement state. - // The auto-scroll useLayoutEffect fires after React commits new items, - // so lastIndex is always valid when this is called. - vListRef.current.scrollToIndex(lastIndex, { align: 'end' }); + vListRef.current.scrollTo(vListRef.current.scrollSize); } }, []); diff --git a/src/app/hooks/timeline/useTimelineSync.ts b/src/app/hooks/timeline/useTimelineSync.ts index 948630887..f6d50c904 100644 --- a/src/app/hooks/timeline/useTimelineSync.ts +++ b/src/app/hooks/timeline/useTimelineSync.ts @@ -475,6 +475,9 @@ export function useTimelineSync({ const lastScrolledAtEventsLengthRef = useRef(eventsLength); + const eventsLengthRef = useRef(eventsLength); + eventsLengthRef.current = eventsLength; + useLiveEventArrive( room, useCallback( @@ -496,6 +499,9 @@ export function useTimelineSync({ setUnreadInfo(getRoomUnreadInfo(room)); } + scrollToBottom(getSender.call(mEvt) === mx.getUserId() ? 'instant' : 'smooth'); + lastScrolledAtEventsLengthRef.current = eventsLengthRef.current + 1; + setTimeline((ct) => ({ ...ct })); return; } @@ -505,7 +511,7 @@ export function useTimelineSync({ setUnreadInfo(getRoomUnreadInfo(room)); } }, - [mx, room, isAtBottomRef, unreadInfo, setUnreadInfo, hideReadsRef] + [mx, room, isAtBottomRef, unreadInfo, scrollToBottom, setUnreadInfo, hideReadsRef] ) ); @@ -530,10 +536,10 @@ export function useTimelineSync({ const wasAtBottom = isAtBottomRef.current; resetAutoScrollPendingRef.current = wasAtBottom; setTimeline({ linkedTimelines: getInitialTimeline(room).linkedTimelines }); - // Scroll is handled by the useLayoutEffect auto-scroll recovery which - // fires after React commits the new timeline state — scrolling here - // would operate on the pre-commit DOM with a stale scrollSize. - }, [room, isAtBottomRef]) + if (wasAtBottom) { + scrollToBottom('instant'); + } + }, [room, isAtBottomRef, scrollToBottom]) ); useRelationUpdate( From 1392b779080acaa81487908fc19b8a5e84b13087 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Thu, 16 Apr 2026 22:26:19 -0400 Subject: [PATCH 15/17] fix(timeline): align scrollToBottom with upstream, fix eventId race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove behavior parameter from scrollToBottom — always use scrollTo(scrollSize) matching upstream. The smooth scrollToIndex was scrolling to stale lastIndex (before new item measured), leaving new messages below the fold. - Revert auto-scroll recovery from useLayoutEffect back to useEffect (matches upstream). useLayoutEffect fires before VList measures new items and before setAtBottom(false) in eventId effect. - Remove stale scrollCacheForRoomRef that referenced missing imports. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/app/features/room/RoomTimeline.tsx | 15 +---------- .../hooks/timeline/useTimelineSync.test.tsx | 2 +- src/app/hooks/timeline/useTimelineSync.ts | 25 ++++++------------- 3 files changed, 9 insertions(+), 33 deletions(-) diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index d842f2a28..73895a64a 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -198,13 +198,6 @@ export function RoomTimeline({ const setOpenThread = useSetAtom(openThreadAtom); const vListRef = useRef(null); - // Load any cached scroll state for this room on mount. A fresh RoomTimeline is - // mounted per room (via key={roomId} in RoomView) so this is the only place we - // need to read the cache — the render-phase room-change block below only fires - // in the (hypothetical) case where the room prop changes without a remount. - const scrollCacheForRoomRef = useRef( - roomScrollCache.load(mxUserId, room.roomId) - ); const [atBottomState, setAtBottomState] = useState(!eventId); const atBottomRef = useRef(atBottomState); const setAtBottom = useCallback((val: boolean) => { @@ -249,11 +242,7 @@ export function RoomTimeline({ if (!vListRef.current) return; const lastIndex = processedEventsRef.current.length - 1; if (lastIndex < 0) return; - if (behavior === 'smooth') { - vListRef.current.scrollToIndex(lastIndex, { align: 'end', smooth: true }); - } else { - vListRef.current.scrollTo(vListRef.current.scrollSize); - } + vListRef.current.scrollTo(vListRef.current.scrollSize); }, []); const timelineSync = useTimelineSync({ @@ -405,8 +394,6 @@ export function RoomTimeline({ useEffect(() => { if (!eventId) return; setIsReady(false); - // Ensure auto-scroll to bottom doesn't fire while we're navigating to a - // specific event — atBottom will be updated correctly once the user scrolls. setAtBottom(false); timelineSyncRef.current.loadEventTimeline(eventId); }, [eventId, room.roomId, setAtBottom]); diff --git a/src/app/hooks/timeline/useTimelineSync.test.tsx b/src/app/hooks/timeline/useTimelineSync.test.tsx index d53d74143..e5e7c4cfd 100644 --- a/src/app/hooks/timeline/useTimelineSync.test.tsx +++ b/src/app/hooks/timeline/useTimelineSync.test.tsx @@ -129,7 +129,7 @@ describe('useTimelineSync', () => { await Promise.resolve(); }); - expect(scrollToBottom).toHaveBeenCalledWith('instant'); + expect(scrollToBottom).toHaveBeenCalled(); }); it('resets timeline state when room.roomId changes and eventId is not set', async () => { diff --git a/src/app/hooks/timeline/useTimelineSync.ts b/src/app/hooks/timeline/useTimelineSync.ts index f6d50c904..dda1e0207 100644 --- a/src/app/hooks/timeline/useTimelineSync.ts +++ b/src/app/hooks/timeline/useTimelineSync.ts @@ -1,13 +1,4 @@ -import { - useState, - useMemo, - useCallback, - useRef, - useEffect, - useLayoutEffect, - Dispatch, - SetStateAction, -} from 'react'; +import { useState, useMemo, useCallback, useRef, useEffect, Dispatch, SetStateAction } from 'react'; import to from 'await-to-js'; import * as Sentry from '@sentry/react'; import { @@ -360,7 +351,7 @@ export interface UseTimelineSyncOptions { eventId?: string; isAtBottom: boolean; isAtBottomRef: React.MutableRefObject; - scrollToBottom: (behavior?: 'instant' | 'smooth') => void; + scrollToBottom: () => void; unreadInfo: ReturnType; setUnreadInfo: Dispatch>>; hideReadsRef: React.MutableRefObject; @@ -469,7 +460,7 @@ export function useTimelineSync({ useCallback(() => { if (!alive()) return; setTimeline({ linkedTimelines: getInitialTimeline(room).linkedTimelines }); - scrollToBottom('instant'); + scrollToBottom(); }, [alive, room, scrollToBottom]) ); @@ -499,7 +490,7 @@ export function useTimelineSync({ setUnreadInfo(getRoomUnreadInfo(room)); } - scrollToBottom(getSender.call(mEvt) === mx.getUserId() ? 'instant' : 'smooth'); + scrollToBottom(); lastScrolledAtEventsLengthRef.current = eventsLengthRef.current + 1; setTimeline((ct) => ({ ...ct })); @@ -537,7 +528,7 @@ export function useTimelineSync({ resetAutoScrollPendingRef.current = wasAtBottom; setTimeline({ linkedTimelines: getInitialTimeline(room).linkedTimelines }); if (wasAtBottom) { - scrollToBottom('instant'); + scrollToBottom(); } }, [room, isAtBottomRef, scrollToBottom]) ); @@ -556,9 +547,7 @@ export function useTimelineSync({ }, []) ); - // useLayoutEffect so scroll fires before paint — prevents the one-frame flash - // where new VList content is briefly visible at the wrong position. - useLayoutEffect(() => { + useEffect(() => { const resetAutoScrollPending = resetAutoScrollPendingRef.current; if (resetAutoScrollPending) resetAutoScrollPendingRef.current = false; @@ -576,7 +565,7 @@ export function useTimelineSync({ if (eventsLength <= lastScrolledAtEventsLengthRef.current && !resetAutoScrollPending) return; lastScrolledAtEventsLengthRef.current = eventsLength; - scrollToBottom('instant'); + scrollToBottom(); }, [isAtBottom, liveTimelineLinked, eventsLength, scrollToBottom]); useEffect(() => { From 3bea590f5151e4bc5eb04c2f4d4669d1b35e33f9 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Sat, 18 Apr 2026 20:49:00 -0400 Subject: [PATCH 16/17] fix(bookmarks): remove unrelated branch spillover --- config.json | 9 --- src/app/features/room/RoomTimeline.tsx | 5 +- src/app/features/room/message/Message.tsx | 4 +- .../hooks/timeline/useTimelineSync.test.tsx | 2 +- src/app/hooks/timeline/useTimelineSync.ts | 10 +-- src/app/hooks/useClientConfig.ts | 67 ------------------- src/app/hooks/useRoomNavigate.ts | 25 +++---- src/app/pages/client/ClientNonUIFeatures.tsx | 8 +-- src/app/pages/client/home/Home.tsx | 4 +- src/app/pages/client/inbox/Inbox.tsx | 6 +- src/client/slidingSync.ts | 6 +- 11 files changed, 26 insertions(+), 120 deletions(-) diff --git a/config.json b/config.json index 4f3dae570..f0c3c8b61 100644 --- a/config.json +++ b/config.json @@ -19,15 +19,6 @@ "enabled": true }, - "experiments": { - "messageBookmarks": { - "enabled": false, - "rolloutPercentage": 0, - "controlVariant": "control", - "variants": ["enabled"] - } - }, - "featuredCommunities": { "openAsDefault": false, "spaces": [ diff --git a/src/app/features/room/RoomTimeline.tsx b/src/app/features/room/RoomTimeline.tsx index 0630dcc9b..d026ec1fa 100644 --- a/src/app/features/room/RoomTimeline.tsx +++ b/src/app/features/room/RoomTimeline.tsx @@ -201,7 +201,7 @@ export function RoomTimeline({ const setOpenThread = useSetAtom(openThreadAtom); const vListRef = useRef(null); - const [atBottomState, setAtBottomState] = useState(!eventId); + const [atBottomState, setAtBottomState] = useState(true); const atBottomRef = useRef(atBottomState); const setAtBottom = useCallback((val: boolean) => { setAtBottomState(val); @@ -408,9 +408,8 @@ export function RoomTimeline({ useEffect(() => { if (!eventId) return; setIsReady(false); - setAtBottom(false); timelineSyncRef.current.loadEventTimeline(eventId); - }, [eventId, room.roomId, setAtBottom]); + }, [eventId, room.roomId]); useEffect(() => { if (eventId) return; diff --git a/src/app/features/room/message/Message.tsx b/src/app/features/room/message/Message.tsx index 0eb23c8b0..482a50768 100644 --- a/src/app/features/room/message/Message.tsx +++ b/src/app/features/room/message/Message.tsx @@ -81,7 +81,6 @@ import { MessageForwardItem } from '$components/message/modals/MessageForward'; import { MessageDeleteItem } from '$components/message/modals/MessageDelete'; import { computeBookmarkId, createBookmarkItem } from '$features/bookmarks/bookmarkDomain'; import { useIsBookmarked, useBookmarkActions } from '$features/bookmarks/useBookmarks'; -import { useExperimentVariant } from '$hooks/useClientConfig'; import { MessageReportItem } from '$components/message/modals/MessageReport'; import { filterPronounsByLanguage, getParsedPronouns } from '$utils/pronouns'; import { useMentionClickHandler } from '$hooks/useMentionClickHandler'; @@ -221,14 +220,13 @@ export const MessageBookmarkItem = as< } >(({ room, mEvent, onClose, ...props }, ref) => { const mx = useMatrixClient(); - const bookmarksExperiment = useExperimentVariant('messageBookmarks', mx.getUserId() ?? undefined); const [enableMessageBookmarks] = useSetting(settingsAtom, 'enableMessageBookmarks'); const eventId = mEvent.getId(); const isBookmarked = useIsBookmarked(room.roomId, eventId ?? ''); const { add, remove } = useBookmarkActions(); if (!eventId) return null; - if (!bookmarksExperiment.inExperiment && !enableMessageBookmarks) return null; + if (!enableMessageBookmarks) return null; const handleClick = async () => { if (isBookmarked) { diff --git a/src/app/hooks/timeline/useTimelineSync.test.tsx b/src/app/hooks/timeline/useTimelineSync.test.tsx index e5e7c4cfd..d53d74143 100644 --- a/src/app/hooks/timeline/useTimelineSync.test.tsx +++ b/src/app/hooks/timeline/useTimelineSync.test.tsx @@ -129,7 +129,7 @@ describe('useTimelineSync', () => { await Promise.resolve(); }); - expect(scrollToBottom).toHaveBeenCalled(); + expect(scrollToBottom).toHaveBeenCalledWith('instant'); }); it('resets timeline state when room.roomId changes and eventId is not set', async () => { diff --git a/src/app/hooks/timeline/useTimelineSync.ts b/src/app/hooks/timeline/useTimelineSync.ts index dda1e0207..51c85dda8 100644 --- a/src/app/hooks/timeline/useTimelineSync.ts +++ b/src/app/hooks/timeline/useTimelineSync.ts @@ -351,7 +351,7 @@ export interface UseTimelineSyncOptions { eventId?: string; isAtBottom: boolean; isAtBottomRef: React.MutableRefObject; - scrollToBottom: () => void; + scrollToBottom: (behavior?: 'instant' | 'smooth') => void; unreadInfo: ReturnType; setUnreadInfo: Dispatch>>; hideReadsRef: React.MutableRefObject; @@ -460,7 +460,7 @@ export function useTimelineSync({ useCallback(() => { if (!alive()) return; setTimeline({ linkedTimelines: getInitialTimeline(room).linkedTimelines }); - scrollToBottom(); + scrollToBottom('instant'); }, [alive, room, scrollToBottom]) ); @@ -490,7 +490,7 @@ export function useTimelineSync({ setUnreadInfo(getRoomUnreadInfo(room)); } - scrollToBottom(); + scrollToBottom(getSender.call(mEvt) === mx.getUserId() ? 'instant' : 'smooth'); lastScrolledAtEventsLengthRef.current = eventsLengthRef.current + 1; setTimeline((ct) => ({ ...ct })); @@ -528,7 +528,7 @@ export function useTimelineSync({ resetAutoScrollPendingRef.current = wasAtBottom; setTimeline({ linkedTimelines: getInitialTimeline(room).linkedTimelines }); if (wasAtBottom) { - scrollToBottom(); + scrollToBottom('instant'); } }, [room, isAtBottomRef, scrollToBottom]) ); @@ -565,7 +565,7 @@ export function useTimelineSync({ if (eventsLength <= lastScrolledAtEventsLengthRef.current && !resetAutoScrollPending) return; lastScrolledAtEventsLengthRef.current = eventsLength; - scrollToBottom(); + scrollToBottom('instant'); }, [isAtBottom, liveTimelineLinked, eventsLength, scrollToBottom]); useEffect(() => { diff --git a/src/app/hooks/useClientConfig.ts b/src/app/hooks/useClientConfig.ts index d3e85df09..e523f15a7 100644 --- a/src/app/hooks/useClientConfig.ts +++ b/src/app/hooks/useClientConfig.ts @@ -5,21 +5,6 @@ export type HashRouterConfig = { basename?: string; }; -export type ExperimentConfig = { - enabled?: boolean; - rolloutPercentage?: number; - variants?: string[]; - controlVariant?: string; -}; - -export type ExperimentSelection = { - key: string; - enabled: boolean; - rolloutPercentage: number; - variant: string; - inExperiment: boolean; -}; - export type ClientConfig = { defaultHomeserver?: number; homeserverList?: string[]; @@ -29,8 +14,6 @@ export type ClientConfig = { disableAccountSwitcher?: boolean; hideUsernamePasswordFields?: boolean; - experiments?: Record; - pushNotificationDetails?: { pushNotifyUrl?: string; vapidPublicKey?: string; @@ -72,56 +55,6 @@ export function useClientConfig(): ClientConfig { return config; } -const DEFAULT_CONTROL_VARIANT = 'control'; - -const normalizeRolloutPercentage = (value?: number): number => { - if (typeof value !== 'number' || Number.isNaN(value)) return 100; - if (value < 0) return 0; - if (value > 100) return 100; - return value; -}; - -const hashToUInt32 = (input: string): number => { - let hash = 0; - for (let index = 0; index < input.length; index += 1) { - hash = (hash * 131 + input.charCodeAt(index)) % 4294967291; - } - return hash; -}; - -export const selectExperimentVariant = ( - key: string, - experiment: ExperimentConfig | undefined, - subjectId: string | undefined -): ExperimentSelection => { - const controlVariant = experiment?.controlVariant ?? DEFAULT_CONTROL_VARIANT; - const variants = (experiment?.variants?.filter((v) => v.length > 0) ?? []).filter( - (v) => v !== controlVariant - ); - - const enabled = Boolean(experiment?.enabled); - const rolloutPercentage = normalizeRolloutPercentage(experiment?.rolloutPercentage); - - if (!enabled || !subjectId || variants.length === 0 || rolloutPercentage === 0) { - return { key, enabled, rolloutPercentage, variant: controlVariant, inExperiment: false }; - } - - // Two independent hashes keep rollout and variant assignment stable but decorrelated. - const rolloutBucket = hashToUInt32(`${key}:rollout:${subjectId}`) % 10000; - const rolloutCutoff = Math.floor(rolloutPercentage * 100); - if (rolloutBucket >= rolloutCutoff) { - return { key, enabled, rolloutPercentage, variant: controlVariant, inExperiment: false }; - } - - const variantIndex = hashToUInt32(`${key}:variant:${subjectId}`) % variants.length; - return { key, enabled, rolloutPercentage, variant: variants[variantIndex], inExperiment: true }; -}; - -export const useExperimentVariant = (key: string, subjectId?: string): ExperimentSelection => { - const clientConfig = useClientConfig(); - return selectExperimentVariant(key, clientConfig.experiments?.[key], subjectId); -}; - export const clientDefaultServer = (clientConfig: ClientConfig): string => clientConfig.homeserverList?.[clientConfig.defaultHomeserver ?? 0] ?? 'matrix.org'; diff --git a/src/app/hooks/useRoomNavigate.ts b/src/app/hooks/useRoomNavigate.ts index c2918d5ca..51555125c 100644 --- a/src/app/hooks/useRoomNavigate.ts +++ b/src/app/hooks/useRoomNavigate.ts @@ -37,20 +37,7 @@ export const useRoomNavigate = () => { const roomIdOrAlias = getCanonicalAliasOrRoomId(mx, roomId); const openSpaceTimeline = developerTools && spaceSelectedId === roomId; - // Developer-mode: view the space's own timeline (must be checked first). - if (openSpaceTimeline) { - navigate(getSpaceRoomPath(roomIdOrAlias, roomId, eventId), opts); - return; - } - - // DMs take priority over space membership so direct chats always open - // via the direct route, even when the room also belongs to a space. - if (mDirects.has(roomId)) { - navigate(getDirectRoomPath(roomIdOrAlias, eventId), opts); - return; - } - - const orphanParents = getOrphanParents(roomToParents, roomId); + const orphanParents = openSpaceTimeline ? [roomId] : getOrphanParents(roomToParents, roomId); if (orphanParents.length > 0) { let parentSpace: string; if (spaceSelectedId && orphanParents.includes(spaceSelectedId)) { @@ -61,7 +48,15 @@ export const useRoomNavigate = () => { const pSpaceIdOrAlias = getCanonicalAliasOrRoomId(mx, parentSpace); - navigate(getSpaceRoomPath(pSpaceIdOrAlias, roomIdOrAlias, eventId), opts); + navigate( + getSpaceRoomPath(pSpaceIdOrAlias, openSpaceTimeline ? roomId : roomIdOrAlias, eventId), + opts + ); + return; + } + + if (mDirects.has(roomId)) { + navigate(getDirectRoomPath(roomIdOrAlias, eventId), opts); return; } diff --git a/src/app/pages/client/ClientNonUIFeatures.tsx b/src/app/pages/client/ClientNonUIFeatures.tsx index ca0b82b81..caebe459a 100644 --- a/src/app/pages/client/ClientNonUIFeatures.tsx +++ b/src/app/pages/client/ClientNonUIFeatures.tsx @@ -514,12 +514,8 @@ function MessageNotifications() { }); } - // In-app audio: play when the app is in the foreground (has focus) and - // notification sounds are enabled for this notification type. - // Gating on hasFocus() rather than just visibilityState prevents a race - // where the page is still 'visible' for a brief window after the user - // backgrounds the app on mobile — hasFocus() flips false first. - if (notificationSound && isLoud && document.hasFocus()) { + // In-app audio: play when notification sounds are enabled AND this notification is loud. + if (notificationSound && isLoud) { playSound(); } }; diff --git a/src/app/pages/client/home/Home.tsx b/src/app/pages/client/home/Home.tsx index 175b48f9d..e7b832df5 100644 --- a/src/app/pages/client/home/Home.tsx +++ b/src/app/pages/client/home/Home.tsx @@ -59,7 +59,6 @@ import { useClosedNavCategoriesAtom } from '$state/hooks/closedNavCategories'; import { stopPropagation } from '$utils/keyboard'; import { useSetting } from '$state/hooks/settings'; import { settingsAtom } from '$state/settings'; -import { useExperimentVariant } from '$hooks/useClientConfig'; import { getRoomNotificationMode, useRoomsNotificationPreferencesContext, @@ -210,9 +209,8 @@ export function Home() { const createRoomSelected = useHomeCreateSelected(); const searchSelected = useHomeSearchSelected(); const bookmarksSelected = useHomeBookmarksSelected(); - const bookmarksExperiment = useExperimentVariant('messageBookmarks', mx.getUserId() ?? undefined); const [enableMessageBookmarks] = useSetting(settingsAtom, 'enableMessageBookmarks'); - const showBookmarks = bookmarksExperiment.inExperiment || enableMessageBookmarks; + const showBookmarks = enableMessageBookmarks; const noRoomToDisplay = rooms.length === 0; const [closedCategories, setClosedCategories] = useAtom(useClosedNavCategoriesAtom()); diff --git a/src/app/pages/client/inbox/Inbox.tsx b/src/app/pages/client/inbox/Inbox.tsx index d594a928e..ab42f7dcb 100644 --- a/src/app/pages/client/inbox/Inbox.tsx +++ b/src/app/pages/client/inbox/Inbox.tsx @@ -15,10 +15,8 @@ import { UnreadBadge } from '$components/unread-badge'; import { allInvitesAtom } from '$state/room-list/inviteList'; import { useNavToActivePathMapper } from '$hooks/useNavToActivePathMapper'; import { PageNav, PageNavContent, PageNavHeader } from '$components/page'; -import { useMatrixClient } from '$hooks/useMatrixClient'; import { useSetting } from '$state/hooks/settings'; import { settingsAtom } from '$state/settings'; -import { useExperimentVariant } from '$hooks/useClientConfig'; function InvitesNavItem() { const invitesSelected = useInboxInvitesSelected(); @@ -76,11 +74,9 @@ function BookmarksNavItem() { export function Inbox() { useNavToActivePathMapper('inbox'); - const mx = useMatrixClient(); const notificationsSelected = useInboxNotificationsSelected(); - const bookmarksExperiment = useExperimentVariant('messageBookmarks', mx.getUserId() ?? undefined); const [enableMessageBookmarks] = useSetting(settingsAtom, 'enableMessageBookmarks'); - const showBookmarks = bookmarksExperiment.inExperiment || enableMessageBookmarks; + const showBookmarks = enableMessageBookmarks; return ( diff --git a/src/client/slidingSync.ts b/src/client/slidingSync.ts index 5c147179c..43fdf39ea 100644 --- a/src/client/slidingSync.ts +++ b/src/client/slidingSync.ts @@ -34,9 +34,9 @@ export const LIST_SEARCH = 'search'; export const LIST_ROOM_SEARCH = 'room_search'; // Dynamic list key used for space-scoped room views. export const LIST_SPACE = 'space'; -// Higher limit avoids empty previews when the most-recent events are -// reactions/edits/state that useRoomLatestRenderedEvent skips over. -const LIST_TIMELINE_LIMIT = 3; +// One event of timeline per list room is enough to compute unread counts; +// the full history is loaded when the user opens the room. +const LIST_TIMELINE_LIMIT = 1; const DEFAULT_LIST_PAGE_SIZE = 250; const DEFAULT_POLL_TIMEOUT_MS = 20000; const DEFAULT_MAX_ROOMS = 5000; From fd9e0079525be17d2f6a0e62f3366df8afc61a59 Mon Sep 17 00:00:00 2001 From: Evie Gauthier Date: Sat, 18 Apr 2026 20:50:42 -0400 Subject: [PATCH 17/17] chore(bookmarks): fix branch validation issues --- src/app/features/bookmarks/useInitBookmarks.test.tsx | 11 ++++------- src/app/features/room/message/Message.tsx | 1 - 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/app/features/bookmarks/useInitBookmarks.test.tsx b/src/app/features/bookmarks/useInitBookmarks.test.tsx index 893b5d64c..afb24f953 100644 --- a/src/app/features/bookmarks/useInitBookmarks.test.tsx +++ b/src/app/features/bookmarks/useInitBookmarks.test.tsx @@ -44,9 +44,9 @@ const { accountDataCB, syncStateCB, mockMx } = vi.hoisted(() => { }; const store: Record = { - ['org.matrix.msc4438.bookmarks.index']: index, - ['org.matrix.msc4438.bookmark.bmk_aabb']: item, - ['org.matrix.msc4438.bookmark.bmk_ccdd']: deletedItem, + 'org.matrix.msc4438.bookmarks.index': index, + 'org.matrix.msc4438.bookmark.bmk_aabb': item, + 'org.matrix.msc4438.bookmark.bmk_ccdd': deletedItem, }; const mx = { @@ -67,10 +67,7 @@ vi.mock('$hooks/useMatrixClient', () => ({ })); vi.mock('$hooks/useAccountDataCallback', () => ({ - useAccountDataCallback: ( - _mx: unknown, - cb: (event: { getType: () => string }) => void - ) => { + useAccountDataCallback: (_mx: unknown, cb: (event: { getType: () => string }) => void) => { accountDataCB.current = cb; }, })); diff --git a/src/app/features/room/message/Message.tsx b/src/app/features/room/message/Message.tsx index 482a50768..04fc23563 100644 --- a/src/app/features/room/message/Message.tsx +++ b/src/app/features/room/message/Message.tsx @@ -219,7 +219,6 @@ export const MessageBookmarkItem = as< onClose?: () => void; } >(({ room, mEvent, onClose, ...props }, ref) => { - const mx = useMatrixClient(); const [enableMessageBookmarks] = useSetting(settingsAtom, 'enableMessageBookmarks'); const eventId = mEvent.getId(); const isBookmarked = useIsBookmarked(room.roomId, eventId ?? '');