Terminal PR sidebar: Status replaces Review summary, Mergeability hidden (Forgejo #602) #609

Merged

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

2026-09-15 22:31:06 +00:00

crueber commented

2026-09-15 22:27:07 +00:00

Owner

Pure UI branching in the Pull.jsx details aside (one divide-y card, same p-3/micro-label slot shape), per #602 acceptance:

Terminal = pr().merged (first) or closed state; new headless isTerminalPull(thread, pr) + terminalMergeDetail(pr, events) in pull-state.js.
Terminal Status REPLACES the Review summary slot: Merged via chip-merged + SHA/strategy/by detail (SHA links to the commit page, the #595 idiom), plain-closed via chip-closed.
Whole Mergeability div hidden on terminal PRs; Reviewers/Checks/Merge keep rendering. Open PRs byte-identical.
No new wire fields, no API change (payload already in hand); Tailwind-only, existing chips, no new deps (law 1); FIXED amendment in docs/go/12_web_ui.md same commit (law 12).

Verify: terminal-sidebar-602.test.js 13/13; related 531/594/599/588/592/595/517/521 green; full-minus-smoke 1559/1559; vite build + esbuild green; go vet clean. #568 census 25→26 (prose only, shared rule covers the new site).

Pure UI branching in the Pull.jsx details aside (one divide-y card, same p-3/micro-label slot shape), per #602 acceptance: - Terminal = pr().merged (first) or closed state; new headless isTerminalPull(thread, pr) + terminalMergeDetail(pr, events) in pull-state.js. - Terminal Status REPLACES the Review summary slot: Merged via chip-merged + SHA/strategy/by detail (SHA links to the commit page, the #595 idiom), plain-closed via chip-closed. - Whole Mergeability div hidden on terminal PRs; Reviewers/Checks/Merge keep rendering. Open PRs byte-identical. - No new wire fields, no API change (payload already in hand); Tailwind-only, existing chips, no new deps (law 1); FIXED amendment in docs/go/12_web_ui.md same commit (law 12). Verify: terminal-sidebar-602.test.js 13/13; related 531/594/599/588/592/595/517/521 green; full-minus-smoke 1559/1559; vite build + esbuild green; go vet clean. #568 census 25→26 (prose only, shared rule covers the new site).

crueber added 1 commit

2026-09-15 22:27:07 +00:00

web: terminal PR sidebar shows Status, hides Mergeability (Forgejo #602 ) f89f3a7e04

docs/go/12_web_ui.md §Decisions: FIXED (Forgejo #602) amendment in same
commit (law 12). Pure UI branching in the Pull.jsx details aside, same
p-3/micro-label slot shape; isTerminalPull + terminalMergeDetail in
pull-state.js (no new deps, law 1); #568 census 25→26 (prose only). No
wire/API change.

crueber referenced this pull request from a commit

2026-09-15 22:30:41 +00:00

Review #609: correct isTerminalPull state-source comment (thread is payload truth; PRDoc has no state)

crueber added 1 commit

2026-09-15 22:30:41 +00:00

Review #609 : correct isTerminalPull state-source comment (thread is payload truth; PRDoc has no state) ad4baba66e

crueber commented

2026-09-15 22:30:49 +00:00

Author

Owner

Independent review — APPROVED (with one comment-only fix, pushed as `ad4baba`)

Reviewed diff origin/main..origin/fix/issue-602 (f89f3a7, 6 files +265/-13) against #602 acceptance. All criteria met; no wire/API change; no new deps (law 1); law-12 amendment present in 12_web_ui.md.

Acceptance

Merged → Status slot (chip-merged 'Merged' + SHA/strategy/by detail), no Review summary, no Mergeability. Closed → chip-closed 'Closed', both hidden. Open → summary fallback + Mergeability untouched (diff confirms inner lines byte-identical, only Show wrappers added). Reviewers/Checks/Merge outside the gate, still render on terminal. Verified in source, not just claimed.
Tests: new terminal-sidebar-602.test.js (13 tests) + link-sweep-568 census 25→26 pass; full unit suite 1561/1561 green (smoke.test.js excluded — needs a live server, fails identically without one). New tests fail pre-fix (isTerminalPull/terminalMergeDetail absent on main).

Checklist findings

isTerminalPull: merged-wins correct; loading→open (fail-open to full UI) correct per #594 precedent. Note: PRDoc carries no state field (model.go PRDoc vs Thread), so pr?.state is always undefined in the page payload and the thread decides in practice — the fallback is safe but the old comment misdescribed it. Fixed in ad4baba (lib comment + doc prose, comment-only).
terminalMergeDetail: sidecar → merged-event → ''' priority correct; ?? handles null sidecars; caller gates detail on sha and strategy/by individually — missing event/SHA/strategy renders chip-alone, no 'undefined'.
SHA link reuses the exact #595 idiom (A.link.font-mono, full SHA href /{full}/commit/sha, 12-char text) and improves on it (sha-gated, #595 renders an empty link when sha missing).
Status section matches the #531 slot idiom (p-3, same micro-label classes, no new chrome/CSS — ui.css change is comment census only).
Mergeability gate wraps the whole div incl. base/head SHAs + commits/files links, per ticket. No other dependents: no getElementById/querySelector targets those nodes; pr-structure-531 + pull-event-text-521 suites still green.
Census 25→26 justified: exactly one real new class=link site (Pull.jsx 5→6, verified by grep); ui.css rule body untouched.

Fix commits: ad4baba (review-nit, comment-only, tests re-run green). No behavior change since f89f3a7.

## Independent review — APPROVED (with one comment-only fix, pushed as ad4baba) Reviewed diff origin/main..origin/fix/issue-602 (f89f3a7, 6 files +265/-13) against #602 acceptance. All criteria met; no wire/API change; no new deps (law 1); law-12 amendment present in 12_web_ui.md. **Acceptance** - Merged → Status slot (chip-merged 'Merged' + SHA/strategy/by detail), no Review summary, no Mergeability. Closed → chip-closed 'Closed', both hidden. Open → summary fallback + Mergeability untouched (diff confirms inner lines byte-identical, only Show wrappers added). Reviewers/Checks/Merge outside the gate, still render on terminal. Verified in source, not just claimed. - Tests: new terminal-sidebar-602.test.js (13 tests) + link-sweep-568 census 25→26 pass; full unit suite 1561/1561 green (smoke.test.js excluded — needs a live server, fails identically without one). New tests fail pre-fix (isTerminalPull/terminalMergeDetail absent on main). **Checklist findings** - isTerminalPull: merged-wins correct; loading→open (fail-open to full UI) correct per #594 precedent. Note: PRDoc carries no state field (model.go PRDoc vs Thread), so pr?.state is always undefined in the page payload and the thread decides in practice — the fallback is safe but the old comment misdescribed it. Fixed in ad4baba (lib comment + doc prose, comment-only). - terminalMergeDetail: sidecar → merged-event → ''' priority correct; ?? handles null sidecars; caller gates detail on sha and strategy/by individually — missing event/SHA/strategy renders chip-alone, no 'undefined'. - SHA link reuses the exact #595 idiom (A.link.font-mono, full SHA href /{full}/commit/sha, 12-char text) and improves on it (sha-gated, #595 renders an empty link when sha missing). - Status section matches the #531 slot idiom (p-3, same micro-label classes, no new chrome/CSS — ui.css change is comment census only). - Mergeability gate wraps the whole div incl. base/head SHAs + commits/files links, per ticket. No other dependents: no getElementById/querySelector targets those nodes; pr-structure-531 + pull-event-text-521 suites still green. - Census 25→26 justified: exactly one real new class=link site (Pull.jsx 5→6, verified by grep); ui.css rule body untouched. **Fix commits:** ad4baba (review-nit, comment-only, tests re-run green). No behavior change since f89f3a7.

crueber merged commit 3ff863dc95 into main

2026-09-15 22:31:06 +00:00

crueber referenced this pull request

2026-09-15 22:31:07 +00:00

Terminal PRs (merged/closed) show a merged/closed status in the sidebar instead of the Review Summary, and hide the moot Mergeability section #602

crueber referenced this pull request from a commit

2026-09-15 22:31:08 +00:00

Merge pull request 'Terminal PR sidebar: Status replaces Review summary, Mergeability hidden (Forgejo #602)' (#609) from fix/issue-602 into main