feat(accounts): loading overlay for first-time webview opens (#867) (#887)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>

app/src/components/accounts/WebviewHost.tsx

@@ -6,7 +6,8 @@ import {
   openWebviewAccount,
   setWebviewAccountBounds,
 } from '../../services/webviewAccountService';
-import type { AccountProvider } from '../../types/accounts';
+import { useAppSelector } from '../../store/hooks';
+import type { AccountProvider, AccountStatus } from '../../types/accounts';
 
 const log = debug('webview-accounts:host');
 
@@ -15,12 +16,19 @@ interface WebviewHostProps {
   provider: AccountProvider;
 }
 
+const LOADING_STATUSES: ReadonlySet<AccountStatus> = new Set(['pending', 'loading']);
+
 /**
  * Reserves a rectangular slot in the React layout that the native child
  * webview is glued to. We measure the placeholder's bounding rect and
  * tell Rust to position the webview at the same spot. On unmount or
  * route change the webview is hidden (not destroyed) so its session
  * stays warm in the background.
+ *
+ * During the first-open cycle the CEF subview is parked off-screen by Rust so
+ * the React loading overlay below isn't covered by an empty native view. The
+ * overlay is dismissed when the `webview-account:load` event flips the account
+ * status out of `pending`/`loading`.
  */
 const WebviewHost = ({ accountId, provider }: WebviewHostProps) => {
   const ref = useRef<HTMLDivElement | null>(null);
@@ -28,6 +36,14 @@ const WebviewHost = ({ accountId, provider }: WebviewHostProps) => {
     null
   );
   const openedRef = useRef(false);
+  const status = useAppSelector(s => s.accounts.accounts[accountId]?.status);
+  // Only render the spinner when the account is *actively* loading. We used
+  // to also treat `status === undefined` as loading, but that meant a host
+  // mounted for an account that's not in the store (e.g. a render race with
+  // `addAccount`) would spin forever. The brief microtask between mount and
+  // the `setAccountStatus('pending')` dispatch in `openWebviewAccount` is
+  // visually indistinguishable from no overlay, so this is safe.
+  const isLoading = status !== undefined && LOADING_STATUSES.has(status);
 
   // Spawn / show + keep bounds synced on every layout change.
   // IMPORTANT: both refs are reset on cleanup so switching accountIds
@@ -105,8 +121,19 @@ const WebviewHost = ({ accountId, provider }: WebviewHostProps) => {
     <div
       ref={ref}
       className="relative h-full w-full overflow-hidden rounded-lg border border-stone-200 bg-stone-100"
-      aria-label={`webview host for account ${accountId}`}
-    />
+      aria-label={`webview host for account ${accountId}`}>
+      {isLoading ? (
+        <div
+          data-testid={`webview-loading-${accountId}`}
+          className="pointer-events-none absolute inset-0 flex flex-col items-center justify-center gap-3 text-stone-500"
+          role="status"
+          aria-live="polite"
+          aria-label="Loading account">
+          <div className="h-8 w-8 animate-spin rounded-full border-2 border-stone-300 border-t-stone-600" />
+          <span className="text-xs font-medium tracking-wide">Loading…</span>
+        </div>
+      ) : null}
+    </div>
   );
 };
 

app/src/services/__tests__/webviewAccountService.loadListener.test.ts

@@ -0,0 +1,173 @@
+import { invoke } from '@tauri-apps/api/core';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+import { store } from '../../store';
+import { addAccount, resetAccountsState } from '../../store/accountsSlice';
+import {
+  closeWebviewAccount,
+  openWebviewAccount,
+  setWebviewAccountBounds,
+  startWebviewAccountService,
+  stopWebviewAccountService,
+} from '../webviewAccountService';
+
+// Capture the handlers attached via `listen(...)` so tests can fire synthetic
+// events and verify downstream behaviour without actually wiring Tauri IPC.
+type EventHandler = (evt: { payload: unknown }) => void;
+const listeners = new Map<string, EventHandler>();
+
+vi.mock('@tauri-apps/api/core', () => ({
+  invoke: vi.fn().mockResolvedValue(undefined),
+  isTauri: vi.fn().mockReturnValue(true),
+}));
+
+vi.mock('@tauri-apps/api/event', () => ({
+  listen: vi.fn(async (event: string, handler: EventHandler) => {
+    listeners.set(event, handler);
+    return () => {
+      listeners.delete(event);
+    };
+  }),
+}));
+
+// The service pulls in heavy deps for unrelated flows (Meet transcript + core
+// RPC). Stub them so the listener test doesn't drag the whole dependency graph
+// through its setup.
+vi.mock('../api/threadApi', () => ({ threadApi: { createNewThread: vi.fn() } }));
+vi.mock('../chatService', () => ({ chatSend: vi.fn() }));
+vi.mock('../coreRpcClient', () => ({ callCoreRpc: vi.fn() }));
+vi.mock('../notificationService', () => ({ ingestNotification: vi.fn() }));
+
+const ACCOUNT_ID = 'acct-123';
+
+function seedAccount(): void {
+  store.dispatch(resetAccountsState());
+  store.dispatch(
+    addAccount({
+      id: ACCOUNT_ID,
+      provider: 'telegram',
+      label: 'Test',
+      createdAt: new Date().toISOString(),
+      status: 'closed',
+    })
+  );
+}
+
+async function fireLoadEvent(payload: { state: string; url?: string }): Promise<void> {
+  const handler = listeners.get('webview-account:load');
+  if (!handler) throw new Error('webview-account:load listener not attached');
+  handler({ payload: { account_id: ACCOUNT_ID, url: '', ...payload } });
+  // Drain to a macrotask so chained `.catch()` / `.then()` on the
+  // `invoke()` promise inside the handler also settle before we assert.
+  await new Promise(r => setTimeout(r, 0));
+}
+
+describe('webviewAccountService load listener', () => {
+  beforeEach(async () => {
+    listeners.clear();
+    stopWebviewAccountService();
+    // Tear down any per-account state left from the previous test (bounds
+    // cache + loading flag) before re-arming the listener for this one.
+    // `stopWebviewAccountService` already clears the module-level Maps;
+    // `closeWebviewAccount` is the no-Tauri-side close path (the invoke is
+    // mocked) and is here only as belt-and-braces.
+    await closeWebviewAccount(ACCOUNT_ID);
+    // Single mock reset so individual tests can rely on the `invoke`
+    // resolved-value config they set up after this hook returns.
+    vi.clearAllMocks();
+    seedAccount();
+    startWebviewAccountService();
+  });
+
+  it('transitions pending → loading on openWebviewAccount resolve', async () => {
+    const bounds = { x: 10, y: 20, width: 800, height: 600 };
+    await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds });
+
+    expect(store.getState().accounts.accounts[ACCOUNT_ID]?.status).toBe('loading');
+    expect(vi.mocked(invoke)).toHaveBeenCalledWith(
+      'webview_account_open',
+      expect.objectContaining({
+        args: expect.objectContaining({ account_id: ACCOUNT_ID, provider: 'telegram' }),
+      })
+    );
+  });
+
+  it('reveals with cached bounds + flips to open on finished event', async () => {
+    const bounds = { x: 5, y: 15, width: 1024, height: 768 };
+    await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds });
+    vi.mocked(invoke).mockClear();
+
+    await fireLoadEvent({ state: 'finished', url: 'https://web.telegram.org/' });
+
+    expect(vi.mocked(invoke)).toHaveBeenCalledWith('webview_account_reveal', {
+      args: { account_id: ACCOUNT_ID, bounds },
+    });
+    expect(store.getState().accounts.accounts[ACCOUNT_ID]?.status).toBe('open');
+  });
+
+  it('reveals with latest bounds when resize landed during loading', async () => {
+    const initial = { x: 0, y: 0, width: 800, height: 600 };
+    await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds: initial });
+
+    // Resize during loading — invoke should be skipped, cache should still update.
+    vi.mocked(invoke).mockClear();
+    const resized = { x: 0, y: 0, width: 1200, height: 900 };
+    await setWebviewAccountBounds(ACCOUNT_ID, resized);
+    expect(vi.mocked(invoke)).not.toHaveBeenCalledWith('webview_account_bounds', expect.anything());
+
+    await fireLoadEvent({ state: 'finished', url: 'x' });
+
+    expect(vi.mocked(invoke)).toHaveBeenCalledWith('webview_account_reveal', {
+      args: { account_id: ACCOUNT_ID, bounds: resized },
+    });
+  });
+
+  it('forwards bounds once loading is done', async () => {
+    const initial = { x: 0, y: 0, width: 800, height: 600 };
+    await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds: initial });
+    await fireLoadEvent({ state: 'finished', url: 'x' });
+    vi.mocked(invoke).mockClear();
+
+    const next = { x: 10, y: 10, width: 900, height: 700 };
+    await setWebviewAccountBounds(ACCOUNT_ID, next);
+
+    expect(vi.mocked(invoke)).toHaveBeenCalledWith('webview_account_bounds', {
+      args: { account_id: ACCOUNT_ID, bounds: next },
+    });
+  });
+
+  it('still reveals on timeout fallback', async () => {
+    const bounds = { x: 0, y: 0, width: 800, height: 600 };
+    await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds });
+    vi.mocked(invoke).mockClear();
+
+    await fireLoadEvent({ state: 'timeout', url: '' });
+
+    expect(vi.mocked(invoke)).toHaveBeenCalledWith('webview_account_reveal', {
+      args: { account_id: ACCOUNT_ID, bounds },
+    });
+    expect(store.getState().accounts.accounts[ACCOUNT_ID]?.status).toBe('open');
+  });
+
+  it('treats `reused` event as finished (warm re-open path)', async () => {
+    const bounds = { x: 0, y: 0, width: 800, height: 600 };
+    await openWebviewAccount({ accountId: ACCOUNT_ID, provider: 'telegram', bounds });
+    vi.mocked(invoke).mockClear();
+
+    await fireLoadEvent({ state: 'reused', url: 'https://web.telegram.org/' });
+
+    expect(vi.mocked(invoke)).toHaveBeenCalledWith('webview_account_reveal', {
+      args: { account_id: ACCOUNT_ID, bounds },
+    });
+    expect(store.getState().accounts.accounts[ACCOUNT_ID]?.status).toBe('open');
+  });
+
+  it('skips reveal when the account has already unmounted', async () => {
+    // Fire load event without ever having opened the account (no cached bounds).
+    vi.mocked(invoke).mockClear();
+
+    await fireLoadEvent({ state: 'finished', url: 'x' });
+
+    expect(vi.mocked(invoke)).not.toHaveBeenCalled();
+  });
+});

