Skip to content

fix(mobile): zoom previews and sync Codex compose - #608

Open
shenlvkang-collab wants to merge 3 commits into
Ark0N:masterfrom
shenlvkang-collab:fix/mobile-image-zoom-codex-compose
Open

shenlvkang-collab wants to merge 3 commits into
Ark0N:masterfrom
shenlvkang-collab:fix/mobile-image-zoom-codex-compose

Conversation

@shenlvkang-collab

@shenlvkang-collab shenlvkang-collab commented Oct 11, 2026 •

Copy link
Copy Markdown
Contributor

What changed

  • Add pinch-to-zoom, drag-to-pan, wheel zoom, double-click zoom, and fit/zoom controls to image previews in the mobile path picker.
  • Keep Codex's mobile Compose textarea and terminal composer synchronized: adopt the current prompt, mirror edits back into the Codex prompt, and preserve the current text when switching back to the terminal keyboard or canceling.
  • Keep Claude's existing local-echo handoff unchanged.
  • Document the Codex handoff and expose the predictive addon's unconfirmed text as a read-only getter.

Verification

  • test/mobile-prompt-composer.test.ts: 20 passed, including Codex live-sync and Claude handoff regression coverage.
  • test/path-picker-ui.test.ts and packages/xterm-zerolag-input/test/predictive-echo-addon.test.ts passed before this follow-up change.
  • Typecheck, format, frontend syntax, and production build passed for this follow-up.
  • Real-browser image preview check loaded an actual image and verified pinch from 100% to 200%, followed by pan.
  • Full CI passed for code commit e00314b (run 38149143773). The current head cbb291c adds documentation only; its CI is running.

@Ark0N

Ark0N commented Oct 11, 2026

Copy link
Copy Markdown
Owner

Thanks for this, @shenlvkang-collab! It adds pinch, pan and wheel zoom to the path picker's image preview, and it makes the mobile Compose dialog take over whatever is already typed in a Codex composer. Today Send just appends to that text, so this fixes a real problem. The zoom code is clean (the teardown in closePreview() is exactly right), and for a single paragraph the Codex handoff works.

