From 57f21e5cac2d630098157279ac353e1595815f89 Mon Sep 17 00:00:00 2001
From: Jared Vititoe <jjvititoe1@gmail.com>
Date: Fri, 10 Jul 2026 00:43:32 -0400
Subject: [PATCH] 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>
---
 .../room/message/ForwardMessageDialog.tsx     | 23 +++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/src/app/features/room/message/ForwardMessageDialog.tsx b/src/app/features/room/message/ForwardMessageDialog.tsx
index 01f8eddc0..171d89e68 100644
--- a/src/app/features/room/message/ForwardMessageDialog.tsx
+++ b/src/app/features/room/message/ForwardMessageDialog.tsx
@@ -144,6 +144,8 @@ function ForwardPreview({
       shrink="No"
       gap="200"
       alignItems="Center"
+      role="group"
+      aria-label="Message to forward"
       style={{
         margin: `${config.space.S200} ${config.space.S400} 0`,
         padding: config.space.S200,
@@ -224,7 +226,7 @@ function RecentChip({
             src={avatarUrl}
             alt={room.name}
             renderFallback={() => (
-              <RoomIcon roomType={room.getType()} size="50" joinRule={room.getJoinRule()} filled />
+              <RoomIcon roomType={room.getType()} size="100" joinRule={room.getJoinRule()} filled />
             )}
           />
         </Avatar>
@@ -254,6 +256,11 @@ export function ForwardMessageDialog({ mEvent, onClose }: Props) {
   const [sentTo, setSentTo] = useState<string | null>(null);
   const [error, setError] = useState<string | null>(null);
   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
   // of the rendered slice stays selected.
   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);
         // Send the optional comment first so it reads as a note above the
         // 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
               // eslint-disable-next-line @typescript-eslint/no-explicit-any
               .sendMessage(id, null, { msgtype: MsgType.Text, body: commentBody } as any)
-              .then(sendForward)
-          : sendForward();
+              .then(() => {
+                commentSentRef.current.add(id);
+              })
+          : Promise.resolve();
+        return step.then(sendForward);
       }),
     );
 
@@ -408,6 +421,7 @@ export function ForwardMessageDialog({ mEvent, onClose }: Props) {
                   size="400"
                   radii="400"
                   outlined
+                  aria-label="Search rooms"
                   placeholder="Search rooms…"
                   value={query}
                   onChange={(e: ChangeEvent<HTMLInputElement>) => setQuery(e.target.value)}
@@ -417,6 +431,7 @@ export function ForwardMessageDialog({ mEvent, onClose }: Props) {
                   size="400"
                   radii="400"
                   outlined
+                  aria-label="Add a comment"
                   placeholder="Add a comment (optional)…"
                   value={comment}
                   disabled={sending}