mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-15 18:59:33 +01:00
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 <noreply@anthropic.com>
This commit is contained in:
1 parent
3e334357e8
commit
e8f66c6446
3 files changed
+4
-4
No files matched your search
+1
-1
@@ -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 `<details>` there needs `.set-adv-chev` plus both marker suppressions. ⚠️ Model cards and the effort segment are views over hidden `<select>`s, which stay the source of truth. ⚠️ `.modal-tabs*` classes are retired; `admin-ui.js` needs `.set-rail-items` + `.set-doc` to survive any restructure. Guard: `test/app-settings-structure.test.ts`. → [architecture-invariants#settings-surface-app-settings-session-options-add-case](../../docs/architecture-invariants.md#settings-surface-app-settings-session-options-add-case)
|
||||
|
||||
|
||||
@@ -3389,8 +3389,8 @@ const KEYBOARD_CLIP_CURSOR_REACH = 2;
|
||||
* - Reading history: the offset follows the scroll position, so the top of the
|
||||
* scrollback is reachable and scrolling stays continuous.
|
||||
*
|
||||
* ponytail: recomputed on every render with no hysteresis; add some if a TUI's
|
||||
* mid-redraw frames ever make the view jitter.
|
||||
* Recomputed on every render with no hysteresis; add some if a TUI's mid-redraw
|
||||
* frames ever make the view jitter.
|
||||
*
|
||||
* @param {{rows: number, cellHeight: number, viewHeight: number, cursorRow: number,
|
||||
* lastContentRow: number, following: boolean, viewportY: number, baseY: number}} input
|
||||
|
||||
@@ -6638,7 +6638,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
if (!container) return;
|
||||
const rows = this._keyboardHeldRowsActive() ? this.terminal.rows : 0;
|
||||
const cellHeight = rows ? this.terminal._core?._renderService?.dimensions?.css?.cell?.height : 0;
|
||||
const viewHeight = container.clientHeight;
|
||||
const viewHeight = rows ? container.clientHeight : 0;
|
||||
let clip = 0;
|
||||
if (cellHeight && viewHeight) {
|
||||
const buffer = this.terminal.buffer.active;
|
||||
|
||||
Reference in new issue
Block a user