diff --git a/frontend/src/app/pages/AgentChat/tool-ui/openUrlFullscreen.test.ts b/frontend/src/app/pages/AgentChat/tool-ui/openUrlFullscreen.test.ts new file mode 100644 index 00000000..62d8fa3d --- /dev/null +++ b/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'); +}); diff --git a/frontend/src/app/pages/AgentChat/tool-ui/useOpenUrlInBrowserCard.ts b/frontend/src/app/pages/AgentChat/tool-ui/useOpenUrlInBrowserCard.ts index c148df31..735a7573 100644 --- a/frontend/src/app/pages/AgentChat/tool-ui/useOpenUrlInBrowserCard.ts +++ b/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(); @@ -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], );