Chore/docs retry and modal race fixes (#1289)

Closes #1043 
Closes #1210 
Closes #1239 
Closes #1247

Author: nonso7

Dechat/dex_with_fiat_frontend/src/components/ChatMessages.tsx

@@ -22,6 +22,11 @@ interface ChatMessagesProps {
     actionType: string,
     data?: Record<string, unknown>,
   ) => void;
+  /**
+   * Resend a message that failed to send. Receives the failed message's id and
+   * its original content, ready to be submitted again as-is.
+   */
+  onRetry?: (messageId: string, content: string) => void | Promise<void>;
   isLoading?: boolean;
   searchQuery?: string;
 }
@@ -107,6 +112,7 @@ function HelpCard({
 export default function ChatMessages({
   messages: allMessages,
   onActionClick,
+  onRetry,
   isLoading = false,
   searchQuery = '',
 }: ChatMessagesProps) {
@@ -402,6 +408,7 @@ export default function ChatMessages({
               key={message.id}
               message={message}
               onActionClick={onActionClick}
+              onRetry={onRetry}
               shouldAnimate={isReadyToAnimate && !isLoadingMore && !seenMessageIds.current.has(message.id)}
             />
           ))}

Dechat/dex_with_fiat_frontend/src/components/Message.tsx

@@ -12,6 +12,7 @@ import React from 'react';
 import ReactMarkdown from 'react-markdown';
 import type { Components } from 'react-markdown';
 import { toDate } from '@/lib/messageUtils';
+import { MAX_AUTO_RETRIES, useMessageRetry } from '@/hooks/useMessageRetry';
 import { useTranslation } from '@/contexts/TranslationContext';
 import { motion, useReducedMotion } from 'framer-motion';
 import CopyButton from '@/components/ui/CopyButton';
@@ -23,7 +24,12 @@ interface MessageProps {
     actionType: string,
     data?: Record<string, unknown>,
   ) => void;
-  onRetry?: (messageId: string) => void;
+  /**
+   * Resend a message that failed to send. Receives the message id and the
+   * *original* content the user typed (from `originalPayload` when available),
+   * so the caller never has to reconstruct it and the user never has to retype.
+   */
+  onRetry?: (messageId: string, content: string) => void | Promise<void>;
   shouldAnimate?: boolean;
 }
 
@@ -44,6 +50,19 @@ export default function Message({ message, onActionClick, onRetry, shouldAnimate
   const isPending = message.metadata?.status === 'pending';
   const isFailed = message.metadata?.status === 'failed';
 
+  // Resend uses the payload captured at send time so the user's original text
+  // survives any post-failure rewrite of `content`.
+  const retryContent = message.originalPayload?.content ?? message.content;
+  const retry = useMessageRetry({
+    messageId: message.id,
+    content: retryContent,
+    isFailed: hasError,
+    onRetry,
+  });
+  // Attempts already recorded on the message, plus the ones this component has
+  // made since it mounted.
+  const totalRetryAttempts = (message.error?.retryAttempts ?? 0) + retry.attempts;
+
 
   // Currency conversion hook for transaction amounts
   const amountForConversion = message.metadata?.transactionData?.amountIn 
@@ -216,6 +235,8 @@ export default function Message({ message, onActionClick, onRetry, shouldAnimate
             {/* Error State */}
             {hasError && (
               <div
+                data-testid="message-error"
+                role="alert"
                 className={`mt-3 inline-flex flex-col gap-2 rounded-lg border px-3 py-2 text-xs ${
                   isDarkMode
                     ? 'border-red-700 bg-red-950/40 text-red-200'
@@ -228,23 +249,51 @@ export default function Message({ message, onActionClick, onRetry, shouldAnimate
                     {message.error?.message || 'Failed to send message'}
                   </span>
                 </div>
-                {message.error?.retryAttempts && message.error.retryAttempts > 0 && (
+                {totalRetryAttempts > 0 && (
                   <div className="text-xs opacity-75">
-                    Retry attempts: {message.error.retryAttempts}
+                    Retry attempts: {totalRetryAttempts}
                   </div>
                 )}
                 {onRetry && (
-                  <button
-                    onClick={() => onRetry(message.id)}
-                    className={`mt-2 flex items-center justify-center gap-2 px-3 py-1 rounded-lg text-xs font-medium transition-all transform hover:scale-105 active:scale-95 ${
-                      isDarkMode
-                        ? 'bg-red-700/40 hover:bg-red-700/60 border border-red-600'
-                        : 'bg-red-100 hover:bg-red-200 border border-red-300'
-                    }`}
-                  >
-                    <RotateCcw className="w-3 h-3" />
-                    Retry
-                  </button>
+                  <>
+                    <div
+                      className="text-xs opacity-75"
+                      aria-live="polite"
+                      data-testid="message-retry-status"
+                    >
+                      {retry.isRetrying
+                        ? t('chat.resending')
+                        : retry.secondsUntilNextRetry !== null
+                          ? t('chat.retry_countdown', {
+                              seconds: retry.secondsUntilNextRetry,
+                              attempt: retry.attempts + 1,
+                              max: MAX_AUTO_RETRIES,
+                            })
+                          : t('chat.retry_exhausted', {
+                              max: MAX_AUTO_RETRIES,
+                            })}
+                    </div>
+                    <button
+                      type="button"
+                      onClick={retry.retryNow}
+                      disabled={retry.isRetrying}
+                      data-testid="message-retry-button"
+                      aria-label={t('chat.retry_message', {
+                        content: retryContent,
+                      })}
+                      title={retryContent}
+                      className={`mt-2 flex items-center justify-center gap-2 px-3 py-1 rounded-lg text-xs font-medium transition-all transform hover:scale-105 active:scale-95 disabled:opacity-60 disabled:cursor-not-allowed disabled:hover:scale-100 ${
+                        isDarkMode
+                          ? 'bg-red-700/40 hover:bg-red-700/60 border border-red-600'
+                          : 'bg-red-100 hover:bg-red-200 border border-red-300'
+                      }`}
+                    >
+                      <RotateCcw
+                        className={`w-3 h-3 ${retry.isRetrying ? 'animate-spin' : ''}`}
+                      />
+                      {retry.isRetrying ? t('common.loading') : t('common.retry')}
+                    </button>
+                  </>
                 )}
               </div>
             )}

Dechat/dex_with_fiat_frontend/src/components/OfflineStatusBanner.test.tsx

@@ -1,8 +1,8 @@
 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
-import { renderHook, act } from '@testing-library/react';
-import { render, screen, waitFor } from '@testing-library/react';
+import { act, render, screen } from '@testing-library/react';
 import OfflineStatusBanner from './OfflineStatusBanner';
-import * as offlineMessageQueue from '@/lib/offlineMessageQueue';
+import { useOnlineStatus } from '@/hooks/useOnlineStatus';
+import { subscribeToQueuedMessageCount } from '@/lib/offlineMessageQueue';
 
 // Mock dependencies
 vi.mock('@/hooks/useOnlineStatus', () => ({
@@ -26,14 +26,42 @@ vi.mock('@/lib/offlineStatusSchema', () => ({
 }));
 
 vi.mock('@/lib/offlineMessageQueue', () => ({
-  subscribeToQueuedMessageCount: vi.fn(),
+  subscribeToQueuedMessageCount: vi.fn(() => () => {}),
   setQueuedMessageCount: vi.fn(),
   getQueuedMessageCount: vi.fn(() => 0),
 }));
 
+const mockedUseOnlineStatus = vi.mocked(useOnlineStatus);
+const mockedSubscribe = vi.mocked(subscribeToQueuedMessageCount);
+
+/** Elapse the 300ms initial loading gate so it cannot mask later renders. */
+function settleLoadingGate() {
+  act(() => {
+    vi.advanceTimersByTime(300);
+  });
+}
+
+/** Point `useOnlineStatus` at a value the test can flip between renders. */
+function stubOnlineStatus(getIsOnline: () => boolean, wasOffline = false) {
+  const resetWasOffline = vi.fn();
+  mockedUseOnlineStatus.mockImplementation(() => ({
+    get isOnline() {
+      return getIsOnline();
+    },
+    wasOffline,
+    resetWasOffline,
+  }));
+  return resetWasOffline;
+}
+
 describe('OfflineStatusBanner - Optimistic UI Updates', () => {
   beforeEach(() => {
     vi.clearAllMocks();
+    mockedSubscribe.mockImplementation(() => () => {});
+    // Fake timers are required for the 300ms loading skeleton and the 500ms
+    // reconnect dismissal. Assertions below are made directly after an explicit
+    // `advanceTimersByTime` rather than through `waitFor`, whose polling never
+    // fires while the clock is frozen.
     vi.useFakeTimers();
   });
 
@@ -42,138 +70,104 @@ describe('OfflineStatusBanner - Optimistic UI Updates', () => {
     vi.useRealTimers();
   });
 
-  it('should show banner immediately when going offline', async () => {
-    const { useOnlineStatus } = await import('@/hooks/useOnlineStatus');
-    (useOnlineStatus as any).mockReturnValue({
-      isOnline: false,
-      wasOffline: false,
-      resetWasOffline: vi.fn(),
-    });
+  it('should show banner immediately when going offline', () => {
+    stubOnlineStatus(() => false);
 
     render(<OfflineStatusBanner />);
 
-    await waitFor(() => {
-      expect(screen.getByRole('status')).toBeInTheDocument();
-    });
-
+    expect(screen.getByRole('status')).toBeInTheDocument();
     expect(screen.getByText(/You are offline/i)).toBeInTheDocument();
   });
 
-  it('should show reconnecting state when coming back online', async () => {
-    const { useOnlineStatus } = await import('@/hooks/useOnlineStatus');
+  it('should show reconnecting state when coming back online', () => {
     let isOnline = false;
-    
-    (useOnlineStatus as any).mockImplementation(() => ({
-      get isOnline() { return isOnline; },
-      wasOffline: true,
-      resetWasOffline: vi.fn(),
-    }));
+    stubOnlineStatus(() => isOnline, true);
 
     const { rerender } = render(<OfflineStatusBanner />);
-
-    await waitFor(() => {
-      expect(screen.getByText(/You are offline/i)).toBeInTheDocument();
-    });
+    settleLoadingGate();
+    expect(screen.getByText(/You are offline/i)).toBeInTheDocument();
 
     // Simulate coming back online
     isOnline = true;
     rerender(<OfflineStatusBanner />);
 
-    await waitFor(() => {
-      expect(screen.getByText(/Reconnecting/i)).toBeInTheDocument();
-    });
+    expect(screen.getByText(/Reconnecting/i)).toBeInTheDocument();
   });
 
-  it('should display optimistic pending count', async () => {
-    const { useOnlineStatus } = await import('@/hooks/useOnlineStatus');
-    (useOnlineStatus as any).mockReturnValue({
-      isOnline: false,
-      wasOffline: false,
-      resetWasOffline: vi.fn(),
+  it('should display optimistic pending count', () => {
+    stubOnlineStatus(() => false);
+    // The count is pushed by `offlineMessageQueue` subscribers, not pulled.
+    mockedSubscribe.mockImplementation((listener) => {
+      listener(3);
+      return () => {};
     });
 
-    (offlineMessageQueue.getQueuedMessageCount as any).mockReturnValue(3);
-
     render(<OfflineStatusBanner />);
 
-    await waitFor(() => {
-      expect(screen.getByText(/3 messages waiting to send/i)).toBeInTheDocument();
+    expect(screen.getByText(/3 messages waiting to send/i)).toBeInTheDocument();
+  });
+
+  it('should pluralise a single pending message', () => {
+    stubOnlineStatus(() => false);
+    mockedSubscribe.mockImplementation((listener) => {
+      listener(1);
+      return () => {};
     });
+
+    render(<OfflineStatusBanner />);
+
+    expect(screen.getByText(/1 message waiting to send/i)).toBeInTheDocument();
   });
 
-  it('should hide banner after reconnection delay', async () => {
-    const { useOnlineStatus } = await import('@/hooks/useOnlineStatus');
+  it('should hide banner after reconnection delay', () => {
     let isOnline = false;
-    
-    (useOnlineStatus as any).mockImplementation(() => ({
-      get isOnline() { return isOnline; },
-      wasOffline: true,
-      resetWasOffline: vi.fn(),
-    }));
+    const resetWasOffline = stubOnlineStatus(() => isOnline, true);
 
     const { rerender } = render(<OfflineStatusBanner />);
-
-    await waitFor(() => {
-      expect(screen.getByRole('status')).toBeInTheDocument();
-    });
+    settleLoadingGate();
+    expect(screen.getByRole('status')).toBeInTheDocument();
 
     // Simulate coming back online
     isOnline = true;
     rerender(<OfflineStatusBanner />);
+    expect(screen.getByRole('status')).toBeInTheDocument();
 
     act(() => {
       vi.advanceTimersByTime(500);
     });
 
-    await waitFor(() => {
-      expect(screen.queryByRole('status')).not.toBeInTheDocument();
-    });
+    expect(screen.queryByRole('status')).not.toBeInTheDocument();
+    expect(resetWasOffline).toHaveBeenCalled();
   });
 
-  it('should update aria-label based on connection state', async () => {
-    const { useOnlineStatus } = await import('@/hooks/useOnlineStatus');
+  it('should update aria-label based on connection state', () => {
     let isOnline = false;
-    
-    (useOnlineStatus as any).mockImplementation(() => ({
-      get isOnline() { return isOnline; },
-      wasOffline: true,
-      resetWasOffline: vi.fn(),
-    }));
+    stubOnlineStatus(() => isOnline, true);
 
     const { rerender } = render(<OfflineStatusBanner />);
-
-    await waitFor(() => {
-      expect(screen.getByLabelText('Offline status')).toBeInTheDocument();
-    });
+    settleLoadingGate();
+    expect(screen.getByLabelText('Offline status')).toBeInTheDocument();
 
     isOnline = true;
     rerender(<OfflineStatusBanner />);
 
-    await waitFor(() => {
-      expect(screen.getByLabelText('Reconnecting')).toBeInTheDocument();
-    });
+    expect(screen.getByLabelText('Reconnecting')).toBeInTheDocument();
   });
 
-  it('should show loading skeleton initially when online', async () => {
-    const { useOnlineStatus } = await import('@/hooks/useOnlineStatus');
-    (useOnlineStatus as any).mockReturnValue({
-      isOnline: true,
-      wasOffline: false,
-      resetWasOffline: vi.fn(),
-    });
+  it('should show loading skeleton initially when online', () => {
+    stubOnlineStatus(() => true);
 
     render(<OfflineStatusBanner />);
 
     // Should show loading skeleton initially
-    const skeleton = document.querySelector('[aria-hidden="true"]');
-    expect(skeleton).toBeInTheDocument();
+    expect(document.querySelector('[aria-hidden="true"]')).toBeInTheDocument();
 
     act(() => {
       vi.advanceTimersByTime(300);
     });
 
-    await waitFor(() => {
-      expect(document.querySelector('[aria-hidden="true"]')).not.toBeInTheDocument();
-    });
+    expect(
+      document.querySelector('[aria-hidden="true"]'),
+    ).not.toBeInTheDocument();
   });
 });