|
| 1 | +# Mobile Terminal Long-Press Line Copy Design |
| 2 | + |
| 3 | +## 1. Goal |
| 4 | + |
| 5 | +Replace the current mobile terminal copy-mode overlay with a simpler interaction: |
| 6 | + |
| 7 | +- when `Copy on select` is enabled on mobile, long-pressing terminal output copies the current wrapped logical line directly |
| 8 | +- the copied text comes from the frontend xterm active buffer, not a server request |
| 9 | +- wrapped terminal rows are merged into one logical line before copying |
| 10 | +- right-side terminal padding whitespace is trimmed from the logical line ending, while meaningful internal spaces are preserved |
| 11 | +- success feedback is a toast plus a short vibration |
| 12 | + |
| 13 | +This change intentionally removes the current mobile copy-mode overlay flow instead of refining it. |
| 14 | + |
| 15 | +## 2. Current Baseline |
| 16 | + |
| 17 | +Relevant current implementation: |
| 18 | + |
| 19 | +- `packages/web/src/features/terminal-panel/views/shared/xterm-host.tsx` |
| 20 | +- `packages/web/src/features/terminal-panel/mobile/copy-mode-snapshot.ts` |
| 21 | +- `packages/web/src/styles/components.css` |
| 22 | +- `packages/web/src/features/terminal-panel/__tests__/xterm-host.test.tsx` |
| 23 | +- `e2e/specs/settings/mobile-copy-on-select.spec.ts` |
| 24 | +- `packages/web/src/locales/en.json` |
| 25 | +- `packages/web/src/locales/zh.json` |
| 26 | + |
| 27 | +Today: |
| 28 | + |
| 29 | +- mobile long press enters a full copy-mode overlay |
| 30 | +- the overlay is built from cloned `.xterm-rows` DOM |
| 31 | +- once copy mode is active, our custom terminal touch handling exits and the browser takes over native text-selection behavior |
| 32 | +- the overlay is rendered inside the fullscreen mobile terminal sheet |
| 33 | + |
| 34 | +This baseline caused a UX problem on mobile: |
| 35 | + |
| 36 | +- after entering copy mode, long-pressing and dragging overlay text can trigger browser-native selection and viewport/layer movement that feels like the whole DOM is being dragged |
| 37 | + |
| 38 | +## 3. User Decisions Captured |
| 39 | + |
| 40 | +Confirmed product decisions: |
| 41 | + |
| 42 | +- completely replace the current mobile copy-mode overlay |
| 43 | +- keep the new behavior gated by the existing `Copy on select` setting |
| 44 | +- copy the wrapped logical line, not a single visual row |
| 45 | +- merge wrapped rows into one logical line |
| 46 | +- preserve meaningful internal spaces |
| 47 | +- trim only trailing padding whitespace from the logical line ending |
| 48 | +- show success feedback as `copied current line` toast plus short vibration |
| 49 | +- use a simple phase-one targeting model: |
| 50 | + - start from `touchstart.target` |
| 51 | + - walk upward to find the nearest xterm row element |
| 52 | + - if no row element is found, do nothing |
| 53 | + - do not add coordinate-based fallback in this phase |
| 54 | + |
| 55 | +## 4. Requirements |
| 56 | + |
| 57 | +### 4.1 Functional rules |
| 58 | + |
| 59 | +- On mobile, `Copy on select = true` enables long-press copy. |
| 60 | +- On mobile, `Copy on select = false` keeps the current non-copy long-press behavior. |
| 61 | +- A stable long press on terminal output must copy the current wrapped logical line directly to the clipboard. |
| 62 | +- The copied line must come from the frontend xterm active buffer. |
| 63 | +- No server request is allowed for this copy path. |
| 64 | +- If the long-press target does not resolve to a rendered xterm row, nothing should be copied. |
| 65 | +- If the clipboard write fails, existing failure toast behavior must be reused. |
| 66 | +- Desktop behavior must remain unchanged. |
| 67 | + |
| 68 | +### 4.2 Non-functional rules |
| 69 | + |
| 70 | +- The new behavior must remove the mobile copy overlay entirely. |
| 71 | +- The implementation should stay small and phase-one scoped. |
| 72 | +- The targeting logic should avoid speculative heuristics for empty areas in this phase. |
| 73 | +- The line-resolution logic should be extracted from `xterm-host.tsx` into a focused helper. |
| 74 | + |
| 75 | +## 5. Why The Old Overlay Is Being Removed |
| 76 | + |
| 77 | +The old design intentionally opened browser-native text selection inside a fullscreen fixed mobile sheet: |
| 78 | + |
| 79 | +- `user-select: text` |
| 80 | +- `-webkit-touch-callout: default` |
| 81 | +- native scrolling inside the overlay |
| 82 | +- custom terminal touch handling disabled while copy mode is active |
| 83 | + |
| 84 | +That combination makes the mobile browser the primary gesture handler after copy mode opens. In practice, this can produce viewport or composited-layer movement that feels like dragging the whole page instead of just selecting text. |
| 85 | + |
| 86 | +The new design avoids that class of problem by eliminating the text-selection overlay entirely. |
| 87 | + |
| 88 | +## 6. High-Level Approach |
| 89 | + |
| 90 | +Recommended approach: |
| 91 | + |
| 92 | +- keep the existing long-press detection and movement tolerance in `XtermHost` |
| 93 | +- when the long press matures on mobile and `Copy on select` is enabled: |
| 94 | + - resolve the pressed xterm row from `touchstart.target` |
| 95 | + - convert that row to a visual row index within `.xterm-rows` |
| 96 | + - map the visual row index into `terminal.buffer.active` |
| 97 | + - walk upward and downward across wrapped rows to collect one logical line |
| 98 | + - copy the final logical line to the clipboard |
| 99 | + - show success feedback |
| 100 | +- remove all mobile copy-mode overlay state, rendering, styles, and tests |
| 101 | + |
| 102 | +## 7. Interaction Model |
| 103 | + |
| 104 | +### 7.1 Normal mobile terminal behavior |
| 105 | + |
| 106 | +When the feature setting is disabled: |
| 107 | + |
| 108 | +- long press does not copy |
| 109 | +- drag continues to behave as terminal scrolling |
| 110 | + |
| 111 | +When the feature setting is enabled: |
| 112 | + |
| 113 | +- a stable long press on rendered terminal text copies one logical line |
| 114 | +- if the gesture turns into a scroll before the long-press timer fires, copying is cancelled |
| 115 | + |
| 116 | +### 7.2 Long-press trigger |
| 117 | + |
| 118 | +The current long-press model already has the right shape and should remain: |
| 119 | + |
| 120 | +- single-touch only |
| 121 | +- movement over the tolerance cancels the long press |
| 122 | +- scroll gestures win over copy |
| 123 | +- long-press maturity is still driven by the existing timer |
| 124 | + |
| 125 | +The copied target is locked to the original `touchstart.target`. |
| 126 | + |
| 127 | +### 7.3 Success and failure feedback |
| 128 | + |
| 129 | +On success: |
| 130 | + |
| 131 | +- push a lightweight success toast: `Copied current line` |
| 132 | +- vibrate briefly if `navigator.vibrate` is available |
| 133 | + |
| 134 | +On failure: |
| 135 | + |
| 136 | +- reuse the existing copy failure toast path |
| 137 | +- do not open any fallback overlay |
| 138 | + |
| 139 | +## 8. Target Resolution Rules |
| 140 | + |
| 141 | +Phase one deliberately uses a minimal targeting rule. |
| 142 | + |
| 143 | +### 8.1 Source of truth for hit testing |
| 144 | + |
| 145 | +Use `touchstart.target` captured when the long-press gesture begins. |
| 146 | + |
| 147 | +Do not use the release target. |
| 148 | +Do not use a `clientY` fallback in this phase. |
| 149 | + |
| 150 | +### 8.2 Row resolution |
| 151 | + |
| 152 | +Resolve the nearest xterm row by walking upward from `touchstart.target`. |
| 153 | + |
| 154 | +Accepted row shape: |
| 155 | + |
| 156 | +- an element that is a direct child of `.xterm-rows` |
| 157 | + |
| 158 | +If upward traversal finds no such row: |
| 159 | + |
| 160 | +- treat the gesture as non-copyable |
| 161 | +- do nothing |
| 162 | + |
| 163 | +This intentionally excludes: |
| 164 | + |
| 165 | +- empty space outside rendered rows |
| 166 | +- unrelated xterm layers |
| 167 | +- ambiguous targets |
| 168 | + |
| 169 | +Those cases are accepted phase-one limitations. |
| 170 | + |
| 171 | +### 8.3 Visual row index |
| 172 | + |
| 173 | +Once the row element is found: |
| 174 | + |
| 175 | +- use its position within `.xterm-rows.children` |
| 176 | +- that zero-based child position is the visual row index |
| 177 | + |
| 178 | +If the resolved row is missing from the current children list: |
| 179 | + |
| 180 | +- do nothing |
| 181 | + |
| 182 | +## 9. Buffer Mapping Rules |
| 183 | + |
| 184 | +The copied text comes from the frontend xterm buffer, not the DOM text. |
| 185 | + |
| 186 | +### 9.1 Buffer source |
| 187 | + |
| 188 | +Use: |
| 189 | + |
| 190 | +- `terminal.buffer.active.viewportY` |
| 191 | +- `terminal.buffer.active.getLine(y)` |
| 192 | +- `IBufferLine.isWrapped` |
| 193 | +- `IBufferLine.translateToString(...)` |
| 194 | + |
| 195 | +This is the active frontend xterm buffer only. |
| 196 | +No backend lookup is needed. |
| 197 | + |
| 198 | +### 9.2 Mapping visual row to buffer row |
| 199 | + |
| 200 | +Compute: |
| 201 | + |
| 202 | +- `bufferRow = terminal.buffer.active.viewportY + visualRowIndex` |
| 203 | + |
| 204 | +If `getLine(bufferRow)` returns `undefined`: |
| 205 | + |
| 206 | +- do nothing |
| 207 | + |
| 208 | +### 9.3 Finding the logical line start |
| 209 | + |
| 210 | +The visual row may be the first segment of a logical line or a wrapped continuation. |
| 211 | + |
| 212 | +Starting from `bufferRow`: |
| 213 | + |
| 214 | +- walk upward while the current line exists and `currentLine.isWrapped === true` |
| 215 | +- the first row where `isWrapped !== true` is the logical line start |
| 216 | + |
| 217 | +### 9.4 Collecting wrapped continuation rows |
| 218 | + |
| 219 | +From the logical line start: |
| 220 | + |
| 221 | +- include that row |
| 222 | +- walk downward while the next row exists and `nextLine.isWrapped === true` |
| 223 | +- include each wrapped continuation row |
| 224 | + |
| 225 | +The resulting ordered row list is the wrapped logical line. |
| 226 | + |
| 227 | +## 10. Text Assembly Rules |
| 228 | + |
| 229 | +The copied text must preserve meaningful spaces while removing terminal padding noise at the end. |
| 230 | + |
| 231 | +### 10.1 Per-row conversion |
| 232 | + |
| 233 | +For a collected logical line with multiple buffer rows: |
| 234 | + |
| 235 | +- every row except the last uses `translateToString(false)` |
| 236 | +- the last row uses `translateToString(true)` |
| 237 | + |
| 238 | +Why: |
| 239 | + |
| 240 | +- wrapped intermediate rows may legitimately end in a space that is part of the logical line continuation |
| 241 | +- trimming every segment would risk deleting meaningful spaces in the middle of the reconstructed line |
| 242 | +- trimming only the last segment removes right-side terminal padding from the final logical line ending |
| 243 | + |
| 244 | +### 10.2 Final join behavior |
| 245 | + |
| 246 | +Join all collected row strings with no separator. |
| 247 | + |
| 248 | +This produces one single logical line string. |
| 249 | + |
| 250 | +### 10.3 Phase-one scope |
| 251 | + |
| 252 | +This feature copies one logical line only. |
| 253 | + |
| 254 | +It does not: |
| 255 | + |
| 256 | +- copy multiple logical lines |
| 257 | +- preserve per-cell styling |
| 258 | +- expose browser-native text selection |
| 259 | + |
| 260 | +## 11. File-Level Design |
| 261 | + |
| 262 | +### 11.1 `xterm-host.tsx` |
| 263 | + |
| 264 | +Keep ownership of: |
| 265 | + |
| 266 | +- touch gesture state |
| 267 | +- long-press timer |
| 268 | +- movement tolerance cancellation |
| 269 | +- clipboard side effects |
| 270 | +- toast side effects |
| 271 | +- success vibration |
| 272 | + |
| 273 | +Change behavior so that mobile long-press success runs direct logical-line copy instead of entering copy mode. |
| 274 | + |
| 275 | +Remove: |
| 276 | + |
| 277 | +- mobile copy-mode snapshot state |
| 278 | +- mobile copy-mode overlay rendering |
| 279 | +- mobile copy-mode background click handling |
| 280 | +- copy-mode entry failure path tied to snapshot generation |
| 281 | + |
| 282 | +### 11.2 New mobile helper |
| 283 | + |
| 284 | +Add a small helper under `packages/web/src/features/terminal-panel/mobile/` that owns: |
| 285 | + |
| 286 | +- resolving the nearest xterm row from a target node |
| 287 | +- computing the visual row index |
| 288 | +- mapping that index into `terminal.buffer.active` |
| 289 | +- expanding the wrapped logical line |
| 290 | +- building the final copy string |
| 291 | + |
| 292 | +Recommended shape: |
| 293 | + |
| 294 | +- accept the target node |
| 295 | +- accept the terminal instance |
| 296 | +- return `string | null` |
| 297 | + |
| 298 | +The helper should not touch clipboard APIs or UI toasts. |
| 299 | + |
| 300 | +### 11.3 Remove `copy-mode-snapshot.ts` |
| 301 | + |
| 302 | +The DOM snapshot builder is specific to the removed overlay workflow. |
| 303 | + |
| 304 | +Once the overlay is removed: |
| 305 | + |
| 306 | +- delete `packages/web/src/features/terminal-panel/mobile/copy-mode-snapshot.ts` |
| 307 | +- delete or replace tests that only exist for overlay DOM snapshot behavior |
| 308 | + |
| 309 | +### 11.4 Styles and locales |
| 310 | + |
| 311 | +Remove overlay-only CSS and copy-mode-only locale strings. |
| 312 | + |
| 313 | +Add new success-toast locale strings for: |
| 314 | + |
| 315 | +- English |
| 316 | +- Chinese |
| 317 | + |
| 318 | +The `Copy on select` setting stays in place but mobile semantics change. |
| 319 | + |
| 320 | +## 12. Testing Strategy |
| 321 | + |
| 322 | +### 12.1 Helper unit tests |
| 323 | + |
| 324 | +Add focused tests for: |
| 325 | + |
| 326 | +- resolves the nearest row from a nested span target |
| 327 | +- returns `null` when no xterm row is found |
| 328 | +- maps visual row index through `viewportY` |
| 329 | +- expands upward to the logical line start when the hit row is wrapped |
| 330 | +- expands downward through wrapped continuation rows |
| 331 | +- trims only the final segment right side |
| 332 | +- preserves meaningful internal spaces |
| 333 | + |
| 334 | +### 12.2 `xterm-host` behavior tests |
| 335 | + |
| 336 | +Rewrite mobile-copy tests around the new flow: |
| 337 | + |
| 338 | +- long press copies the current logical line when the setting is enabled |
| 339 | +- wrapped logical line copies as one merged string |
| 340 | +- long press does not copy when the gesture becomes a scroll |
| 341 | +- long press does not copy when horizontal drift exceeds tolerance |
| 342 | +- long press does not copy when the setting is disabled |
| 343 | +- long press does not scroll the live terminal while the copy action is executing |
| 344 | +- clipboard failure still shows the failure toast |
| 345 | +- success path triggers toast and vibration |
| 346 | + |
| 347 | +Remove tests that only verify overlay behaviors, such as: |
| 348 | + |
| 349 | +- rendering `Copy Mode` |
| 350 | +- exiting via Done |
| 351 | +- background click dismissal |
| 352 | +- overlay content DOM sizing |
| 353 | +- overlay text tap selection guards |
| 354 | + |
| 355 | +### 12.3 E2E regression coverage |
| 356 | + |
| 357 | +Replace the existing mobile copy-mode e2e assertions with: |
| 358 | + |
| 359 | +- enable `Copy on select` |
| 360 | +- seed a wrapped line in the mobile terminal |
| 361 | +- long press rendered text |
| 362 | +- verify clipboard content matches the merged logical line |
| 363 | +- verify success feedback appears |
| 364 | +- verify no copy-mode overlay is shown |
| 365 | + |
| 366 | +Keep the disabled-setting e2e: |
| 367 | + |
| 368 | +- when `Copy on select` is off, long press should not copy |
| 369 | + |
| 370 | +## 13. Accepted Phase-One Limitations |
| 371 | + |
| 372 | +The following are intentional and should not be treated as bugs in this phase: |
| 373 | + |
| 374 | +- pressing empty terminal space does nothing |
| 375 | +- pressing ambiguous non-row xterm layers does nothing |
| 376 | +- no coordinate fallback when target traversal cannot resolve a row |
| 377 | +- only one logical line is copied per long press |
| 378 | +- no native selection UI |
| 379 | +- no recovery path to copy from older scrollback once it has fallen out of frontend xterm buffer retention |
| 380 | + |
| 381 | +If real-world usage shows the target-only resolution is too strict, a future iteration may add a `clientY` fallback. That is explicitly out of scope for this phase. |
0 commit comments