Mergeability wording: human phrases with green/soft-red tones (Forgejo #588) #591

Merged

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

2026-09-15 19:20:22 +00:00

crueber commented

2026-09-15 19:17:04 +00:00

Owner

Decisions: zero-checks reads No checks required in NEUTRAL muted, rewriting only the pending/ready headline; missing reads Unknown, unrecognized passes through (#561 precedent); tooltip keeps machine wording.

Verification: new mergeability-display-588.test.js (11 tests) + related (561/pull-state/checks/531/547) 56/56 green; full-minus-smoke 1481/1481 green (smoke needs live server, pre-existing); vite build + esbuild green with phrase in bundle; web/dist/.keep restored; go vet clean. 390px holds by construction (same value-line paragraphs, text-only swap).

Display-only fix following the #561 pattern: one shared mergeabilityDisplay(state, detail) in web/src/lib/pull-state.js maps every mergeState() return and every mergeable.state wire value to a phrase + tone (Able to be Merged emerald; Draft pull request / Checks failing / Changes requested / Merge conflicts / Merge failed soft red; Checking mergeability / Merging… / Already merged muted zinc), applied at the MergeBox state line and the sidebar mergeability value (mergeableText deleted). Wire/internal values byte-identical (mergeState, comparisons, enabled() gate, tooltip, amber reasons line untouched). Decisions: zero-checks reads No checks required in NEUTRAL muted, rewriting only the pending/ready headline; missing reads Unknown, unrecognized passes through (#561 precedent); tooltip keeps machine wording. Verification: new mergeability-display-588.test.js (11 tests) + related (561/pull-state/checks/531/547) 56/56 green; full-minus-smoke 1481/1481 green (smoke needs live server, pre-existing); vite build + esbuild green with phrase in bundle; web/dist/.keep restored; go vet clean. 390px holds by construction (same value-line paragraphs, text-only swap).

crueber added 1 commit

2026-09-15 19:17:05 +00:00

web: mergeability renders human phrases with green/soft-red tones (Forgejo #588 ) 9357cadc45

Shared mergeabilityDisplay(state, detail) in web/src/lib/pull-state.js
maps every mergeState() return and every mergeable.state wire value to
a phrase + tone (Able to be Merged emerald; Draft/Checks failing/
Changes requested/Merge conflicts/Merge failed soft red; pending and
terminal muted), applied at MergeBox and the sidebar value line.
Wire/internal values byte-identical. Docs: 12_web_ui.md amendment.

crueber commented

2026-09-15 19:20:07 +00:00

Author

Owner

Independent review — APPROVED (no fix commit; tree left clean)

Reviewed diff origin/main..origin/fix/issue-588 (9357cad) against #588, enumerated both state sets myself.

Coverage — no gap. mergeState() returns exactly {merged, failed, merging, draft, blocked, mergeable, ready} (MergeBox.jsx:36-44); wire mergeable.state is exactly {clean, dirty, behind, up_to_date, unknown} (internal/pulls/model.go:70-76). Helper maps: merged/up_to_date→Already merged; failed→Merge failed; merging→Merging…; draft or d.draft→Draft pull request; checksBlockers>0→Checks failing; CHANGES_REQUESTED→Changes requested; dirty state-or-wire→Merge conflicts; clean/mergeable/behind→Able to be Merged (+behind sub-line); ready/checking/unknown→Checking mergeability or No checks required (zeroChecks); null/undefined/''→Unknown; else passthrough. Blocked priority (draft→checks→reviews→conflicts) mirrors mergeState order; behind+blocker correctly yields the blocker (blocker arms precede the Able arm).

Acceptance, point by point:

One shared helper {text, cls, sub}, no per-callsite ternaries (disp() in MergeBox, mergeabilityView in sidebar; the two Shows are presence guards, not phrase forks). PASS
MergeBox line green Able to be Merged / soft-red blocked phrases, never raw words; raw {state()} render gone. PASS
Sidebar same helper, same phrase+color; mergeableText deleted from src (only test-title/assertion strings mention it). PASS
All five blocked conditions + zero-checks decided (zeroChecks=neutral muted, pending/ready-headline only; clean/behind+zeroChecks still Able — coherent with requiredCheckBlockers([])===[] in checks-empty.js, so red would falsely signal blocked). PASS
Wire/internal untouched: mergeState body, blockers(), enabled(), tooltip, amber reasons line, update-branch, task line all byte-identical (diff adds only import+disp+render swap). PASS
Helper unit-tested (11 tests: every value + priority + unknown/missing). PASS
Headless DOM assertions on colors at both call sites (cls===GREEN/RED pins + render-interpolation pins). PASS
Law 12 amendment present in docs/go/12_web_ui.md; law 1 clean (6 files only, no package.json/ui.css/dist changes; no new deps/CSS). PASS
Tests fail pre-fix: mergeabilityDisplay absent on main (new file import-errors), mergeableText present on main (new sidebar pin fails), raw {state()} present on main (raw-word pin fails). PASS

Verified by execution: new file 11/11 green; related suites 64/64 green; full unit run 1483/1484 (sole failure smoke.test.js live-server test, pre-existing, unrelated); vite build green; web/dist/.keep restored after build (tree clean apart from pre-existing untracked web/node_modules).

Notes (non-blocking): (1) pr-structure-531.test.js:65 test title still says 'mergeableText line' — stale wording in a title only, assertion is updated; suggest a rename on a future touch. (2) Transient only: mergeable-null + zeroChecks-true reads Unknown in the sidebar vs No checks required in MergeBox (MergeBox passes machine 'ready', sidebar passes wire undefined); both muted neutral, window requires checks-loaded-but-mergeable-missing race. Documented here; no change — altering it would complicate the reviewed missing→Unknown decision.

## Independent review — APPROVED (no fix commit; tree left clean) Reviewed diff origin/main..origin/fix/issue-588 (9357cad) against #588, enumerated both state sets myself. **Coverage — no gap.** mergeState() returns exactly {merged, failed, merging, draft, blocked, mergeable, ready} (MergeBox.jsx:36-44); wire mergeable.state is exactly {clean, dirty, behind, up_to_date, unknown} (internal/pulls/model.go:70-76). Helper maps: merged/up_to_date→Already merged; failed→Merge failed; merging→Merging…; draft or d.draft→Draft pull request; checksBlockers>0→Checks failing; CHANGES_REQUESTED→Changes requested; dirty state-or-wire→Merge conflicts; clean/mergeable/behind→Able to be Merged (+behind sub-line); ready/checking/unknown→Checking mergeability or No checks required (zeroChecks); null/undefined/''→Unknown; else passthrough. Blocked priority (draft→checks→reviews→conflicts) mirrors mergeState order; behind+blocker correctly yields the blocker (blocker arms precede the Able arm). **Acceptance, point by point:** - One shared helper {text, cls, sub}, no per-callsite ternaries (disp() in MergeBox, mergeabilityView in sidebar; the two Shows are presence guards, not phrase forks). PASS - MergeBox line green Able to be Merged / soft-red blocked phrases, never raw words; raw {state()} render gone. PASS - Sidebar same helper, same phrase+color; mergeableText deleted from src (only test-title/assertion strings mention it). PASS - All five blocked conditions + zero-checks decided (zeroChecks=neutral muted, pending/ready-headline only; clean/behind+zeroChecks still Able — coherent with requiredCheckBlockers([])===[] in checks-empty.js, so red would falsely signal blocked). PASS - Wire/internal untouched: mergeState body, blockers(), enabled(), tooltip, amber reasons line, update-branch, task line all byte-identical (diff adds only import+disp+render swap). PASS - Helper unit-tested (11 tests: every value + priority + unknown/missing). PASS - Headless DOM assertions on colors at both call sites (cls===GREEN/RED pins + render-interpolation pins). PASS - Law 12 amendment present in docs/go/12_web_ui.md; law 1 clean (6 files only, no package.json/ui.css/dist changes; no new deps/CSS). PASS - Tests fail pre-fix: mergeabilityDisplay absent on main (new file import-errors), mergeableText present on main (new sidebar pin fails), raw {state()} present on main (raw-word pin fails). PASS **Verified by execution:** new file 11/11 green; related suites 64/64 green; full unit run 1483/1484 (sole failure smoke.test.js live-server test, pre-existing, unrelated); vite build green; web/dist/.keep restored after build (tree clean apart from pre-existing untracked web/node_modules). **Notes (non-blocking):** (1) pr-structure-531.test.js:65 test title still says 'mergeableText line' — stale wording in a title only, assertion is updated; suggest a rename on a future touch. (2) Transient only: mergeable-null + zeroChecks-true reads Unknown in the sidebar vs No checks required in MergeBox (MergeBox passes machine 'ready', sidebar passes wire undefined); both muted neutral, window requires checks-loaded-but-mergeable-missing race. Documented here; no change — altering it would complicate the reviewed missing→Unknown decision.

crueber merged commit 65082f82a5 into main

2026-09-15 19:20:22 +00:00

crueber referenced this pull request from a commit

2026-09-15 19:20:22 +00:00

Merge pull request 'Mergeability wording: human phrases with green/soft-red tones (Forgejo #588)' (#591) from fix/issue-588 into main

crueber referenced this pull request

2026-09-15 19:20:23 +00:00

Mergeability wording: "Able to be Merged" in green; simplified soft-red phrases for every blocked state #588