fix(audit): correctness wave — ghost sends, Escape coordination, panel exclusion
- ScheduledMessagesTray: cancel prunes local state ONLY on confirmed server cancel; failures keep the item + show an inline error (was: a failed cancel looked cancelled but still sent at the scheduled time). - Escape semantics: the composer consumes Escape (preventDefault+stopPropagation) iff autocomplete is open or a reply draft is set; the thread panel and Room's markAsRead act only on unconsumed Escape, and markAsRead defers entirely while a thread panel is open (listener order made it fire before the panel closed). - Room: thread panel / media gallery are mutually exclusive (most-recently- opened wins); on mobile at most one right panel renders (thread > gallery > members) instead of stacked fullscreen overlays. - RemindMeDialog: busy-disabled presets (no more double-click duplicates), try/catch with inline error, close only on success. - ThreadTimeline: "Jump to Latest" floating chip when scrolled up (RoomTimeline idiom). From the 4-auditor deep-audit wave; reviewer-verified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -1,4 +1,4 @@
|
|||||||
import React, { useCallback } from 'react';
|
import React, { useCallback, useEffect, useRef } from 'react';
|
||||||
import { Box, Line } from 'folds';
|
import { Box, Line } from 'folds';
|
||||||
import { useParams } from 'react-router-dom';
|
import { useParams } from 'react-router-dom';
|
||||||
import { isKeyHotkey } from 'is-hotkey';
|
import { isKeyHotkey } from 'is-hotkey';
|
||||||
@@ -49,15 +49,46 @@ export function Room() {
|
|||||||
useCallback(
|
useCallback(
|
||||||
(evt) => {
|
(evt) => {
|
||||||
if (isKeyHotkey('escape', evt)) {
|
if (isKeyHotkey('escape', evt)) {
|
||||||
|
// Skip when a composer already consumed Escape (it preventDefaults).
|
||||||
|
if (evt.defaultPrevented) return;
|
||||||
|
// Skip while a thread panel is open: listener registration order
|
||||||
|
// means this can run BEFORE the panel's own Escape handler, and the
|
||||||
|
// user's intent there is "close the panel", not "mark room read".
|
||||||
|
if (activeThreadId) return;
|
||||||
markAsRead(mx, room.roomId, hideActivity);
|
markAsRead(mx, room.roomId, hideActivity);
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
[mx, room.roomId, hideActivity],
|
[mx, room.roomId, hideActivity, activeThreadId],
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
|
|
||||||
const callView = callEmbed?.roomId === room.roomId || room.isCallRoom() || callMembers.length > 0;
|
const callView = callEmbed?.roomId === room.roomId || room.isCallRoom() || callMembers.length > 0;
|
||||||
|
|
||||||
|
// Thread panel and media gallery are mutually exclusive on every screen size:
|
||||||
|
// opening one closes the other. Detect the just-opened transition so whichever
|
||||||
|
// was opened most recently wins.
|
||||||
|
const prevThreadRef = useRef(activeThreadId);
|
||||||
|
const prevGalleryRef = useRef(galleryOpen);
|
||||||
|
useEffect(() => {
|
||||||
|
const threadJustOpened = Boolean(activeThreadId) && !prevThreadRef.current;
|
||||||
|
const galleryJustOpened = galleryOpen && !prevGalleryRef.current;
|
||||||
|
if (threadJustOpened && galleryOpen) {
|
||||||
|
setGalleryOpen(false);
|
||||||
|
} else if (galleryJustOpened && activeThreadId) {
|
||||||
|
setActiveThreadId(null);
|
||||||
|
}
|
||||||
|
prevThreadRef.current = activeThreadId;
|
||||||
|
prevGalleryRef.current = galleryOpen;
|
||||||
|
}, [activeThreadId, galleryOpen, setGalleryOpen, setActiveThreadId]);
|
||||||
|
|
||||||
|
// On non-desktop screens at most one right-side panel may show, priority
|
||||||
|
// thread > gallery > members. On desktop thread + members may coexist while
|
||||||
|
// thread + gallery stay mutually exclusive (via the effect above).
|
||||||
|
const isDesktop = screenSize === ScreenSize.Desktop;
|
||||||
|
const showThreadPanel = !callView && Boolean(activeThreadId);
|
||||||
|
const showGallery = !callView && galleryOpen && (isDesktop || !activeThreadId);
|
||||||
|
const showMembers = !callView && isDrawer && (isDesktop || (!activeThreadId && !galleryOpen));
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<PowerLevelsContextProvider value={powerLevels}>
|
<PowerLevelsContextProvider value={powerLevels}>
|
||||||
<Box grow="Yes">
|
<Box grow="Yes">
|
||||||
@@ -86,7 +117,7 @@ export function Room() {
|
|||||||
<CallChatView />
|
<CallChatView />
|
||||||
</>
|
</>
|
||||||
)}
|
)}
|
||||||
{!callView && galleryOpen && (
|
{showGallery && (
|
||||||
<>
|
<>
|
||||||
{screenSize === ScreenSize.Desktop && (
|
{screenSize === ScreenSize.Desktop && (
|
||||||
<Line variant="Background" direction="Vertical" size="300" />
|
<Line variant="Background" direction="Vertical" size="300" />
|
||||||
@@ -94,7 +125,7 @@ export function Room() {
|
|||||||
<MediaGallery key={room.roomId} room={room} onClose={() => setGalleryOpen(false)} />
|
<MediaGallery key={room.roomId} room={room} onClose={() => setGalleryOpen(false)} />
|
||||||
</>
|
</>
|
||||||
)}
|
)}
|
||||||
{!callView && activeThreadId && (
|
{showThreadPanel && activeThreadId && (
|
||||||
<>
|
<>
|
||||||
{screenSize === ScreenSize.Desktop && (
|
{screenSize === ScreenSize.Desktop && (
|
||||||
<Line variant="Background" direction="Vertical" size="300" />
|
<Line variant="Background" direction="Vertical" size="300" />
|
||||||
@@ -107,7 +138,7 @@ export function Room() {
|
|||||||
/>
|
/>
|
||||||
</>
|
</>
|
||||||
)}
|
)}
|
||||||
{!callView && isDrawer && (
|
{showMembers && (
|
||||||
<>
|
<>
|
||||||
{screenSize === ScreenSize.Desktop && (
|
{screenSize === ScreenSize.Desktop && (
|
||||||
<Line variant="Background" direction="Vertical" size="300" />
|
<Line variant="Background" direction="Vertical" size="300" />
|
||||||
|
|||||||
@@ -679,15 +679,23 @@ export const RoomInput = forwardRef<HTMLDivElement, RoomInputProps>(
|
|||||||
submit();
|
submit();
|
||||||
}
|
}
|
||||||
if (isKeyHotkey('escape', evt)) {
|
if (isKeyHotkey('escape', evt)) {
|
||||||
evt.preventDefault();
|
// Only consume Escape (and stop it bubbling to the thread panel / room
|
||||||
|
// window handlers) when the composer actually has something to dismiss.
|
||||||
|
// If we did nothing, let Escape propagate so those handlers can run.
|
||||||
if (autocompleteQuery) {
|
if (autocompleteQuery) {
|
||||||
|
evt.preventDefault();
|
||||||
|
evt.stopPropagation();
|
||||||
setAutocompleteQuery(undefined);
|
setAutocompleteQuery(undefined);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
if (replyDraft) {
|
||||||
|
evt.preventDefault();
|
||||||
|
evt.stopPropagation();
|
||||||
setReplyDraft(undefined);
|
setReplyDraft(undefined);
|
||||||
}
|
}
|
||||||
|
}
|
||||||
},
|
},
|
||||||
[submit, setReplyDraft, enterForNewline, autocompleteQuery, isComposing],
|
[submit, replyDraft, setReplyDraft, enterForNewline, autocompleteQuery, isComposing],
|
||||||
);
|
);
|
||||||
|
|
||||||
const handleKeyUp: KeyboardEventHandler = useCallback(
|
const handleKeyUp: KeyboardEventHandler = useCallback(
|
||||||
|
|||||||
@@ -33,6 +33,7 @@ export function ScheduledMessagesTray({ roomId }: ScheduledMessagesTrayProps) {
|
|||||||
const [scheduledMessages, setScheduledMessages] = useAtom(scheduledMessagesAtom);
|
const [scheduledMessages, setScheduledMessages] = useAtom(scheduledMessagesAtom);
|
||||||
const [expanded, setExpanded] = useState(false);
|
const [expanded, setExpanded] = useState(false);
|
||||||
const [cancelling, setCancelling] = useState<Set<string>>(new Set());
|
const [cancelling, setCancelling] = useState<Set<string>>(new Set());
|
||||||
|
const [cancelErrors, setCancelErrors] = useState<Set<string>>(new Set());
|
||||||
|
|
||||||
const messages = useMemo(() => scheduledMessages.get(roomId) ?? [], [scheduledMessages, roomId]);
|
const messages = useMemo(() => scheduledMessages.get(roomId) ?? [], [scheduledMessages, roomId]);
|
||||||
|
|
||||||
@@ -68,12 +69,17 @@ export function ScheduledMessagesTray({ roomId }: ScheduledMessagesTrayProps) {
|
|||||||
async (msg: ScheduledMessage) => {
|
async (msg: ScheduledMessage) => {
|
||||||
if (cancelling.has(msg.delayId)) return;
|
if (cancelling.has(msg.delayId)) return;
|
||||||
setCancelling((prev) => new Set(prev).add(msg.delayId));
|
setCancelling((prev) => new Set(prev).add(msg.delayId));
|
||||||
|
setCancelErrors((prev) => {
|
||||||
|
if (!prev.has(msg.delayId)) return prev;
|
||||||
|
const next = new Set(prev);
|
||||||
|
next.delete(msg.delayId);
|
||||||
|
return next;
|
||||||
|
});
|
||||||
try {
|
try {
|
||||||
await cancelScheduledMessage(mx, msg.delayId);
|
await cancelScheduledMessage(mx, msg.delayId);
|
||||||
} catch {
|
// Only prune local state once the server confirms cancellation. If we
|
||||||
// If cancellation fails on the server, still remove locally
|
// removed it optimistically the still-live delayed event would fire and
|
||||||
// since the user intends to remove it
|
// the "cancelled" message would send anyway.
|
||||||
} finally {
|
|
||||||
setScheduledMessages((prev) => {
|
setScheduledMessages((prev) => {
|
||||||
const next = new Map(prev);
|
const next = new Map(prev);
|
||||||
const current = next.get(roomId) ?? [];
|
const current = next.get(roomId) ?? [];
|
||||||
@@ -85,6 +91,11 @@ export function ScheduledMessagesTray({ roomId }: ScheduledMessagesTrayProps) {
|
|||||||
}
|
}
|
||||||
return next;
|
return next;
|
||||||
});
|
});
|
||||||
|
} catch {
|
||||||
|
// Keep the item (still cancellable) and surface an inline error; the
|
||||||
|
// delayed event is still scheduled on the server.
|
||||||
|
setCancelErrors((prev) => new Set(prev).add(msg.delayId));
|
||||||
|
} finally {
|
||||||
setCancelling((prev) => {
|
setCancelling((prev) => {
|
||||||
const next = new Set(prev);
|
const next = new Set(prev);
|
||||||
next.delete(msg.delayId);
|
next.delete(msg.delayId);
|
||||||
@@ -131,13 +142,13 @@ export function ScheduledMessagesTray({ roomId }: ScheduledMessagesTrayProps) {
|
|||||||
{messages.map((msg) => (
|
{messages.map((msg) => (
|
||||||
<Box
|
<Box
|
||||||
key={msg.delayId}
|
key={msg.delayId}
|
||||||
alignItems="Center"
|
direction="Column"
|
||||||
gap="200"
|
|
||||||
style={{
|
style={{
|
||||||
padding: `${config.space.S100} ${config.space.S300}`,
|
padding: `${config.space.S100} ${config.space.S300}`,
|
||||||
borderTop: `${config.borderWidth.B300} solid ${color.SurfaceVariant.ContainerLine}`,
|
borderTop: `${config.borderWidth.B300} solid ${color.SurfaceVariant.ContainerLine}`,
|
||||||
}}
|
}}
|
||||||
>
|
>
|
||||||
|
<Box alignItems="Center" gap="200">
|
||||||
<Text
|
<Text
|
||||||
size="T200"
|
size="T200"
|
||||||
priority="400"
|
priority="400"
|
||||||
@@ -148,7 +159,9 @@ export function ScheduledMessagesTray({ roomId }: ScheduledMessagesTrayProps) {
|
|||||||
whiteSpace: 'nowrap',
|
whiteSpace: 'nowrap',
|
||||||
}}
|
}}
|
||||||
>
|
>
|
||||||
{typeof msg.content.body === 'string' ? (msg.content.body as string) : '(message)'}
|
{typeof msg.content.body === 'string'
|
||||||
|
? (msg.content.body as string)
|
||||||
|
: '(message)'}
|
||||||
</Text>
|
</Text>
|
||||||
<Text size="T200" priority="300" style={{ whiteSpace: 'nowrap', flexShrink: 0 }}>
|
<Text size="T200" priority="300" style={{ whiteSpace: 'nowrap', flexShrink: 0 }}>
|
||||||
{formatSendAt(msg.sendAt)}
|
{formatSendAt(msg.sendAt)}
|
||||||
@@ -167,6 +180,15 @@ export function ScheduledMessagesTray({ roomId }: ScheduledMessagesTrayProps) {
|
|||||||
<Icon src={Icons.Cross} size="50" />
|
<Icon src={Icons.Cross} size="50" />
|
||||||
</IconButton>
|
</IconButton>
|
||||||
</Box>
|
</Box>
|
||||||
|
{cancelErrors.has(msg.delayId) && (
|
||||||
|
<Text
|
||||||
|
size="T200"
|
||||||
|
style={{ color: color.Critical.Main, paddingTop: config.space.S100 }}
|
||||||
|
>
|
||||||
|
Could not cancel this message. Try again.
|
||||||
|
</Text>
|
||||||
|
)}
|
||||||
|
</Box>
|
||||||
))}
|
))}
|
||||||
</Box>
|
</Box>
|
||||||
)}
|
)}
|
||||||
|
|||||||
@@ -1,8 +1,9 @@
|
|||||||
import React, { useMemo } from 'react';
|
import React, { useMemo, useState } from 'react';
|
||||||
import FocusTrap from 'focus-trap-react';
|
import FocusTrap from 'focus-trap-react';
|
||||||
import {
|
import {
|
||||||
Box,
|
Box,
|
||||||
Button,
|
Button,
|
||||||
|
color,
|
||||||
config,
|
config,
|
||||||
Dialog,
|
Dialog,
|
||||||
Header,
|
Header,
|
||||||
@@ -43,8 +44,14 @@ export function RemindMeDialog({ roomId, eventId, previewText, onClose }: Remind
|
|||||||
const modalStyle = useModalStyle(320);
|
const modalStyle = useModalStyle(320);
|
||||||
const { addReminder } = useReminders();
|
const { addReminder } = useReminders();
|
||||||
const presets = useMemo(() => getPresets(), []);
|
const presets = useMemo(() => getPresets(), []);
|
||||||
|
const [busy, setBusy] = useState(false);
|
||||||
|
const [error, setError] = useState<string | null>(null);
|
||||||
|
|
||||||
const handlePick = async (ms: number) => {
|
const handlePick = async (ms: number) => {
|
||||||
|
if (busy) return;
|
||||||
|
setBusy(true);
|
||||||
|
setError(null);
|
||||||
|
try {
|
||||||
await addReminder({
|
await addReminder({
|
||||||
roomId,
|
roomId,
|
||||||
eventId,
|
eventId,
|
||||||
@@ -52,6 +59,10 @@ export function RemindMeDialog({ roomId, eventId, previewText, onClose }: Remind
|
|||||||
message: previewText || 'Reminder',
|
message: previewText || 'Reminder',
|
||||||
});
|
});
|
||||||
onClose();
|
onClose();
|
||||||
|
} catch {
|
||||||
|
setBusy(false);
|
||||||
|
setError('Could not set reminder. Try again.');
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
return (
|
return (
|
||||||
@@ -108,6 +119,7 @@ export function RemindMeDialog({ roomId, eventId, previewText, onClose }: Remind
|
|||||||
variant="Secondary"
|
variant="Secondary"
|
||||||
fill="Soft"
|
fill="Soft"
|
||||||
radii="300"
|
radii="300"
|
||||||
|
disabled={busy}
|
||||||
onClick={() => handlePick(p.ms)}
|
onClick={() => handlePick(p.ms)}
|
||||||
>
|
>
|
||||||
<Text size="B300" truncate>
|
<Text size="B300" truncate>
|
||||||
@@ -115,6 +127,14 @@ export function RemindMeDialog({ roomId, eventId, previewText, onClose }: Remind
|
|||||||
</Text>
|
</Text>
|
||||||
</Button>
|
</Button>
|
||||||
))}
|
))}
|
||||||
|
{error && (
|
||||||
|
<Text
|
||||||
|
size="T200"
|
||||||
|
style={{ color: color.Critical.Main, paddingTop: config.space.S100 }}
|
||||||
|
>
|
||||||
|
{error}
|
||||||
|
</Text>
|
||||||
|
)}
|
||||||
</Box>
|
</Box>
|
||||||
</Dialog>
|
</Dialog>
|
||||||
</FocusTrap>
|
</FocusTrap>
|
||||||
|
|||||||
@@ -123,6 +123,10 @@ export function ThreadPanel({ room, threadId, requestClose }: ThreadPanelProps)
|
|||||||
useCallback(
|
useCallback(
|
||||||
(evt) => {
|
(evt) => {
|
||||||
if (isKeyHotkey('escape', evt)) {
|
if (isKeyHotkey('escape', evt)) {
|
||||||
|
// The composer preventDefaults Escape when it consumes it (dismissing
|
||||||
|
// autocomplete / clearing a reply draft). Don't close the panel in
|
||||||
|
// that case — only when Escape wasn't already handled.
|
||||||
|
if (evt.defaultPrevented) return;
|
||||||
evt.preventDefault();
|
evt.preventDefault();
|
||||||
evt.stopPropagation();
|
evt.stopPropagation();
|
||||||
requestClose();
|
requestClose();
|
||||||
|
|||||||
@@ -11,6 +11,15 @@ export const ThreadTimelineContent = style({
|
|||||||
padding: `${config.space.S400} 0`,
|
padding: `${config.space.S400} 0`,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
export const ThreadTimelineFloat = style({
|
||||||
|
position: 'absolute',
|
||||||
|
bottom: config.space.S400,
|
||||||
|
left: '50%',
|
||||||
|
transform: 'translateX(-50%)',
|
||||||
|
zIndex: 1,
|
||||||
|
minWidth: 'max-content',
|
||||||
|
});
|
||||||
|
|
||||||
export const ThreadCentered = style({
|
export const ThreadCentered = style({
|
||||||
height: '100%',
|
height: '100%',
|
||||||
padding: config.space.S700,
|
padding: config.space.S700,
|
||||||
|
|||||||
@@ -29,7 +29,7 @@ import { Editor } from 'slate';
|
|||||||
import { ReactEditor } from 'slate-react';
|
import { ReactEditor } from 'slate-react';
|
||||||
import to from 'await-to-js';
|
import to from 'await-to-js';
|
||||||
import { useAtomValue, useSetAtom } from 'jotai';
|
import { useAtomValue, useSetAtom } from 'jotai';
|
||||||
import { Badge, Box, Line, Scroll, Spinner, Text, color, config } from 'folds';
|
import { Badge, Box, Chip, Icon, Icons, Line, Scroll, Spinner, Text, color, config } from 'folds';
|
||||||
import classNames from 'classnames';
|
import classNames from 'classnames';
|
||||||
import { Opts as LinkifyOpts } from 'linkifyjs';
|
import { Opts as LinkifyOpts } from 'linkifyjs';
|
||||||
import { eventWithShortcode, factoryEventSentBy, getMxIdLocalPart } from '../../../utils/matrix';
|
import { eventWithShortcode, factoryEventSentBy, getMxIdLocalPart } from '../../../utils/matrix';
|
||||||
@@ -459,6 +459,14 @@ export function ThreadTimeline({ room, thread, editor }: ThreadTimelineProps) {
|
|||||||
}
|
}
|
||||||
}, [scrollToBottomCount]);
|
}, [scrollToBottomCount]);
|
||||||
|
|
||||||
|
const handleJumpToBottom = useCallback(() => {
|
||||||
|
scrollToBottomRef.current.count += 1;
|
||||||
|
scrollToBottomRef.current.smooth = true;
|
||||||
|
// Flip atBottom so the layout effect re-runs (count re-read) and live
|
||||||
|
// events resume sticking to the bottom.
|
||||||
|
setAtBottom(true);
|
||||||
|
}, []);
|
||||||
|
|
||||||
// Scroll in-place editor into view.
|
// Scroll in-place editor into view.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (editId) {
|
if (editId) {
|
||||||
@@ -949,6 +957,19 @@ export function ThreadTimeline({ room, thread, editor }: ThreadTimelineProps) {
|
|||||||
<span ref={atBottomAnchorRef} />
|
<span ref={atBottomAnchorRef} />
|
||||||
</Box>
|
</Box>
|
||||||
</Scroll>
|
</Scroll>
|
||||||
|
{!atBottom && (
|
||||||
|
<Box className={css.ThreadTimelineFloat} justifyContent="Center" alignItems="Center">
|
||||||
|
<Chip
|
||||||
|
variant="SurfaceVariant"
|
||||||
|
radii="Pill"
|
||||||
|
outlined
|
||||||
|
before={<Icon size="50" src={Icons.ArrowBottom} />}
|
||||||
|
onClick={handleJumpToBottom}
|
||||||
|
>
|
||||||
|
<Text size="L400">Jump to Latest</Text>
|
||||||
|
</Chip>
|
||||||
|
</Box>
|
||||||
|
)}
|
||||||
{editHistoryEvent && (
|
{editHistoryEvent && (
|
||||||
<EditHistoryModal
|
<EditHistoryModal
|
||||||
room={room}
|
room={room}
|
||||||
|
|||||||
Reference in New Issue
Block a user