app/src/services/webviewAccountService.ts

@@ -59,6 +59,22 @@ interface NotificationClickPayload {
   provider: string;
 }
 
+interface WebviewAccountLoadPayload {
+  account_id: string;
+  // `'finished'` — native `on_page_load` or CDP `Page.loadEventFired` fired
+  // `'timeout'`  — 15 s watchdog elapsed; reveal anyway so spinner isn't stuck
+  // `'reused'`   — warm re-open of already-loaded account; reveal synchronously
+  state: 'finished' | 'timeout' | 'reused' | string;
+  url: string;
+}
+
+interface WebviewAccountBounds {
+  x: number;
+  y: number;
+  width: number;
+  height: number;
+}
+
 interface RecipeNotifyPayload {
   title?: string;
   body?: string;
@@ -70,9 +86,23 @@ interface RecipeNotifyPayload {
 
 let unlisten: UnlistenFn | null = null;
 let unlistenNotifyClick: UnlistenFn | null = null;
+let unlistenLoad: UnlistenFn | null = null;
 let started = false;
 let permissionChecked = false;
 
+// Last bounds the frontend handed to Rust per account. Updated on every
+// `setWebviewAccountBounds` call (even when the invoke itself is skipped
+// because the account is still loading). The `webview-account:load` listener
+// reads back from here so it can issue `webview_account_reveal` with the
+// correct rect without a second round-trip.
+const lastBoundsByAccount = new Map<string, WebviewAccountBounds>();
+
+// Track which accounts are still in their initial load cycle (spawned
+// off-screen, waiting for the first page-loaded signal). Bounds updates for
+// these are cached but NOT forwarded to Rust — moving the off-screen webview
+// to the on-screen rect prematurely would defeat the loading overlay.
+const loadingAccounts = new Set<string>();
+
 export function startWebviewAccountService(): void {
   if (started) return;
   if (!isTauri()) {
@@ -100,6 +130,17 @@ export function startWebviewAccountService(): void {
     } catch (err) {
       errLog('failed to attach notification:click listener', err);
     }
+    try {
+      // Rust emits `webview-account:load` from three independent signals
+      // (native `on_page_load`, CDP `Page.loadEventFired`, 15 s watchdog).
+      // It dedups server-side so we see exactly one event per cold open.
+      unlistenLoad = await listen<WebviewAccountLoadPayload>('webview-account:load', evt => {
+        handleWebviewAccountLoad(evt.payload);
+      });
+      log('webview-account:load listener attached');
+    } catch (err) {
+      errLog('failed to attach webview-account:load listener', err);
+    }
   })();
 }
 
@@ -112,9 +153,55 @@ export function stopWebviewAccountService(): void {
     unlistenNotifyClick();
     unlistenNotifyClick = null;
   }
+  if (unlistenLoad) {
+    unlistenLoad();
+    unlistenLoad = null;
+  }
+  // Drop module-level state so a subsequent start (HMR / shutdown→restart)
+  // doesn't see stale per-account entries that survived the listener
+  // teardown. Otherwise an account whose webview was destroyed mid-load
+  // would resurface as "still loading" on restart and silently drop bounds
+  // updates because `loadingAccounts.has(...)` is true.
+  lastBoundsByAccount.clear();
+  loadingAccounts.clear();
   started = false;
 }
 
