Repository navigation
fix(terminal): fold an attached pane a phone watches at a wider grid - #1963
Conversation
An attached pane drawn at a grid this device does not size (a device paired to watch, or a driving phone under its soft keyboard while the pty keeps another device's size) is folded to the screen's width with #1961's view, drawn from the xterm buffer the attach stream wrote. The pane menu offers Original width / Wrap to screen for such panes under the same terminalMirrorWrap setting (ovis:mirror-wrap kept). A phone that drives the pty is never folded.
Review follow-ups. A grid held under the soft keyboard at another device's size is not adopted, so its mount kept overflow hidden: a wheel or a finger now pans its folded rows (overflow-y on every fold; a touch pans before it scrolls the pane at the folded rows' edges). A pane switch decides the fold again, so the pane menu forgets the last pane's width. A tap on folded rows stays a link or a focus, not a mouse click: herdr's attach reports the mouse for every pane, so a program that wants clicks cannot be told apart (Original width keeps clicks). The regression replays herdr's mouse modes in its attached fixture and drives real swipes and wheels instead of setting scroll positions.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
| Layer / File(s) | Summary |
|---|---|
Foreign-grid fold state src/lib/terminalFold.ts, src/lib/useTerminalMount.ts, src/lib/settings.ts, src/lib/terminalFold.test.ts |
Folding now checks foreign-grid status, width, and the wrap setting. Mounts track wide-grid ownership and synchronize fold state during keyboard hold and restoration. |
Pane menu and folded-grid interaction src/lib/usePaneAttach.ts, src/lib/useTerminalRefits.ts, src/lib/useTerminalMount.ts, src/components/PaneTerminal.tsx, src/components/PaneTerminal.css, src/components/PaneCanvas.tsx |
The pane menu offers the wrap choice for tracked wide grids. Folded rows support vertical scrolling. Attach resets and observed-terminal refits call the cursor-follow callback. |
Attached-pane regression coverage scripts/mirror-fold-demo-regression.ts, DESIGN.md, changelog.d/wrap-view-attach.md, docs/development.md |
The regression fixture checks driving and watching phones, original-width panning, keyboard-held folding, and IME visibility. Documentation describes the attached-pane behavior. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Bug fix
Possibly related issues
- devswha/herdr-web-ui issue 1053: The PR covers related phone sizing and touch-scroll behavior for watched or keyboard-held foreign grids.
Merge Risk: 
a091b
-
The attached-pane folding change is mergeable after normal checks; no concrete blocking risk remains identified.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 9 files. (4 skipped: 4… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title "fix(terminal): fold an attached pane a phone watches at a wider grid" matches the main change. The PR folds attached panes whose pty grid is wider than a phone that does not size the grid. … Description check The description follows the repository template. It has a Change section that describes the problem, the fix, and the resulting behavior. It has a Validation section that lists the browser regression … Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "fix(terminal): fold an attached pane a phone watches at a wider grid" matches the main change. The PR folds attached panes whose pty grid is wider than a phone that does not size the grid. The title is specific and uses a conventional commit prefix. It omits the keyboard-held case and the pane menu option, which the title does not need to cover.
Full details: Description check
-
Explanation
The description follows the repository template. It has a Change section that describes the problem, the fix, and the resulting behavior. It has a Validation section that lists the browser regression script, unit tests, the bun check run, and manual runs on Headless Chrome, Android, and iOS Simulator. The template asks for contract-test coverage for HTTP/WS changes. The PR states the change is client only and does not touch server/ or shared/, so that item does not apply. The description does not link an issue for the feature. The template only requires that for features, and this is a fix that references
#1961and#1045. The description also refers to a changelog entry only in the summary, not in the description body. The template asks authors to see CONTRIBUTING.md for the changelog. The changelog.d entry exists in the PR summary, so this is covered.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 37.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 9 files. (4 skipped: 4 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
Merge pass review of a091b37, part 1: Codex Sol (gpt-6.1-sol, xhigh) requests changes. The Astra gate is still running, and I will post its result separately. Checks so far:
MAJOR: an editable attached fold has no cursor, and the preedit lands on the wrong segment (
MINOR: a pan that takes over a swipe leaves queued history reports (
The re-review will cover only the new commits. |
|
Merge pass review of a091b37, part 2: the Astra gate (omo-native-gate-reviewer, gpt-6-astra) approves the attached-pane fold against the criteria it was given.
Its scenarios did not cover the case Codex found in part 1: a caret moved back into an earlier wrapped segment. So Codex's MAJOR (no drawn cursor and a misplaced IME preedit on an editable fold) still needs the fix and a regression case before merge. The MINOR (queued |
…t on it Merge-pass review follow-ups. A folded attached pane hid xterm's cursor and drew none, and the IME stood at the end of the cursor's folded row. The cursor's cell is now drawn in the folded rows in the grid's shape and colours (read from the cell xterm's DOM renderer marks, so a mirror's hidden cursor stays hidden), and the hidden grid moves down to the folded line the caret is on and sideways to its column: it scrolls within .xterm as far as that goes and moves by left for the rest, so the input stays inside .xterm's clip. A swipe that turns into a local pan also cancels the history reports it had already queued. The regression moves a shell's caret back into the first line of a long folded command and onto its last, and checks the drawn caret and that a Korean preedit stands on it.
|
Review follow-up in 66aada6 (one commit, pushed after MAJOR, no caret and a misplaced preedit on an editable fold: fixed.
MINOR, queued history reports after a pan takes the swipe: fixed. The pan now also cancels Astra note (Original width under a driving phone's held keyboard does not pan sideways): this predates the PR, as Astra says (#1045's held grid is not adopted, so its mount clips). It is left as is here. Checks on 66aada6: |
|
Merge pass re-review of 66aada6 (Codex Sol): one small fix is left. Fixed:
MINOR, still open: cursor at the right margin (
Only this change will be re-reviewed. |
…sor waits to wrap A write that fills a row leaves xterm's cursor one past the last column until the next character wraps it; xterm draws it and stands its input on the last cell. The fold now does the same, so the caret and the preedit stay at the right margin.
|
Re-review follow-up in fed0c50: the cursor at the right margin.
|
|
Merge pass review report. Overall: PASSED. This is an ordinary client change in the terminal attach path. Reviewers: the Astra gate (omo-native-gate-reviewer, gpt-6-astra), Codex Sol (gpt-6.1-sol, xhigh) and my own QA. Astra gate (a091b37): APPROVE.
Codex findings and how each was resolved:
Tests:
Known gap (predates this PR, from the Astra gate): Original width under a driving phone's held keyboard still has no local horizontal panning. This PR shares files only with outside PRs #1639 and #1911, which already conflict with main. |
Problem
#1961 folds a mirrored pane (a Windows PC) that is wider than a phone. An attached pane (a Linux or
macOS PC) gets cut off in the same way whenever the phone is not the one sizing the pty:
held the pane at 174x53. On a 390 px phone every row ran off the right edge, with 1365 px to pan
sideways.
(Terminal (phone): opening the soft keyboard resizes the shared pty #1045). If a desktop takes the pty in the meantime, the phone draws the desktop's grid. The same
happens when the terminal lens comes back from Chat with the keyboard still up.
When the phone drives the pty, the program redraws at the phone's width, so there is nothing to fold.
The Collie comparison (lane collie-research, section 8 item 1) recommended this reading view for
these cases.
Fix
The change is client only.
server/,shared/, herdr and the way devices share a pane's size areunchanged.
lib/terminalFold.ts: whether to fold is now decided byforeign()(this browser does not sizethe grid) and
wrap()(the user's choice). The fold also reports when a foreign grid is wider thanits mount.
lib/useTerminalMount.ts: a grid counts as foreign when it is a mirror, a watched grid, or a gridheld under the keyboard. A split's following half never counts. The folded rows come from the xterm
buffer the attach stream wrote: there is no
pane.read, scrollback stays 0, and nothing resizes,takes over or refocuses the pane. Cursor, IME and hidden-input handling is fix(terminal): fold a mirrored pane wider than a phone to the screen #1961's: the hidden grid
still moves under the folded cursor row.
wideGridsstore, fed by the fold, lets the tab's ⋯ sheet offer Originalwidth / Wrap to screen for such panes, as it already did for mirrors. It uses the same device
setting (
terminalMirrorWrap, keyovis:mirror-wrap), which is not renamed, so choices alreadysaved still apply. A driving phone's menu does not show the item.
overflow: hidden. Every fold nowscrolls up and down, and a finger pans the folded rows before it scrolls the pane at their edges.
choice for the pane left behind.
send a mouse click to the program. herdr's attach turns on mouse reporting for every pane
(
?1000h ?1002h ?1003h ?1006h, seen on a plain bash pane and on btop), so the client cannot tellwhich program wants clicks. Original width still passes clicks through. DESIGN.md says so.
Design precedent: the same as #1961. The apps this repo compares against size their terminal to
the window, which a watching device cannot do. This follows Collie (AltanS/collie,
ansi-output.tsx), which never resizes a pane. No code was copied. DESIGN.md extends #1961's entry.Validation
scripts/mirror-fold-demo-regression.ts(browser lane) now covers an attached pane whose pty grid(120x40) is wider than the phone. The fixture replays herdr's mouse modes, as a real attach does.
Original width.
in order, and the Korean words fold whole.
and real swipes pan it to its right edge. Wrap to screen folds it again. Both choices are kept
in
ovis:mirror-wrap.drawn over the folded rows, panned by wheel and by swipe. Once the keyboard goes, the catch-up fit
unfolds it.
What fails without the change:
foldState(watching):{"folded":false}.showing the menu item only for mirrors, gating the keyboard fold on "no mouse reporting", and the
first commit without the scroll fixes (
scrollTopstuck under the keyboard).Other checks:
src/lib/terminalFold.test.ts: thewideGridsstore keeps a pane wide while any terminal thatdraws it says so.
bun run check fastis green on a091b37 (unit tests: 7176 pass, 0 fail).Real apps were run against an isolated test herdr (
bun run check run) with an omp-like pane (longKorean lines, a status box) and btop. A 1700 px desktop client held the pane at 174x53. The phone was
a device paired to watch: the server reported
via: device, role: watch. "Before" is an origin/mainserver and "after" is this branch's.
Screenshots and the Korean summary for the maintainer (tailnet): http://100.123.228.51:7593/
Reviews
An
omo-native-code-reviewerpass approved with notes, and a Codex (gpt-6.1-sol, xhigh) pass ran onthe first commit. Neither found anything HIGH.
overflow: hidden, so a wheel could not pan itscrollLeftdirectlywideGridskey after a pane switch; comments; the geometry push before the keyboard step; try/finally in the unit testKnown limits
checked in portrait only. Android covered the toggle and landscape.