Fix #566: inline draft composers dismissable (visible Cancel + Escape) #569

Merged

crueber merged 1 commit from fix/issue-566 into main

2026-09-15 13:24:01 +00:00

crueber commented

2026-09-15 13:21:48 +00:00

Owner

Fixes #566. Follow-up sweep filed separately as #568 (not bundled).

Option (b): small secondary .btn Cancel on both composers (canonical idiom, both themes, no new ui.css rule) + panel-level Escape dismissal with trigger refocus (SplitCloseMenu convention; focus-never-click, no toggle-fight; no document listener).

Verification: new composer-dismiss-566.test.js 17/17; related 87/87; full-minus-smoke 1352/1352 (smoke needs live server, pre-existing); vite build + esbuild SDK green, web/dist/.keep restored; go vet clean. 390px reasoned (no layout change — same panel, button-sized Cancel). Docs: FIXED (Forgejo #566) amendment in docs/go/12_web_ui.md, same commit (law 12).

Fixes #566. Follow-up sweep filed separately as #568 (not bundled). Option (b): small secondary .btn Cancel on both composers (canonical idiom, both themes, no new ui.css rule) + panel-level Escape dismissal with trigger refocus (SplitCloseMenu convention; focus-never-click, no toggle-fight; no document listener). Verification: new composer-dismiss-566.test.js 17/17; related 87/87; full-minus-smoke 1352/1352 (smoke needs live server, pre-existing); vite build + esbuild SDK green, web/dist/.keep restored; go vet clean. 390px reasoned (no layout change — same panel, button-sized Cancel). Docs: FIXED (Forgejo #566) amendment in docs/go/12_web_ui.md, same commit (law 12).

crueber added 1 commit

2026-09-15 13:21:48 +00:00

FIXED (Forgejo #566 ) — inline draft composers dismissable via visible Cancel + Escape 9f22490ab4

Conversation DiffFile drafts (closeDraft) and Files-tab staged composer
(setStaged) each rendered Cancel as invisible text (.link has zero CSS
rules — preflight resets the button to plain muted text) and Escape did
nothing. Takes the issue's option (b): small secondary .btn treatment
(btn ml-2 px-2 py-0.5 text-xs, canonical idiom, both themes, no new
ui.css rule); panel-level onKeyDown dismisses on Escape and refocuses
the staging trigger (SplitCloseMenu convention — focus, never click, so
no toggle-fight; no document listener, no onCleanup). Dismissal drops
only that draft, calls nothing (no onStage, no POST), siblings and
getCreated untouched; re-stage mounts a fresh empty composer. The other
~10 .link uses stay as-is — systemic sweep is follow-up #568.

Headless cover: web/test/unit/composer-dismiss-566.test.js (17 tests).
Full-minus-smoke: 1352 pass. vite build + esbuild SDK green; go vet
clean. Docs: 12_web_ui.md amendment in this commit (law 12).

crueber commented

2026-09-15 13:23:47 +00:00

Author

Owner

APPROVE — independent review of fix/issue-566 (9f22490) against #566. All acceptance criteria hold; no fix commits needed (working tree clean on the branch).

What I verified:

Visible Cancel, both surfaces, both themes: both composers now use btn ml-2 px-2 py-0.5 text-xs (Pull.jsx draft composer, PullFiles.jsx staged composer). .btn is the canonical Controls idiom (guideline §2) and ships its own dark: variants (ui.css:78-80), so no per-call-site theme code and no new ui.css rule — option (b) as prescribed.
Escape, both surfaces: panel-level onKeyDown (Escape-only, preventDefault + stopPropagation) calling closeDraft(key) / dismissStaged(true). No document listener, so no onCleanup needed (pinned by test). Textarea Escape reaches the panel because onSubmitKeys returns early for non-Enter keys — verified in submitKeys.js. No toggle-fight: refocus is focus() only, never click(), on both surfaces.
Refocus targets: conversation uses a per-key triggerRefs Map fed by the gutter '+' ref under its own draft key; Files-tab queries [aria-label^="Comment on selected lines"], which matches the DiffBody trigger (DiffTable.jsx:329). Stale Map entries are focus-no-ops, as commented.
Isolation/semantics: closeDraft deletes exactly one Map key (no clear/wipe); dismissStaged touches only setStaged(null) — getCreated untouched (grep-confirmed), no onStage/replay, no POST/invalidate near any dismiss path. Re-stage remounts under the same Show gates with a fresh empty CommentComposer body; no draft-text stash.
Sweep correctly excluded: other .link uses intact (Pull.jsx 5+, PullFiles.jsx 2+), ui.css gains no .link rule, and #568 exists (open) covering the systemic sweep.
Lawfulness: package.json/go.mod untouched (still exactly the 4 allowed runtime deps); FIXED (#566) amendment in docs/go/12_web_ui.md in the same commit (law 12).
Tests: new composer-dismiss-566.test.js 17/17 green; related suites green; full-minus-smoke 1352/1352 green; go vet clean. Fail-pre-fix confirmed: origin/main has class="link ml-2" cancel (2x Pull.jsx, 1x PullFiles.jsx) and zero Dismissable markers, so the new pins fail there. The single smoke failure (smoke.test.js, 403 from whatever answers on :8080 here) is environmental and pre-disclosed — no walhub server running in this sandbox.

No defects found — nothing to fix, working tree untouched.

APPROVE — independent review of fix/issue-566 (9f22490) against #566. All acceptance criteria hold; no fix commits needed (working tree clean on the branch). What I verified: - Visible Cancel, both surfaces, both themes: both composers now use `btn ml-2 px-2 py-0.5 text-xs` (Pull.jsx draft composer, PullFiles.jsx staged composer). `.btn` is the canonical Controls idiom (guideline §2) and ships its own `dark:` variants (ui.css:78-80), so no per-call-site theme code and no new ui.css rule — option (b) as prescribed. - Escape, both surfaces: panel-level `onKeyDown` (Escape-only, preventDefault + stopPropagation) calling `closeDraft(key)` / `dismissStaged(true)`. No document listener, so no onCleanup needed (pinned by test). Textarea Escape reaches the panel because `onSubmitKeys` returns early for non-Enter keys — verified in submitKeys.js. No toggle-fight: refocus is `focus()` only, never `click()`, on both surfaces. - Refocus targets: conversation uses a per-key `triggerRefs` Map fed by the gutter '+' `ref` under its own draft key; Files-tab queries `[aria-label^="Comment on selected lines"]`, which matches the DiffBody trigger (DiffTable.jsx:329). Stale Map entries are focus-no-ops, as commented. - Isolation/semantics: `closeDraft` deletes exactly one Map key (no clear/wipe); `dismissStaged` touches only `setStaged(null)` — `getCreated` untouched (grep-confirmed), no `onStage`/replay, no POST/invalidate near any dismiss path. Re-stage remounts under the same Show gates with a fresh empty CommentComposer body; no draft-text stash. - Sweep correctly excluded: other `.link` uses intact (Pull.jsx 5+, PullFiles.jsx 2+), ui.css gains no `.link` rule, and #568 exists (open) covering the systemic sweep. - Lawfulness: package.json/go.mod untouched (still exactly the 4 allowed runtime deps); FIXED (#566) amendment in docs/go/12_web_ui.md in the same commit (law 12). - Tests: new composer-dismiss-566.test.js 17/17 green; related suites green; full-minus-smoke 1352/1352 green; `go vet` clean. Fail-pre-fix confirmed: origin/main has `class="link ml-2"` cancel (2x Pull.jsx, 1x PullFiles.jsx) and zero Dismissable markers, so the new pins fail there. The single smoke failure (smoke.test.js, 403 from whatever answers on :8080 here) is environmental and pre-disclosed — no walhub server running in this sandbox. No defects found — nothing to fix, working tree untouched.

crueber merged commit c2f23afe1a into main

2026-09-15 13:24:01 +00:00

crueber referenced this pull request

2026-09-15 13:24:02 +00:00

Inline unstaged diff comment composer cannot be dismissed: Cancel renders as invisible text, Escape does nothing #566

crueber referenced this pull request from a commit

2026-09-15 13:24:03 +00:00

Merge pull request 'Fix #566: inline draft composers dismissable (visible Cancel + Escape)' (#569) from fix/issue-566 into main