+function handleWebviewAccountLoad(payload: WebviewAccountLoadPayload) {
+  const accountId = payload?.account_id;
+  if (!accountId) {
+    errLog('webview-account:load missing account_id — ignoring: %o', payload);
+    return;
+  }
+  log('load event account=%s state=%s url=%s', accountId, payload.state, payload.url);
+  loadingAccounts.delete(accountId);
+
+  // Rust already resized the webview to `requested_bounds` as part of
+  // `emit_load_finished`, so the native side is already correct. We still
+  // issue `webview_account_reveal` here as a belt-and-braces idempotent
+  // no-op: if the frontend bounds diverged from the Rust-stored ones (e.g.
+  // a resize landed during the load window) this reapplies the latest
+  // measured rect. When the cache is empty (host already unmounted) we
+  // simply skip.
+  //
+  // Dispatch `'open'` after the reveal settles (success or failure) so the
+  // spinner is only dismissed once the webview is actually positioned. On
+  // error we still flip to `'open'` so the spinner never hangs indefinitely —
+  // the webview will have been positioned server-side by `emit_load_finished`.
+  const bounds = lastBoundsByAccount.get(accountId);
+  if (bounds) {
+    invoke('webview_account_reveal', { args: { account_id: accountId, bounds } })
+      .catch(err => {
+        errLog('webview_account_reveal failed account=%s: %o', accountId, err);
+      })
+      .finally(() => {
+        store.dispatch(setAccountStatus({ accountId, status: 'open' }));
+      });
+  } else {
+    store.dispatch(setAccountStatus({ accountId, status: 'open' }));
+  }
+}
+
 function handleNotificationClick(payload: NotificationClickPayload) {
   const accountId = payload?.account_id;
   const provider = payload?.provider;
@@ -720,16 +807,23 @@ export async function openWebviewAccount(args: OpenAccountArgs): Promise<void> {
   if (!isTauri()) throw new Error('webview accounts require the desktop app');
   log('open account=%s provider=%s', args.accountId, args.provider);
   store.dispatch(setAccountStatus({ accountId: args.accountId, status: 'pending' }));
+  lastBoundsByAccount.set(args.accountId, args.bounds);
+  loadingAccounts.add(args.accountId);
   void ensureNotificationPermission();
   try {
     await invoke('webview_account_open', {
       args: { account_id: args.accountId, provider: args.provider, bounds: args.bounds },
     });
-    store.dispatch(setAccountStatus({ accountId: args.accountId, status: 'open' }));
+    // Rust confirmed `add_child`. The webview is spawned off-screen; keep us
+    // in the loading state until `webview-account:load` arrives (at which point
+    // the listener dispatches `'open'`). Warm re-opens are resolved by the
+    // `'reused'` event which the listener also handles.
+    store.dispatch(setAccountStatus({ accountId: args.accountId, status: 'loading' }));
     void setFocusedAccount(args.accountId);
   } catch (err) {
     const msg = err instanceof Error ? err.message : String(err);
     errLog('open failed: %s', msg);
+    loadingAccounts.delete(args.accountId);
     store.dispatch(
       setAccountStatus({ accountId: args.accountId, status: 'error', lastError: msg })
     );
@@ -739,9 +833,18 @@ export async function openWebviewAccount(args: OpenAccountArgs): Promise<void> {
 
 export async function setWebviewAccountBounds(
   accountId: string,
-  bounds: { x: number; y: number; width: number; height: number }
+  bounds: WebviewAccountBounds
 ): Promise<void> {
   if (!isTauri()) return;
+  // Always keep the cache fresh — the load-event listener needs it whether or
+  // not we forward this particular call to Rust.
+  lastBoundsByAccount.set(accountId, bounds);
+  if (loadingAccounts.has(accountId)) {
+    // Webview is parked off-screen waiting for its first page-loaded signal.
+    // Skip the invoke so we don't drag the CEF subview back on-screen over
+    // the React loading overlay.
+    return;
+  }
   try {
     await invoke('webview_account_bounds', { args: { account_id: accountId, bounds } });
   } catch (err) {
@@ -771,6 +874,8 @@ export async function closeWebviewAccount(accountId: string): Promise<void> {
   if (!isTauri()) return;
   log('close account=%s', accountId);
   await flushMeetingIfAny(accountId, 'webview-closed');
+  lastBoundsByAccount.delete(accountId);
+  loadingAccounts.delete(accountId);
   try {
     await invoke('webview_account_close', { args: { account_id: accountId } });
     store.dispatch(setAccountStatus({ accountId, status: 'closed' }));
@@ -787,6 +892,8 @@ export async function purgeWebviewAccount(accountId: string): Promise<void> {
   if (!isTauri()) return;
   log('purge account=%s', accountId);
   await flushMeetingIfAny(accountId, 'webview-purged');
+  lastBoundsByAccount.delete(accountId);
+  loadingAccounts.delete(accountId);
   try {
     await invoke('webview_account_purge', { args: { account_id: accountId } });
     store.dispatch(setAccountStatus({ accountId, status: 'closed' }));

app/src/types/accounts.ts

@@ -11,7 +11,14 @@ export type AccountProvider =
   | 'zoom'
   | 'browserscan';
 
-export type AccountStatus = 'pending' | 'open' | 'error' | 'closed';
+// Status lifecycle for an embedded webview account:
+//   'pending'  — openWebviewAccount invoked, Rust-side add_child not yet confirmed
+//   'loading'  — CEF child webview spawned off-screen, waiting for first page-loaded
+//                signal; WebviewHost shows its spinner
+//   'open'     — page loaded, webview_account_reveal completed, webview on-screen
+//   'closed'   — webview destroyed
+//   'error'    — open/reveal failed (lastError populated)
+export type AccountStatus = 'pending' | 'loading' | 'open' | 'error' | 'closed';
 
 export interface Account {
   id: string;