[eric] tool-ui: a link clicked in a fullscreen chat opens the browser IN fullscreen instead of behind it

frontend/src/app/pages/AgentChat/tool-ui/openUrlFullscreen.test.ts

@@ -0,0 +1,60 @@
+// Clicking a widget link from a FULLSCREEN chat used to look like a broken click: the chat dropped
+// fullscreen for a frame ("glitches"), the browser card landed on the canvas behind it, and the page
+// only appeared later when the user left fullscreen by hand. ENG-234 fixed the card being invisible;
+// it did not fix WHERE the card goes.
+//
+// The rule this codebase already applies for apps (AppShell.navigateToApp): while something is
+// fullscreen, opening a new thing SWAPS the fullscreen to it, because otherwise the new card lands
+// invisibly behind. The reducer evicts any other fullscreen in the same write, so the swap is atomic
+// and there is no un-tiled frame for anything to race with.
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { readFileSync } from 'node:fs';
+
+import reducer, { addBrowserCard, setTiledCard } from '../../../../shared/state/dashboardLayoutSlice';
+
+const SRC = readFileSync('src/app/pages/AgentChat/tool-ui/useOpenUrlInBrowserCard.ts', 'utf8');
+
+test('the hook swaps fullscreen instead of dropping it', () => {
+  assert.match(SRC, /setTiledCard\(\{ cardId: openedId, zone: 'fullscreen' \}\)/,
+    'a link opened from fullscreen must put the BROWSER in the fullscreen slot');
+  assert.doesNotMatch(SRC, /clearTiledCard/,
+    'dropping fullscreen is what left the page hidden behind the chat');
+});
+
+test('it only swaps when something WAS fullscreen (a normal click must not tile)', () => {
+  const gate = SRC.indexOf('if (!wasFullscreen) return;');
+  const swap = SRC.indexOf('setTiledCard(');
+  assert.ok(gate > 0 && gate < swap, 'the non-fullscreen path must return before tiling anything');
+});
+
+test('a browser card is a tileable owner, so the swap is not a silent no-op', () => {
+  // setTiledCard refuses ids it does not recognise (tileOwnerExists). If browserCards were not in
+  // that list the fix would dispatch, change nothing, and look exactly like the bug it replaced.
+  let s = reducer(undefined, { type: '@@init' });
+  s = reducer(s, addBrowserCard({ url: 'https://example.com' }));
+  const id = s.pendingFocusBrowserId as string;
+  s = reducer(s, setTiledCard({ cardId: id, zone: 'fullscreen' }));
+  assert.equal(s.tiledCards[id], 'fullscreen', 'the browser card must actually take the slot');
+});
+
+test('the swap evicts the previous fullscreen in one write (what makes it atomic)', () => {
+  let s = reducer(undefined, { type: '@@init' });
+  s = reducer(s, addBrowserCard({ url: 'https://first.example' }));
+  const first = s.pendingFocusBrowserId as string;
+  s = reducer(s, setTiledCard({ cardId: first, zone: 'fullscreen' }));
+  assert.equal(s.tiledCards[first], 'fullscreen');
+  s = reducer(s, addBrowserCard({ url: 'https://second.example' }));
+  const second = s.pendingFocusBrowserId as string;
+  s = reducer(s, setTiledCard({ cardId: second, zone: 'fullscreen' }));
+  assert.equal(s.tiledCards[second], 'fullscreen');
+  assert.equal(s.tiledCards[first], undefined,
+    'two fullscreen cards at once would be the invisible-card bug all over again');
+});
+
+test('addBrowserCard hands the new id back through pendingFocusBrowserId', () => {
+  let s = reducer(undefined, { type: '@@init' });
+  s = reducer(s, addBrowserCard({ url: 'https://example.com' }));
+  const id = s.pendingFocusBrowserId;
+  assert.ok(id && s.browserCards[id], 'the hook has no other handle on the card it just created');
+});

frontend/src/app/pages/AgentChat/tool-ui/useOpenUrlInBrowserCard.ts

@@ -1,9 +1,13 @@
 import { useCallback } from 'react';
 import { useDispatch, useStore } from 'react-redux';
-import { addBrowserCard, clearTiledCard } from '@/shared/state/dashboardLayoutSlice';
+import { addBrowserCard, setTiledCard } from '@/shared/state/dashboardLayoutSlice';
 import type { RootState } from '@/shared/state/store';
 
-/** Opens a widget link as an in-app browser card, leaving fullscreen first so the card is actually visible (ENG-234). */
+/** Opens a widget link as an in-app browser card. From a FULLSCREEN chat the browser takes the
+ * fullscreen slot, the same swap the sidebar already does for apps ("otherwise the new card would
+ * land invisibly behind it"). The old version instead dropped fullscreen and left the card on the
+ * canvas behind the chat, which read as a click that glitched and did nothing; the page only turned
+ * up later, after leaving fullscreen by hand (ENG-234 fixed the visibility, not the destination). */
 export function useOpenUrlInBrowserCard(): (url: string) => void {
   const dispatch = useDispatch();
   const store = useStore<RootState>();
@@ -12,10 +16,14 @@ export function useOpenUrlInBrowserCard(): (url: string) => void {
       if (!/^https?:\/\//i.test(url)) return;
       // Lazy read, no subscription: this fires on a click, and widgets must not re-render on every tile change.
       const tiled = store.getState().dashboardLayout.tiledCards || {};
-      for (const [cardId, zone] of Object.entries(tiled)) {
-        if (zone === 'fullscreen') dispatch(clearTiledCard(cardId));
-      }
+      const wasFullscreen = Object.values(tiled).some((zone) => zone === 'fullscreen');
       dispatch(addBrowserCard({ url }));
+      if (!wasFullscreen) return;
+      // The reducer mints the id, so the handle comes back through pendingFocusBrowserId; tiling it
+      // fullscreen evicts the chat's own fullscreen in the SAME reducer, so there is no un-tile
+      // frame to flicker through and nothing for a re-tile to race against.
+      const openedId = store.getState().dashboardLayout.pendingFocusBrowserId;
+      if (openedId) dispatch(setTiledCard({ cardId: openedId, zone: 'fullscreen' }));
     },
     [dispatch, store],
   );