Reviews section and finish-review trigger do not match sibling containers (card padding, button treatment)

crueber commented

2026-09-15 02:12:56 +00:00

Owner

Follow-up to #545 (screenshot: PR conversation on mobile web, Reviews block vs comment-composer card vs Files card).

Evidence (current tree):

ReviewsList (web/src/pages/Pull.jsx:151) renders div.card with NO padding utility — sibling cards (comment composer form.card, Files card) carry card padding, so Reviews content touches the border edges and reads flat/uncomposed next to them.
The finish-review trigger (Pull.jsx:958) is a bare lowercase 'finish review' button, inconsistent with the btn/button idioms elsewhere on the page.
Empty state 'No reviews yet.' inherits the unpadded box.

Fix: compose the Reviews section and its trigger with the canonical card/padding/button idioms (docs/style-guideline.md); rendered-verified with screenshots at desktop + 390px before closing (AGENTS.md rule).

Follow-up to #545 (screenshot: PR conversation on mobile web, Reviews block vs comment-composer card vs Files card). Evidence (current tree): - ReviewsList (web/src/pages/Pull.jsx:151) renders div.card with NO padding utility — sibling cards (comment composer form.card, Files card) carry card padding, so Reviews content touches the border edges and reads flat/uncomposed next to them. - The finish-review trigger (Pull.jsx:958) is a bare lowercase 'finish review' button, inconsistent with the btn/button idioms elsewhere on the page. - Empty state 'No reviews yet.' inherits the unpadded box. Fix: compose the Reviews section and its trigger with the canonical card/padding/button idioms (docs/style-guideline.md); rendered-verified with screenshots at desktop + 390px before closing (AGENTS.md rule).

crueber referenced this issue from a commit

2026-09-15 02:38:42 +00:00

Fix #554: Reviews container styling (docs/go/12_web_ui.md amendment)

crueber referenced this issue from a pull request that will close it,

2026-09-15 02:38:44 +00:00

Fix #554: Reviews container styling #556

crueber commented

2026-09-15 02:38:53 +00:00

Author

Owner

Fix ready for review: #556 (#556) — styling/markup only, no merge. Headless: 1289 total / 1288 pass / 1 pre-existing live-server smoke failure (head-to-head clean); vite+esbuild green. No browser proof from this rig (shared daemon blocks loopback) — needs orchestrator screenshot verification at desktop + 390px pre-merge.

Fix ready for review: #556 (https://git.packden.us/crueber/walhub/pulls/556) — styling/markup only, no merge. Headless: 1289 total / 1288 pass / 1 pre-existing live-server smoke failure (head-to-head clean); vite+esbuild green. No browser proof from this rig (shared daemon blocks loopback) — needs orchestrator screenshot verification at desktop + 390px pre-merge.

crueber commented

2026-09-15 02:58:51 +00:00

Author

Owner

Review of PR #556 (fix/issue-554) — verified in scratch worktree at 013bb14.

BROWSER PROOF: per orchestrator instruction, rendered verification (1280px + 390px) was already done by the orchestrator with headless Chromium — I treat browser proof as CLOSED and evaluated code + tests only. (Note: the PR body and the 12_web_ui.md amendment text still say 'Browser proof open'; that line is superseded by the orchestrator verification, not a code defect.)

(1) Card padding composition — PASS. web/src/pages/Pull.jsx:154 ReviewsList panel is now card p-3, matching the CommentComposer sibling idiom (web/src/components/CommentComposer.jsx:232 form.card ... p-3). Base .card (ui.css) carries no padding, so the utility is load-bearing and correct. Flat divide-y list, card-header, card-meta rows unchanged; no card-in-card regression.

(2) Trigger treatment — PASS. Pull.jsx:960-962 wears btn px-3 py-1 (same sizing as its cancel sibling in the FinishReview modal, Pull.jsx:692) with proper Finish review casing and the staged-count suffix kept. Deliberately not primary — the modal submit (Pull.jsx:689) owns the one primary CTA per guideline §2. Correct call.

(3) Empty state — PASS. Pull.jsx:157 fallback No reviews yet. renders inside the padded panel; copy byte-identical, no separate change needed.

(4) Behavior identical — PASS. Dismiss flow/endpoint, reload, verdict mapping, renderBody, setFinishing staging, submit flow all intact (pinned by the no-behavior-change test).

