Move inline-composer Cancel to the bottom action row (Fix #587) #590

Merged

crueber merged 2 commits from fix/issue-587 into main

2026-09-15 19:10:46 +00:00

crueber commented

2026-09-15 19:08:14 +00:00

Owner

Implements Forgejo #587: Cancel moves from the header row to the bottom action row (lower left, opposite submit) in both inline comment composers.

CommentComposer gains an optional onCancel/cancelLabel pair: Cancel renders at the LEFT of the bottom row (left slot = Cancel, right slot = existing close/submit cluster in its own inner flex); without onCancel the pre-#587 flat right-aligned row renders unchanged (shared actions() cluster, no per-page fork).
Pull.jsx (conversation draft) and PullFiles.jsx (Files-tab staged composer) drop their header-row buttons and pass the handler; header
keeps only the anchor label. Behavior unchanged: Cancel/Escape drops only the keyed draft, calls nothing, refocuses trigger.
Tests: new composer-cancel-row-587.test.js (14 tests); justified updates to three #566 pins + one #568 pin (old header-position assertions).
Docs: FIXED (Forgejo #587) amendment in docs/go/12_web_ui.md, same commit.

Verification: full-minus-smoke 1470/1470 green; vite build + esbuild SDK green; go vet clean. 390px holds by construction (both row levels flex-wrap).

Implements Forgejo #587: Cancel moves from the header row to the bottom action row (lower left, opposite submit) in both inline comment composers. - CommentComposer gains an optional onCancel/cancelLabel pair: Cancel renders at the LEFT of the bottom row (left slot = Cancel, right slot = existing close/submit cluster in its own inner flex); without onCancel the pre-#587 flat right-aligned row renders unchanged (shared actions() cluster, no per-page fork). - Pull.jsx (conversation draft) and PullFiles.jsx (Files-tab staged composer) drop their header-row buttons and pass the handler; header <p> keeps only the anchor label. Behavior unchanged: Cancel/Escape drops only the keyed draft, calls nothing, refocuses trigger. - Tests: new composer-cancel-row-587.test.js (14 tests); justified updates to three #566 pins + one #568 pin (old header-position assertions). - Docs: FIXED (Forgejo #587) amendment in docs/go/12_web_ui.md, same commit. Verification: full-minus-smoke 1470/1470 green; vite build + esbuild SDK green; go vet clean. 390px holds by construction (both row levels flex-wrap).

crueber added 1 commit

2026-09-15 19:08:15 +00:00

Move inline-composer Cancel to the bottom action row (Forgejo #587 ) 91118b50e6

CommentComposer takes an optional onCancel/cancelLabel pair rendering
Cancel at the LEFT of the bottom row (12_web_ui.md Decisions); both
inline call sites drop their header-row button and pass the handler.
Behavior unchanged: Cancel/Escape drops only the keyed draft, calls
nothing, refocuses the trigger.

crueber added 1 commit

2026-09-15 19:10:21 +00:00

Fix #587 amendment test count: 13 -> 14 (review pin) f93f9e04be

The new composer-cancel-row-587.test.js file holds 14 test() blocks;
the law-12 amendment line said 13. No code change.

crueber commented

2026-09-15 19:10:29 +00:00

Author

Owner

Independent review — APPROVED (one trivial fix pushed as f93f9e0).

Verified against #587 acceptance, origin/main..fix/issue-587:

Left-slot wiring: CommentComposer onCancel/cancelLabel renders Cancel first in a justify-between row with the close/submit cluster in its own inner justify-end flex (cluster never spreads); justify-between exists only in the onCancel branch. Fallback is the pre-#587 div (same class string) with flat {actions()} children — actions() is one shared definition used exactly twice with exactly one type=submit, so no fork; no-onCancel consumers (Issue.jsx: zero onCancel; Pull.jsx main composer: no onCancel, only the 1 inline-draft onCancel; PullFiles.jsx: exactly 1) render as before.
Handlers verbatim: onCancel={() => closeDraft(draftKey(hi(), ri()))} and onCancel={() => dismissStaged(false)} match the removed header-button onClicks character-for-character. Refocus parity exact: old header Cancel did NOT refocus on either surface (Pull: bare closeDraft, no refocusTrigger; Files: dismissStaged(false)); new Cancel adds no refocus. Escape untouched on both (Pull: closeDraft(key)+refocusTrigger(key); Files: dismissStaged(true)). No POST/onStage in the Cancel path (single synchronous props.onCancel() call, no busy guard).
Headers button-free: both header
keep only 'commenting on ' — pinned plus old-markup-gone pins. Call-site comments updated to describe the bottom-row slot (both files).
Pin updates justified, not weakened: 3x #566 pins (header-button markup -> onCancel wiring + header button-free + composer-invocation pins; no-onStage window check -> exact handler-expression pin, required because onCancel now sits adjacent to the onSubmit prop that legitimately mentions onStage) and 1x #568 pin (per-surface header .btn assertions -> single composer .btn definition + wiring pins, same button-not-link intent). All still assert behavior, not just presence.
Docs/lawfulness: FIXED (#587) amendment in docs/go/12_web_ui.md (law 12); no package.json/ui.css changes — 4 runtime deps intact, no cancel CSS rule; Tailwind + canonical .btn composition only (law 1); 390px by construction (both row levels flex-wrap).
Tests: new composer-cancel-row-587.test.js 14/14 green; full-minus-smoke 1470/1470 green in /tmp/walhub-587 (smoke excluded: stale-dist 403 on /setup in the worktree, unrelated to this web-only change). New tests fail pre-fix (verified by running the new file against an origin/main worktree — onCancel/doc pins fail).

Fix pushed: f93f9e0 corrects the amendment's '13 tests' to '14 tests' (file holds 14 test blocks). No other defects found.

Independent review — APPROVED (one trivial fix pushed as f93f9e0). Verified against #587 acceptance, origin/main..fix/issue-587: - Left-slot wiring: CommentComposer onCancel/cancelLabel renders Cancel first in a justify-between row with the close/submit cluster in its own inner justify-end flex (cluster never spreads); justify-between exists only in the onCancel branch. Fallback is the pre-#587 div (same class string) with flat {actions()} children — actions() is one shared definition used exactly twice with exactly one type=submit, so no fork; no-onCancel consumers (Issue.jsx: zero onCancel; Pull.jsx main composer: no onCancel, only the 1 inline-draft onCancel; PullFiles.jsx: exactly 1) render as before. - Handlers verbatim: onCancel={() => closeDraft(draftKey(hi(), ri()))} and onCancel={() => dismissStaged(false)} match the removed header-button onClicks character-for-character. Refocus parity exact: old header Cancel did NOT refocus on either surface (Pull: bare closeDraft, no refocusTrigger; Files: dismissStaged(false)); new Cancel adds no refocus. Escape untouched on both (Pull: closeDraft(key)+refocusTrigger(key); Files: dismissStaged(true)). No POST/onStage in the Cancel path (single synchronous props.onCancel() call, no busy guard). - Headers button-free: both header <p> keep only 'commenting on <anchor>' — pinned plus old-markup-gone pins. Call-site comments updated to describe the bottom-row slot (both files). - Pin updates justified, not weakened: 3x #566 pins (header-button markup -> onCancel wiring + header button-free + composer-invocation pins; no-onStage window check -> exact handler-expression pin, required because onCancel now sits adjacent to the onSubmit prop that legitimately mentions onStage) and 1x #568 pin (per-surface header .btn assertions -> single composer .btn definition + wiring pins, same button-not-link intent). All still assert behavior, not just presence. - Docs/lawfulness: FIXED (#587) amendment in docs/go/12_web_ui.md (law 12); no package.json/ui.css changes — 4 runtime deps intact, no cancel CSS rule; Tailwind + canonical .btn composition only (law 1); 390px by construction (both row levels flex-wrap). - Tests: new composer-cancel-row-587.test.js 14/14 green; full-minus-smoke 1470/1470 green in /tmp/walhub-587 (smoke excluded: stale-dist 403 on /setup in the worktree, unrelated to this web-only change). New tests fail pre-fix (verified by running the new file against an origin/main worktree — onCancel/doc pins fail). Fix pushed: f93f9e0 corrects the amendment's '13 tests' to '14 tests' (file holds 14 test blocks). No other defects found.

crueber merged commit bc3d9a2134 into main

2026-09-15 19:10:46 +00:00

crueber referenced this pull request from a commit

2026-09-15 19:10:48 +00:00

Merge pull request 'Move inline-composer Cancel to the bottom action row (Fix #587)' (#590) from fix/issue-587 into main

crueber referenced this pull request

2026-09-15 19:10:48 +00:00

Inline comment composer: move Cancel from header row to bottom action row (lower left, opposite Stage comment) #587