I ran the handoff against a real codex-cli 0.162.0. Two composer states go wrong, and both need fixing before merge:

  1. Paste and image placeholders lose their content (src/web/public/terminal-ui.js:4689). Codex shows a paste over about 1000 characters as [Pasted Content 1500 chars] and an attached image as [Image #1]. The real text lives only inside Codex and is expanded at submit. The handoff copies the placeholder string, and the Ctrl+U sent when the dialog opens deletes the placeholder along with its content. Send then submits the literal [Pasted Content 1500 chars]. I confirmed this in Codex's history.jsonl: a direct submit stores the 1500 characters, the Compose path stores the placeholder. Cancel cannot get it back either. Please return '' and send nothing when any adopted row matches /\[Pasted Content \d+ chars\]|\[Image #\d+\]/, and add a unit test for it.

  2. Multi-line prompts get submitted twice (src/web/public/terminal-ui.js:4717). A real newline renders as a row, the same as a soft wrap, so the reader adopts every line. But one Ctrl+U in Codex only clears the current logical line. With first paragraph text and then second paragraph on a new line (Ctrl+J), Ctrl+U leaves the first line in place, and Send submits it again in front of the composed prompt. Please clear every logical line. In Codex, Ctrl+U at the start of a line deletes the newline before it, so '\x15'.repeat(rows.length * 2) empties the composer, and extra presses on an empty composer do nothing. Please check that against a live Codex, the same way the existing predictive-echo notes measure every Codex fact.

Two smaller ones that are worth taking in the same round:

  1. terminal-ui.js:4704: if the user arrowed back into the prompt, the text after the cursor is never adopted, and Ctrl+U (which only kills backwards) leaves it in Codex, so it ends up after the composed prompt. Please refuse the handoff when the cursor row has non-blank text after cursorX or the next row is a non-blank continuation.
  2. terminal-ui.js:4680: please resolve the terminal through _focusedPane() like insertTerminalText and clearTerminalInput next to it do, and return '' when the focused pane is not the primary one. On a wide tablet with the tile grid open, this.terminal is the parked main terminal, so the handoff would read stale rows and clear the live tile's prompt.

If you prefer, I can apply these at merge time:

  • styles.css:15005: object-fit: scale-down instead of contain, so small images are not blown up to full screen.
  • keyboard-accessory.js:665: scale the wheel zoom by deltaY and ignore deltaY === 0. A trackpad scroll currently jumps straight to 600%.
  • keyboard-accessory.js:530: hide the zoom controls when the image fails to load.
  • A behavioural test for _setPreviewImageZoom (focal point and pan clamp) to replace the source-string checks in test/path-picker-ui.test.ts:172, and moving the Codex handoff test into test/mobile-prompt-composer.test.ts.
  • A line in the Mobile prompt composer sections of docs/architecture-invariants.md and CLAUDE.md, which still describe backspace-only adoption.

One process note for next time: zoom and the Codex handoff are unrelated, and two PRs would have been quicker to review. Once 1 and 2 are fixed (and ideally 3 and 4), I'll merge it.

@Ark0N

Ark0N commented Oct 11, 2026

Copy link
Copy Markdown
Owner

Thanks for the follow-up, @shenlvkang-collab. This PR adds pinch, pan and wheel zoom to the path picker's image preview, and makes the mobile Compose dialog take over (and now mirror) whatever is typed in a Codex composer.

The zoom part is in good shape. On the Codex side, terminal-ui.js has not changed since my last review, so points 1 to 4 from that comment are still open, and the live mirroring added in e00314b adds new ways to lose or corrupt a prompt. I reproduced each one against codex-cli 0.162.0 in a 50-column tmux pane, writing the exact bytes the dialog sends.

Must fix before merge

  1. Multi-line drafts are duplicated in Codex on every sync (src/web/public/keyboard-accessory.js:1330, and the same single Ctrl+U at src/web/public/terminal-ui.js:4717). Each debounced sync sends one Ctrl+U and then pastes the whole draft. One Ctrl+U clears only the current logical line, so with line one and line two in the textarea, the next sync leaves line one in place and pastes both lines again. After three edits and Send, the Codex composer held 7 lines where the textarea had 3, and Enter submits that. Enter in the dialog adds a line, so every multi-line Compose on Codex hits this. Please clear every logical line (Ctrl+U at the start of a line deletes the newline before it, and extra presses on an empty composer do nothing).

  2. A long draft is destroyed by Cancel followed by reopening Compose (keyboard-accessory.js:1410, terminal-ui.js:4689). Cancel and "Use terminal keyboard" paste the draft back into Codex, and Codex shows a long paste as a placeholder (a 1500-character draft became [Pasted Content 1500 chars]). Reopening Compose adopts that literal string, overwrites the saved draft with it (pending || saved, then _composerDrafts.set), and pastes the literal back. The real text is gone from both places. This is point 1 of my earlier comment, now triggered by the dialog's own text. Please return '' and send nothing when any adopted row matches /\[Pasted Content \d+ chars\]|\[Image #\d+\]/, never overwrite a saved draft with a prompt you could not adopt, and keep long text as a draft instead of handing it back.

  3. Opening Compose sends a bare Ctrl+U when the adoption declines (keyboard-accessory.js:1451). composePrompt() calls _syncCodexComposerDraft(sessionId, initial) for every Codex session, so when _takeCodexComposerDraft() adopts nothing, the open still sends \x15 and deletes the prompt. Repro: type please review the change in keyboard accessory (with the trailing space) in a 50-column pane. The space wraps, the cursor sits at column 2 of a blank continuation row, _takeCodexComposerDraft() returns '' at terminal-ui.js:4687, and the Ctrl+U from the open wipes the whole prompt. It also cancels out the fix asked for in 2. The open must only write to Codex when the adoption actually took the text.

Should fix in the same round

  1. The adoption rewrites the prompt (terminal-ui.js:4712). Rows are joined with a space, but a row break on screen can be a word wrap, a hard newline or a break inside a token. Codex breaks long paths after /, so fix /home/arkon/codeman-cases/project/src/web/public/keyboard-accessory.js now came back as .../src/web/ public/keyboard-accessory.js now after the handoff, and a Ctrl+J newline becomes a space. On a phone almost every path is longer than a row. The rendered rows cannot tell these cases apart, so please refuse the handoff when the prompt spans more than one row, unless you have a way to reconstruct it exactly.

  2. A prompt submitted from the terminal comes back (keyboard-accessory.js:1410). After Cancel the text lives in Codex and in _composerDrafts. If the user then presses Enter in the terminal, the draft dot stays on, and the next Compose finds an empty Codex composer and pastes the old prompt back into it. If you hand the text back to Codex, clear the draft at that moment.

  3. Points 3 and 4 of my earlier comment are still open: text after the cursor (terminal-ui.js:4702), which with mirroring now stays behind on every sync and is submitted after the composed prompt, and resolving the pane through _focusedPane() (terminal-ui.js:4680).

On the overall shape, I'd drop the per-keystroke mirroring and keep the dialog as the only owner of the prompt while it is open, which is how the Claude path works and what the Mobile prompt composer section of docs/architecture-invariants.md documents. Adopt once on open with a full clear. If you want Cancel to hand the text back, paste it once into the now-empty composer (no Ctrl+U needed) and clear the draft when you do.

Please also update test/mobile-prompt-composer.test.ts with cases for a multi-line draft, a placeholder row and a declined adoption sending nothing (the tests at lines 194 to 216 currently pin the single Ctrl+U), move the _takeCodexComposerDraft test there from test/path-picker-ui.test.ts, and add the Codex exception to the Mobile prompt composer paragraphs in CLAUDE.md and docs/architecture-invariants.md, which still say a non-Send exit leaves the text only in the draft.

The merge-time items from my last comment still stand, and I'll do them myself: object-fit: scale-down (styles.css:15009), wheel zoom scaled by deltaY and ignoring deltaY === 0 (keyboard-accessory.js:667), hiding the zoom controls on image error (keyboard-accessory.js:530), and a behavioural zoom test in place of the source-string checks (test/path-picker-ui.test.ts:172).

Once 1 to 3 are fixed (and ideally 4 to 6), I'll merge. If you'd like the zoom to land sooner, split it into its own PR and I can merge that one right away.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants