feat(mobile): add manual prompt composer - #444
Conversation
|
Thanks for this, and for following the two-slice split from #359 so closely: this is the manual Compose action with the multiline editor, Send, image attach, in-memory drafts and the escape hatch, with the setting and the tap routing left for the follow-up, which is exactly what I asked for. All four of the load-bearing points from that thread are handled, and putting the regression coverage in the CI-visible One blocker, then some small things. Blocker: Send can only ever fire once per session.
So the only The tests miss it because the JSDOM harness mocks The guard is not the problem: with the mode off,
If neither fits in this PR, collapsing to one line and going through the existing Worth fixing in the same round
Small things, I can take these at merge time
Push the bracketed-paste fix and the composer-emptying one and I will merge. The rest I will tidy on the way in. |
|
One housekeeping note: your CI had never actually run. As a first-time contributor the workflow sat at |
|
Thanks for this, and for turning the bracketed-paste blocker around so quickly. The Compose key, the multiline editor with autocorrect, the per-session in-memory drafts with the marker on the key, the image attach that never touches the PTY, and the escape hatch are all exactly the first slice from #359. Every check passes here: typecheck, lint, format, frontend syntax, public assets, and the full gate at 386 files and 7,292 tests. Two things to fix, then I will merge. 1. The new delivery call is right, but the options object changes what happens when the terminal WebSocket is not open for that session, which on a phone is routine (reconnect backoff, a tab the OS suspended, the window between page load and socket attach). On that fallback the POST goes through const hasCarriageReturn = input.includes('\r');
const textPart = input.replace(/\r/g, '').replace(/\n/g, '').trimEnd();Your payload maps newlines to CR one line earlier, so both fire: tmux sends that, waits 50ms, and sends its own Enter. So the prompt arrives with the lines welded together, gets submitted, and your delayed Without the options object the fallback takes 2. This is the leak from my first round and it is still open. An arrow key from the bar adds the session to app._echoPassthroughSessions?.delete(sessionId);Also worth taking while you are in there
I will take these at merge time
One note on the mobile suite: Push those two and it goes in. |
|
Apologies, my agent got a bit ahead of me and pushed before we were done. I'll comment again when this is ready for your final merge review. |
|
Thanks for this, and for turning both items from the last round around quickly: the bracketed-paste write no longer carries I am holding off on merging, and your own note about the agent pushing early is why. 1. The branch that is up now is not the branch I reviewed. Your three commits ( A lot of it is defensible: (a) take the backend edits back out, land the composer on the one-line Either way, please rebase onto master rather than merging master in again, so the next read is per commit. 2. A chunk that will not fit the wake buffer now returns I think you are right on the merits: 3. 4 KiB becomes 256 KiB per sleeping session, and 4. Docs and dead code that go with 2 and 3.
5.
Smaller things, not blocking
I will take the Tell me which way you want to go on point 1 and I will review from there. The composer itself is good work and I want it in. |
fee6098 to
884713c
Compare
|
Thank you for the detailed feedback and guidance, and apologies for letting the previous revision grow beyond the scope of this PR. There has been a bit of a learning curve for me on this repository, and I over-corrected by trying to solve broader delivery guarantees here. I took option (a), rebuilt the branch directly on current The branch now contains:
I left the CSS cascade, documentation, and defensive Three focused peer reviews came back clean. Typecheck, lint, formatting, frontend syntax, public-assets checks, and the full CI gate pass locally: 405 test files and 7,729 tests. Thanks again for helping me get this back to a focused and reviewable change. |
|
Thanks for turning this around so cleanly, and for taking option (a): this is now the manual Compose slice on its own, with the multiline editor, Send, image attach, per-session in-memory drafts and the escape hatch, and nothing outside I checked the shipped code rather than the commit list, and all five points from the last round are genuinely fixed. Two of them I verified against live processes rather than by reading:
I also confirmed the payload is byte-identical to what Nothing blocking. Two small things I would like on top of what I already said I would take:
Follow-up material, not for this PR: the dialog's new copy ( Everything passes here: typecheck, lint, format, frontend syntax, public assets, the full gate at 405 files and 7,729 tests, and the |
|
Done in As discussed, I have left the shared/global surfaces for your merge-time pass: the composer overlay padding and corresponding |
|
Merged into master for 1.32.0, with the merge-time list applied on the way in (d8e8528): the composer keeps its bottom gutter on folding devices and the foldable-layout guard now covers it, the typeof app guard, the Mobile Guide and a CLAUDE.md entry, plus the whitespace-only draft, the derived length guard and the zh-CN strings. Thank you for taking the scope back down to one slice and for verifying the delivery path against a live pane. Good first contribution. |
- styles.css: restate the composer overlay's own bottom gutter after the fold rules (the generic .paste-overlay longhand erased it: 0px flat, hinge strip replacing it folded) and subtract the fold strip from the dialog's max-height - test/foldable-layout.test.ts: simulate the cascade for .paste-overlay.prompt-composer-overlay (fails without the CSS fix); pin the palette anchor by name instead of ELEMENTS.at(-1) - keyboard-accessory.js: guard the app global in refreshForActiveSession() like the rest of the file - keyboard-accessory.js: a whitespace-only draft is empty (Send no longer submits blank lines); the text still goes out untrimmed - keyboard-accessory.js: derive _composerMaxLength and the frame refusal from one 64 KiB frame limit minus both bracketed-paste markers so they cannot drift - keyboard-accessory.js: translate the textarea placeholder and label at build time, since the DOM translator skips <textarea> subtrees - i18n.js: zh-CN entries for the composer dialog copy - docs/wiki/Mobile-Guide.md: describe the Compose key instead of a clipboard key - CLAUDE.md: a "Mobile prompt composer" paragraph after the accessory bar one - test/mobile-prompt-composer.test.ts: pin the whitespace rule and the derived budget Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit f6725ba52da17b0bdbee8be3b5011e7cae514f69)
Summary
This is the manual-action first slice proposed in #359. The auto-open setting and terminal tap routing remain a separate follow-up.
Verification
npm test— 386 files passed, 1 skipped; 7,292 tests passed, 12 skippednpm run typechecknpm run lintnpm run check:frontend-syntaxgit diff --check