diff --git a/mobile/src/session/MobileSessionActiveContent.tsx b/mobile/src/session/MobileSessionActiveContent.tsx index 2481e2b35f9..9c4f0eb313f 100644 --- a/mobile/src/session/MobileSessionActiveContent.tsx +++ b/mobile/src/session/MobileSessionActiveContent.tsx @@ -32,7 +32,6 @@ export function MobileSessionActiveContent({ setShowCreateTabDrawer, dictationMode, toastMessage, - terminalFrameRef, handleTerminalTap, browserScreencastSupported, showToast, @@ -52,11 +51,9 @@ export function MobileSessionActiveContent({ copyMarkdownLocalContent, discardMarkdownLocalContent, saveMarkdownTab, - notifyTerminalFrameHeight, - notifyTerminalFrameWidth, setTerminalWebViewRef, handleTerminalWebReady, - handleTerminalFrameLayout, + notifyTerminalFrame, notifyTerminalCellBoxChange, handleFileTap, handleNativeChatFileTap, @@ -253,20 +250,7 @@ export function MobileSessionActiveContent({ style={styles.contentFrame} onLayout={(e) => { const { width, height } = e.nativeEvent.layout - // Why: notify imperatively so dock settling re-fits the PTY without rerendering SessionScreen. - notifyTerminalFrameHeight(Math.round(height)) - // Why: the page reports a hidden frame as 0x0; it keeps the box it was laid out at. - if (width <= 0) { - return - } - const previous = terminalFrameRef.current - terminalFrameRef.current = { width, height } - if (!previous) { - // Why: a ready document held back for its frame subscribes on the first layout. - handleTerminalFrameLayout() - } else if (width !== previous.width) { - notifyTerminalFrameWidth() - } + notifyTerminalFrame({ width, height }) }} > {content} diff --git a/mobile/src/session/mobile-session-route-parity.test.ts b/mobile/src/session/mobile-session-route-parity.test.ts index 42eef691051..7905a58601e 100644 --- a/mobile/src/session/mobile-session-route-parity.test.ts +++ b/mobile/src/session/mobile-session-route-parity.test.ts @@ -98,14 +98,17 @@ const HOST_COMPONENT_NAMES = new Set([ // width ref and `handleTerminalFrameLayout` subscribe a known box on layout (hooks 282, callbacks 79), // and the pane's `onCellBoxChange` goes to the viewport refit. // Again when one frame ref replaced the height ref, width ref and width state (hooks 280). -const HEAD_MAIN_HOOK_SHA256 = '17366146730f1d910344ea4aad2e74738baea45797380c2b075e63effa1e7fcc' +// Again when one `notifyTerminalFrame` took the frame's layout (hooks 281, callbacks 80). +const HEAD_MAIN_HOOK_SHA256 = '004b011722b17ac82c96f0b3c8e303d39b2431a216424e9b86a1ee6a4896f23e' // Moved when the prompt-cancel flag became one structured-session host support object (main). // Re-recorded against the merged tree. Again when the frame-layout and cell-box-change callbacks // named the refs they read in their dependency lists (react-doctor). // Again when the frame's width and height became one `terminalFrameRef`. -const HEAD_HOOK_BINDING_SHA256 = 'b23d3fb3f8219d1959645f8d4f22cf9414102e9e47abcd68eacec30f4656e7f3' +// Again when the frame's layout moved into `notifyTerminalFrame`. +const HEAD_HOOK_BINDING_SHA256 = 'c1bcb859202aaf8d612d3023cb4d895719da513879c61b01545a249a0bda662b' +// Moved when `notifyTerminalFrame` joined and `handleTerminalFrameLayout` became `subscribeIntendedActiveTerminal`. const HEAD_CALLBACK_IDENTITY_SHA256 = - '19201fe156b3fd9d1bd55418c2c54e2d2319c652bfe55732b26f331c458f56c9' + '9c966301b373c11b27359ef1388c593b638c7f6831186d5c5321553504183e07' // Pins that no callback body in the route changed unnoticed. Body text, not behaviour: the sends // and repo reads inside them now name their `RpcOperation` instead of the raw `sendRequest` port. // Refreshed in step 6 for the gesture flush, whose `terminal.send` became `terminalInputSend` and @@ -132,7 +135,8 @@ const HEAD_CALLBACK_IDENTITY_SHA256 = // Again when the subscribe sized its viewport inline instead of through a helper. // Again when the document took the hold rule and the subscribe stopped holding its grid. // Again when an init took one options object. -const HEAD_CALLBACK_BODY_SHA256 = 'aae361e46d8bf59a4957dfcf9a1167c86af9314a998676235f73b44a3557d52e' +// Again when the frame's layout became one `notifyTerminalFrame`. +const HEAD_CALLBACK_BODY_SHA256 = '8f2ecfc86d6b50dfcf2248135c12a1a97c0b63a50176fc330781e805f388bbd3' // Refreshed for the startup effect: both `worktree.activate` sends became `worktreeActivate`, and // the sleeping-agent check reads that operation's verdict instead of the reply envelope. Refreshed // again when the reporter took the reply and interpreted it itself, retiring the hand-built @@ -201,7 +205,8 @@ const HEAD_RUNTIME_STRING_SHA256 = // Moved again when the terminal frame kept its laid-out width unrounded, for every fit. // Again when the frame's onLayout wrote one frame ref and notified a new width imperatively. // Again when the frame's first laid-out layout alone subscribes a held-back document. -const HEAD_HOST_JSX_SHA256 = '5e2d0ec5bdb838560d06d4b79c8b59d72d1f553394d1580e2250b352cda18401' +// Again when the frame's onLayout made one `notifyTerminalFrame` call. +const HEAD_HOST_JSX_SHA256 = '8ab32926fff2e610ab92bde8437283a11ad2328c6343cd862a1c00b2ed8ed5a6' const HEAD_LEAF_JSX_SHA256 = '62eb05c6e2ac0be6d553a141fc8aa1641fcb0c678777d5d539f490aab8648417' const HEAD_STYLE_REFERENCE_SHA256 = '56a005a1f65b30c11092e3422caef67810e1ec50f66fdd06471c370138b1eeb6' @@ -599,10 +604,10 @@ describe('mobile session route extraction parity', () => { const contentBindings = CONTENT_COMPONENT_NAMES.flatMap( (name) => readHookFacts(name, definitions).bindings ) - expect(main.hooks).toHaveLength(280) + expect(main.hooks).toHaveLength(281) expect(hash(main.hooks)).toBe(HEAD_MAIN_HOOK_SHA256) expect(hash(main.bindings)).toBe(HEAD_HOOK_BINDING_SHA256) - expect(main.callbacks).toHaveLength(79) + expect(main.callbacks).toHaveLength(80) expect(hash(main.callbacks)).toBe(HEAD_CALLBACK_IDENTITY_SHA256) expect(hash(main.callbackBodies)).toBe(HEAD_CALLBACK_BODY_SHA256) expect(main.effects).toHaveLength(24) diff --git a/mobile/src/session/mobile-session-terminal-frame-layout.test.tsx b/mobile/src/session/mobile-session-terminal-frame-layout.test.tsx index fdf340a43b4..1cec63717c7 100644 --- a/mobile/src/session/mobile-session-terminal-frame-layout.test.tsx +++ b/mobile/src/session/mobile-session-terminal-frame-layout.test.tsx @@ -49,12 +49,7 @@ type Branch = 'loading' | 'pending' | 'terminal' function controller( branch: Branch | boolean, - notifyTerminalFrameHeight: (height: number) => void, - frame: { - terminalFrameRef?: { current: { width: number; height: number } | null } - notifyTerminalFrameWidth?: () => void - handleTerminalFrameLayout?: () => void - } = {} + notifyTerminalFrame: (frame: { width: number; height: number }) => void ): Controller { const shown = branch === true ? 'loading' : branch === false ? 'terminal' : branch const scope = { @@ -63,10 +58,7 @@ function controller( isPendingTerminalRecoveryParked: false, showEmptyState: false, terminals: [], - terminalFrameRef: frame.terminalFrameRef ?? { current: null }, - handleTerminalFrameLayout: frame.handleTerminalFrameLayout ?? (() => {}), - notifyTerminalFrameHeight, - notifyTerminalFrameWidth: frame.notifyTerminalFrameWidth ?? (() => {}), + notifyTerminalFrame, dictation: { isRecording: false }, nativeChatSendError: { message: null, clear: () => {} } } @@ -77,8 +69,8 @@ function controller( describe('the terminal frame on the page', () => { it('reports its height when the session opens from the loading state', () => { const heights: number[] = [] - const notify = (height: number): void => { - heights.push(height) + const notify = (frame: { height: number }): void => { + heights.push(frame.height) } let renderer: ReturnType | undefined act(() => { @@ -97,8 +89,8 @@ describe('the terminal frame on the page', () => { it('stays one mounted frame across loading, a pending terminal and the terminal', () => { // A frame that remounts per branch reports again on every return; one that stays reports once. const heights: number[] = [] - const notify = (height: number): void => { - heights.push(height) + const notify = (frame: { height: number }): void => { + heights.push(frame.height) } let renderer: ReturnType | undefined const show = (branch: Branch) => { @@ -119,57 +111,4 @@ describe('the terminal frame on the page', () => { show('terminal') expect(heights).toEqual([FRAME.height]) }) - - it('keeps one frame, notifies a new width, and keeps the box through a hidden 0x0 layout', () => { - const terminalFrameRef: { current: { width: number; height: number } | null } = { - current: null - } - const notifyTerminalFrameWidth = vi.fn() - let renderer: ReturnType | undefined - act(() => { - renderer = create( - createElement(MobileSessionActiveContent, { - controller: controller(false, () => {}, { terminalFrameRef, notifyTerminalFrameWidth }) - }) - ) - }) - expect(terminalFrameRef.current).toEqual(FRAME) - const layOut = (width: number, height: number) => - act(() => { - renderer!.root - .findAll((node) => typeof node.props.onLayout === 'function')[0]! - .props.onLayout({ nativeEvent: { layout: { width, height } } }) - }) - layOut(FRAME.width, 560) - expect(notifyTerminalFrameWidth).not.toHaveBeenCalled() - layOut(0, 0) - expect(terminalFrameRef.current).toEqual({ width: FRAME.width, height: 560 }) - layOut(360.5, 560) - expect(terminalFrameRef.current).toEqual({ width: 360.5, height: 560 }) - expect(notifyTerminalFrameWidth).toHaveBeenCalledTimes(1) - }) - - it('subscribes on the first laid-out frame only, not on every layout after it', () => { - const terminalFrameRef: { current: { width: number; height: number } | null } = { - current: null - } - const handleTerminalFrameLayout = vi.fn() - let renderer: ReturnType | undefined - act(() => { - renderer = create( - createElement(MobileSessionActiveContent, { - controller: controller(false, () => {}, { terminalFrameRef, handleTerminalFrameLayout }) - }) - ) - }) - const layOut = (width: number, height: number) => - act(() => { - renderer!.root - .findAll((node) => typeof node.props.onLayout === 'function')[0]! - .props.onLayout({ nativeEvent: { layout: { width, height } } }) - }) - layOut(FRAME.width, 560) - layOut(360, 560) - expect(handleTerminalFrameLayout).toHaveBeenCalledTimes(1) - }) }) diff --git a/mobile/src/session/use-mobile-session-terminal-subscription.test.tsx b/mobile/src/session/use-mobile-session-terminal-subscription.test.tsx index 270d9083a72..ef2954e183b 100644 --- a/mobile/src/session/use-mobile-session-terminal-subscription.test.tsx +++ b/mobile/src/session/use-mobile-session-terminal-subscription.test.tsx @@ -97,10 +97,13 @@ function subscriptionHarness(opts: { markdownDocs: new Map(), fileDocs: new Map(), readMarkdownTab: vi.fn(), - readFileTab: vi.fn() + readFileTab: vi.fn(), + notifyTerminalFrameHeight: vi.fn(), + notifyTerminalFrameWidth: vi.fn() } let subscribe: ((handle: string) => void) | undefined let webReady: ((handle: string) => void) | undefined + let notifyFrame: ((frame: { width: number; height: number }) => void) | undefined function Probe() { // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the hook destructures only the fields built above. const scope = fields as unknown as MobileSessionTerminalSubscriptionFoundationModel @@ -108,7 +111,9 @@ function subscriptionHarness(opts: { const withSubscribe = { ...fields, subscribeToTerminal: subscribe } // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the web-ready handler reads only the fields built above. const webviewScope = withSubscribe as unknown as MobileSessionTabSwitchingModel - webReady = useMobileSessionTerminalWebview(webviewScope).handleTerminalWebReady + const webview = useMobileSessionTerminalWebview(webviewScope) + webReady = webview.handleTerminalWebReady + notifyFrame = webview.notifyTerminalFrame return null } act(() => { @@ -127,9 +132,10 @@ function subscriptionHarness(opts: { documentReady: () => { act(() => webReady!(HANDLE)) }, - layOutFrame: (width: number) => { - terminalFrameRef.current = { width, height: 751 } - } + fields, + terminalFrameRef, + // The frame's onLayout. + layOutFrame: (width: number, height = 751) => act(() => notifyFrame!({ width, height })) } } @@ -184,11 +190,31 @@ describe('a terminal first subscribe', () => { const harness = subscriptionHarness({ fit: null, webReady: true, frameWidth: 0 }) harness.subscribe() expect(harness.order).toEqual([]) + // The frame's first layout subscribes the document held back for it. harness.layOutFrame(427) - harness.subscribe() expect(harness.order).toEqual(['subscribe null']) }) + it('keeps one frame, notifies a new width, and subscribes on the first layout only', () => { + const harness = subscriptionHarness({ fit: null, webReady: true, frameWidth: 0 }) + const { notifyTerminalFrameHeight, notifyTerminalFrameWidth } = harness.fields + harness.layOutFrame(0, 0) + expect(harness.terminalFrameRef.current).toBeNull() + harness.layOutFrame(427, 751) + harness.layOutFrame(427, 700) + expect(harness.order).toEqual(['subscribe null']) + expect(notifyTerminalFrameWidth).not.toHaveBeenCalled() + // A hidden 0x0 layout keeps the box it was laid out at. + harness.layOutFrame(0, 0) + expect(harness.terminalFrameRef.current).toEqual({ width: 427, height: 700 }) + harness.layOutFrame(360.5, 700) + expect(harness.terminalFrameRef.current).toEqual({ width: 360.5, height: 700 }) + expect(notifyTerminalFrameWidth).toHaveBeenCalledTimes(1) + expect(notifyTerminalFrameHeight.mock.calls.map(([height]) => height)).toEqual([ + 0, 751, 700, 0, 700 + ]) + }) + it('goes without dims when no cell box was reported, and the fit pass resubscribes once', async () => { const harness = subscriptionHarness({ fit: null, webReady: true }) harness.subscribe() diff --git a/mobile/src/session/use-mobile-session-terminal-webview.ts b/mobile/src/session/use-mobile-session-terminal-webview.ts index 08a7386b870..484930f1218 100644 --- a/mobile/src/session/use-mobile-session-terminal-webview.ts +++ b/mobile/src/session/use-mobile-session-terminal-webview.ts @@ -1,5 +1,6 @@ import { useEffect, useCallback } from 'react' import type { TerminalWebViewHandle } from '../terminal/terminal-webview-contract' +import type { TerminalFrame } from '../terminal/terminal-webview-messages' import type { MobileSessionTabSwitchingModel } from './use-mobile-session-tab-switching' export function useMobileSessionTerminalWebview(scope: MobileSessionTabSwitchingModel) { @@ -21,7 +22,10 @@ export function useMobileSessionTerminalWebview(scope: MobileSessionTabSwitching subscribeToTerminal, nativeChatStream, readMarkdownTab, - readFileTab + readFileTab, + terminalFrameRef, + notifyTerminalFrameHeight, + notifyTerminalFrameWidth } = scope // Why: only store the ref; subscribe on web-ready to avoid the blank-terminal race (init queued before xterm.js loaded). const setTerminalWebViewRef = useCallback((handle: string, ref: TerminalWebViewHandle | null) => { @@ -70,14 +74,39 @@ export function useMobileSessionTerminalWebview(scope: MobileSessionTabSwitching [nativeChatStream, subscribeToTerminal, unsubscribeTerminal] ) - /** The frame has a size: a ready document held back for it subscribes now. */ - const handleTerminalFrameLayout = useCallback(() => { + const subscribeIntendedActiveTerminal = useCallback(() => { const handle = pendingActiveTerminalHandleRef.current ?? activeHandleRef.current if (handle && !terminalUnsubsRef.current.has(handle)) { subscribeToTerminal(handle) } }, [activeHandleRef, pendingActiveTerminalHandleRef, subscribeToTerminal, terminalUnsubsRef]) + /** The terminal frame React Native laid out: kept in the one frame ref, then what it changed. */ + const notifyTerminalFrame = useCallback( + (frame: TerminalFrame) => { + // Why: notify height imperatively so dock settling re-fits the PTY without rerendering SessionScreen. + notifyTerminalFrameHeight(Math.round(frame.height)) + // Why: the page reports a hidden frame as 0x0; it keeps the box it was laid out at. + if (frame.width <= 0) { + return + } + const previous = terminalFrameRef.current + terminalFrameRef.current = frame + if (!previous) { + // Why: a ready document held back for its frame subscribes on the first layout. + subscribeIntendedActiveTerminal() + } else if (frame.width !== previous.width) { + notifyTerminalFrameWidth() + } + }, + [ + notifyTerminalFrameHeight, + notifyTerminalFrameWidth, + subscribeIntendedActiveTerminal, + terminalFrameRef + ] + ) + useEffect(() => { if (activeSessionTab?.type !== 'markdown') { return @@ -100,7 +129,7 @@ export function useMobileSessionTerminalWebview(scope: MobileSessionTabSwitching return { setTerminalWebViewRef, handleTerminalWebReady, - handleTerminalFrameLayout + notifyTerminalFrame } } diff --git a/mobile/src/terminal/terminal-viewport-refit.test.ts b/mobile/src/terminal/terminal-viewport-refit.test.ts index a8fa2bf602c..2247347c7e2 100644 --- a/mobile/src/terminal/terminal-viewport-refit.test.ts +++ b/mobile/src/terminal/terminal-viewport-refit.test.ts @@ -15,7 +15,8 @@ import { const hookSource = readFileSync(new URL('./terminal-viewport-refit.ts', import.meta.url), 'utf8') const sessionSource = [ readMobileSessionRouteSource('../session/use-mobile-session-keyboard-state.ts'), - readMobileSessionRouteSource('../session/MobileSessionActiveContent.tsx') + readMobileSessionRouteSource('../session/MobileSessionActiveContent.tsx'), + readMobileSessionRouteSource('../session/use-mobile-session-terminal-webview.ts') ].join('\n') describe('terminal viewport refit', () => { @@ -152,7 +153,8 @@ describe('terminal viewport refit', () => { expect(sessionSource).toContain('tabStripVisible: terminals.length > 1') expect(sessionSource).toContain('textScale: terminalTextScale') expect(sessionSource).toContain('connState,') - expect(sessionSource).toContain('notifyTerminalFrameHeight(Math.round(height))') + expect(sessionSource).toContain('notifyTerminalFrame({ width, height })') + expect(sessionSource).toContain('notifyTerminalFrameHeight(Math.round(frame.height))') expect(sessionSource).toContain('notifyTerminalFrameWidth()') // One seam for both facts: on the page they come apart, because the shell shortens the WebView // and the keyboard covers nothing the screen has to lift for.