fix(forward): harden comment retry + a11y after review
Address findings from 2 review agents on the forward upgrades:
- Duplicate comment on retry (correctness): if a room's comment message
sent but the forward then failed, retrying re-posted the comment. Track
rooms whose comment already delivered (commentSentRef) and skip it on
retry, sending only the missing forward. An already-commented room won't
get the comment again even if the text is later edited (no-duplicate
choice).
- a11y: give the message-preview box role="group" + aria-label
("Message to forward"), add aria-label to the comment and search inputs
(placeholder is not a label), and match the RecentChip's RoomIcon
fallback size (100) to the room-row convention for a size-200 avatar.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -144,6 +144,8 @@ function ForwardPreview({
|
|||||||
shrink="No"
|
shrink="No"
|
||||||
gap="200"
|
gap="200"
|
||||||
alignItems="Center"
|
alignItems="Center"
|
||||||
|
role="group"
|
||||||
|
aria-label="Message to forward"
|
||||||
style={{
|
style={{
|
||||||
margin: `${config.space.S200} ${config.space.S400} 0`,
|
margin: `${config.space.S200} ${config.space.S400} 0`,
|
||||||
padding: config.space.S200,
|
padding: config.space.S200,
|
||||||
@@ -224,7 +226,7 @@ function RecentChip({
|
|||||||
src={avatarUrl}
|
src={avatarUrl}
|
||||||
alt={room.name}
|
alt={room.name}
|
||||||
renderFallback={() => (
|
renderFallback={() => (
|
||||||
<RoomIcon roomType={room.getType()} size="50" joinRule={room.getJoinRule()} filled />
|
<RoomIcon roomType={room.getType()} size="100" joinRule={room.getJoinRule()} filled />
|
||||||
)}
|
)}
|
||||||
/>
|
/>
|
||||||
</Avatar>
|
</Avatar>
|
||||||
@@ -254,6 +256,11 @@ export function ForwardMessageDialog({ mEvent, onClose }: Props) {
|
|||||||
const [sentTo, setSentTo] = useState<string | null>(null);
|
const [sentTo, setSentTo] = useState<string | null>(null);
|
||||||
const [error, setError] = useState<string | null>(null);
|
const [error, setError] = useState<string | null>(null);
|
||||||
const [recents, setRecents] = useAtom(recentForwardTargetsAtom);
|
const [recents, setRecents] = useAtom(recentForwardTargetsAtom);
|
||||||
|
// Rooms whose comment message already delivered this session — so a retry after
|
||||||
|
// a forward failure doesn't re-post the comment (only the missing forward). Kept
|
||||||
|
// for the dialog's lifetime: a room that already got the comment won't get it
|
||||||
|
// again even if the text is later edited, which is the safe (no-duplicate) choice.
|
||||||
|
const commentSentRef = useRef<Set<string>>(new Set());
|
||||||
// Selection persists across query changes: a room selected then filtered out
|
// Selection persists across query changes: a room selected then filtered out
|
||||||
// of the rendered slice stays selected.
|
// of the rendered slice stays selected.
|
||||||
const [selectedRoomIds, setSelectedRoomIds] = useState<Set<string>>(new Set());
|
const [selectedRoomIds, setSelectedRoomIds] = useState<Set<string>>(new Set());
|
||||||
@@ -315,12 +322,18 @@ export function ForwardMessageDialog({ mEvent, onClose }: Props) {
|
|||||||
const sendForward = () => mx.sendEvent(id, null, mEvent.getType() as any, fwdContent);
|
const sendForward = () => mx.sendEvent(id, null, mEvent.getType() as any, fwdContent);
|
||||||
// Send the optional comment first so it reads as a note above the
|
// Send the optional comment first so it reads as a note above the
|
||||||
// forwarded content. The room counts as failed if either send rejects.
|
// forwarded content. The room counts as failed if either send rejects.
|
||||||
return commentBody
|
// Track rooms whose comment already landed so a retry (after the FORWARD
|
||||||
|
// failed) doesn't post the comment twice — only the missing forward.
|
||||||
|
const needsComment = commentBody && !commentSentRef.current.has(id);
|
||||||
|
const step = needsComment
|
||||||
? mx
|
? mx
|
||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
.sendMessage(id, null, { msgtype: MsgType.Text, body: commentBody } as any)
|
.sendMessage(id, null, { msgtype: MsgType.Text, body: commentBody } as any)
|
||||||
.then(sendForward)
|
.then(() => {
|
||||||
: sendForward();
|
commentSentRef.current.add(id);
|
||||||
|
})
|
||||||
|
: Promise.resolve();
|
||||||
|
return step.then(sendForward);
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -408,6 +421,7 @@ export function ForwardMessageDialog({ mEvent, onClose }: Props) {
|
|||||||
size="400"
|
size="400"
|
||||||
radii="400"
|
radii="400"
|
||||||
outlined
|
outlined
|
||||||
|
aria-label="Search rooms"
|
||||||
placeholder="Search rooms…"
|
placeholder="Search rooms…"
|
||||||
value={query}
|
value={query}
|
||||||
onChange={(e: ChangeEvent<HTMLInputElement>) => setQuery(e.target.value)}
|
onChange={(e: ChangeEvent<HTMLInputElement>) => setQuery(e.target.value)}
|
||||||
@@ -417,6 +431,7 @@ export function ForwardMessageDialog({ mEvent, onClose }: Props) {
|
|||||||
size="400"
|
size="400"
|
||||||
radii="400"
|
radii="400"
|
||||||
outlined
|
outlined
|
||||||
|
aria-label="Add a comment"
|
||||||
placeholder="Add a comment (optional)…"
|
placeholder="Add a comment (optional)…"
|
||||||
value={comment}
|
value={comment}
|
||||||
disabled={sending}
|
disabled={sending}
|
||||||
|
|||||||
Reference in New Issue
Block a user