From e8f66c644697b2f23e8c1f4d6e0f8015bcaf59e3 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Tue, 27 Oct 2026 01:21:51 +0100 Subject: [PATCH] fix: apply the review items for #711 at merge - terminal-ui.js _applyKeyboardClip(): read container.clientHeight only while rows are held, so the release path and the setting-off path skip the forced layout on every render and cursor move - constants.js keyboardClipPx JSDoc: drop the stray "ponytail:" marker, the note reads as plain text - src/web/CLAUDE.md "Per-device vs synced settings": list keyboardHoldsRows among the per-device keys SettingsUpdateSchema does not declare Co-Authored-By: Claude Opus 5.5 --- src/web/CLAUDE.md | 2 +- src/web/public/constants.js | 4 ++-- src/web/public/terminal-ui.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/web/CLAUDE.md b/src/web/CLAUDE.md index 4a2bc7bb..d47b6cae 100644 --- a/src/web/CLAUDE.md +++ b/src/web/CLAUDE.md @@ -52,7 +52,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L **Command palette + shortcut registry**: `Ctrl/Cmd/Alt+K` opens the session palette; shortcuts live in a rebindable registry (`DEFAULT_SHORTCUTS`/`getShortcutRegistry()`/`matchesShortcutEvent()` in app.js, overrides in `settings.shortcutOverrides`). ⚠️ Palette-chord keys must ALSO be swallowed in `attachCustomKeyEventHandler` (terminal-ui.js) or xterm writes the control byte into the PTY. ⚠️ The global Escape handler (app.js) calls EVERY close method on every Escape, in the capture phase, so a close method that does more than hide (the palette and Session Manager restore focus) must return early when its overlay is not open. ⚠️ `saveAppSettings()` rebuilds settings from the DOM, so keys edited elsewhere (`shortcutOverrides`, `showTokenCount`, `showCost`) need explicit `_prev` carry-over. ⚠️ **Smart copy (`Ctrl+C`)**: with no selection it must `return true` without `preventDefault()` or the interrupt is lost; keep `copyTerminalSelection` out of `SHORTCUT_ACTIONS`. The gate tests the CLEANED selection (`CodemanCopySelection.clean`: trailing padding, plus a LEADING margin only up to the width the CLI declares in `capabilities.transcriptGutter`, never one derived from the pane); the strip is not idempotent, so clean once and pass the RAW selection on, and leave Alt+drag column selections untouched. → [architecture-invariants#command-palette-and-shortcut-registry](../../docs/architecture-invariants.md#command-palette-and-shortcut-registry) -**Per-device vs synced settings**: the `displayKeys` set in settings-ui.js is a **client-side merge policy**, not a wire filter. A display key seeds from the server only when localStorage has no value for it, which is what prevents one device overwriting another; `showPlanUsageLimits` is additionally `delete`d from the incoming payload outright. Separately, `SettingsUpdateSchema` is `.strict()` and simply **does not declare** `skin`, `showFileViewerButton`, `showCronButton`, `webglRendererEnabled`, `localEchoEnabled`, `cjkInputEnabled`, or `extendedKeyboardBar`, so sending one of those is a validation error. The rest (`showResponseViewer`, `showPlanUsageLimits`, `language`, and most `show*` keys) ARE in the schema and do persist server-side; they are per-device by client policy only. ⚠️ Adding a new per-device setting means deciding **both** questions: membership in `displayKeys`, and presence in the schema. +**Per-device vs synced settings**: the `displayKeys` set in settings-ui.js is a **client-side merge policy**, not a wire filter. A display key seeds from the server only when localStorage has no value for it, which is what prevents one device overwriting another; `showPlanUsageLimits` is additionally `delete`d from the incoming payload outright. Separately, `SettingsUpdateSchema` is `.strict()` and simply **does not declare** `skin`, `showFileViewerButton`, `showCronButton`, `webglRendererEnabled`, `localEchoEnabled`, `keyboardHoldsRows`, `cjkInputEnabled`, or `extendedKeyboardBar`, so sending one of those is a validation error. The rest (`showResponseViewer`, `showPlanUsageLimits`, `language`, and most `show*` keys) ARE in the schema and do persist server-side; they are per-device by client policy only. ⚠️ Adding a new per-device setting means deciding **both** questions: membership in `displayKeys`, and presence in the schema. **Settings surface** (`#appSettingsModal` + `#sessionOptionsModal` + `#createCaseModal`): one `set-*` language shared through a single `:is(...)` id scope in styles.css. App Settings' rail is a table of contents over ONE scrolling document (`switchSettingsTab` scrolls); Session Options and Add Case really switch (`switchOptionsTab` / `switchCaseModalTab`), and their larger per-modal size blocks are the design, not drift. ⚠️ **The load/save contract is `getElementById` by id**: renaming or dropping a control id silently stops it loading or saving. ⚠️ The Session Options "Session" entry still keys off `context` (label-only rename). ⚠️ Add Case keeps its legacy `.form-row` markup via an adapter; every `
` there needs `.set-adv-chev` plus both marker suppressions. ⚠️ Model cards and the effort segment are views over hidden `