(5) Scope/hygiene — PASS. 4 files only (Pull.jsx + 2 test files + 12_web_ui.md amendment); no backend/SDK/API change; no dep diff (package.json/go.mod untouched, law 1); ui.css untouched (composition only, no new classes); law-12 amendment present in the same change, guideline §2 by reference (law 11, composition-only so no guideline extension needed). Disclosed out-of-scope items (FinishReview form.card Pull.jsx:653, ThreadIndex nav.card Pull.jsx:564 stay unpadded, not re-legitimized) are honest scoping, not blockers — candidate follow-up.

Tests (scratch worktree, node_modules symlinked from main): full node --test web/test/unit/*.test.js → 1288 pass / 1 fail, the 1 failure being the live-server smoke subtest (smoke.test.js, 403 — needs a live Go server; environmental, matches the author's head-to-head claim on pristine main). Targeted reviews-card-554 + review-restyle-545 → 14/14 pass. vite build green (2.42s). No fixes pushed — none needed.

MERGE RECOMMENDATION: ready to merge (after orchestrator's screenshot verification is recorded on the issue).

Review of PR #556 (fix/issue-554) — verified in scratch worktree at 013bb14. BROWSER PROOF: per orchestrator instruction, rendered verification (1280px + 390px) was already done by the orchestrator with headless Chromium — I treat browser proof as CLOSED and evaluated code + tests only. (Note: the PR body and the 12_web_ui.md amendment text still say 'Browser proof open'; that line is superseded by the orchestrator verification, not a code defect.) (1) Card padding composition — PASS. web/src/pages/Pull.jsx:154 ReviewsList panel is now `card p-3`, matching the CommentComposer sibling idiom (web/src/components/CommentComposer.jsx:232 `form.card ... p-3`). Base `.card` (ui.css) carries no padding, so the utility is load-bearing and correct. Flat `divide-y` list, `card-header`, `card-meta` rows unchanged; no card-in-card regression. (2) Trigger treatment — PASS. Pull.jsx:960-962 wears `btn px-3 py-1` (same sizing as its cancel sibling in the FinishReview modal, Pull.jsx:692) with proper `Finish review` casing and the staged-count suffix kept. Deliberately not `primary` — the modal submit (Pull.jsx:689) owns the one primary CTA per guideline §2. Correct call. (3) Empty state — PASS. Pull.jsx:157 fallback `No reviews yet.` renders inside the padded panel; copy byte-identical, no separate change needed. (4) Behavior identical — PASS. Dismiss flow/endpoint, reload, verdict mapping, renderBody, setFinishing staging, submit flow all intact (pinned by the no-behavior-change test). (5) Scope/hygiene — PASS. 4 files only (Pull.jsx + 2 test files + 12_web_ui.md amendment); no backend/SDK/API change; no dep diff (package.json/go.mod untouched, law 1); ui.css untouched (composition only, no new classes); law-12 amendment present in the same change, guideline §2 by reference (law 11, composition-only so no guideline extension needed). Disclosed out-of-scope items (FinishReview form.card Pull.jsx:653, ThreadIndex nav.card Pull.jsx:564 stay unpadded, not re-legitimized) are honest scoping, not blockers — candidate follow-up. Tests (scratch worktree, node_modules symlinked from main): full `node --test web/test/unit/*.test.js` → 1288 pass / 1 fail, the 1 failure being the live-server smoke subtest (smoke.test.js, 403 — needs a live Go server; environmental, matches the author's head-to-head claim on pristine main). Targeted reviews-card-554 + review-restyle-545 → 14/14 pass. `vite build` green (2.42s). No fixes pushed — none needed. MERGE RECOMMENDATION: ready to merge (after orchestrator's screenshot verification is recorded on the issue).

crueber referenced this issue

2026-09-15 02:59:15 +00:00

FinishReview form + ThreadIndex nav lack card padding (unpadded siblings) #557

crueber closed this issue

2026-09-15 03:00:02 +00:00

crueber commented

2026-09-15 03:00:02 +00:00

Author

Owner

Fixed by PR #556 (review clean; rendered-verified with screenshots at desktop + 390px; unpadded-sibling follow-up filed as #557), merged. Closing.

crueber referenced this issue from a commit

2026-09-15 03:00:06 +00:00