Rework PR page right column into one sectioned panel and PR list into flat divider rows (issues-surface idioms) — supersedes #521's structure #531
Labels
No labels
actions
bug
cli
duplicate
enhancement
fork
forum
git storage
help wanted
insights
invalid
issues
moderation
oidc
ownership transfer
packages
pr/merge protection rules
projects
pull requests
question
releases
sponsorships
tags
webhooks
wiki
wontfix
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
crueber/walhub#531
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What's requested
Two structural reworks of the pull-request surfaces, both converging on idioms the issues surfaces already define. This supersedes the structure #521 introduced (its markdown/state/header work stands; only the right-column and list shapes are reworked).
1. PR page right column: ONE sectioned panel, not stacked sibling cards.
web/src/pages/Pull.jsxrenders the right column (landed for #521) as a narrow rail of separate cards (Pull.jsx:864<aside … class="grid content-start gap-3">containing ReviewSummaryBar, a Mergeability.card(870), ReviewersPanel, a Checks.card(887), and MergeBox — each its own boxed card with its owncard-header<h2>). The issues page solved exactly this shape in #107: one container, divided sections —web/src/pages/Issue.jsx:549<section class="card divide-y divide-zinc-200 text-sm dark:divide-zinc-800" aria-label="Issue metadata">with each section ap-3block whose uppercase micro-label sits above its value. Rework the PR sidebar to the same idiom: onecard divide-ypanel whose sections are Mergeability / Reviewers / Checks / Merge (Review summary composing in first, as #521 already does). Each section keeps the issues-sidebar section anatomy: tiny uppercase label header + value directly beneath, so a "none" reads as that section's value.Mergeability is a value, not a heading. Currently
Pull.jsx:871makesMergeabilityan<h2 class="card-header">with the status underneath as the content. In the sectioned panel it becomes a section's value: label row "Mergeability" (uppercase micro-label like Labels/Assignees/Milestone), and the value line carriesmergeableText(mergeable())(state-mapped wording fromPull.jsx:52) — keeping the existing sub-lines (base/head refs atPull.jsx:878–884, pending-branch warning at 873, commits/files links at 886) inside the same section, as secondary value detail rather than card body prose.2. PR list: flat hr-separated rows, not boxed cards.
web/src/pages/Pulls.jsx:116–133renders the list as<ul class="card-list">of<li class="card">— one bordered box per PR, each with its owncard-title/card-meta. The issues list is the established row idiom (web/src/pages/Issues.jsx:351–358, per #135): divider-separated rows, never boxed —border-t border-zinc-200 … first:border-t-0rows inside a single container, echoing the ThreadTimeline comment-entry dividers from #109. Rework the PR list to those flat rows: one container, per-PR row = title link, state chip, label/reviewed affordances inline, right-aligned meta (branch refs or updated time, consistent with the issues row's right meta). Row anatomy mirrorsIssues.jsx:359+(truncation safety:min-w-0/max-w-full truncatetitle, wrapping chips,ml-autoright meta).Evidence (static read of the current tree, at
0fdf7ab)Pull.jsx:863–925: aside =grid content-start gap-3of five sibling blocks; Mergeability card at 868–886 withh2.card-headerat 871; Checks card 887–907 with its owncard-header; ReviewSummaryBar and MergeBox are separate sibling cards.Issue.jsx:549–552and comment at 542–548: the #107 one-container/divided-sections idiom, including the "a none reads as that section's value" rationale — the reference implementation to copy.Pulls.jsx:116–119:card-listofli.card; contrastIssues.jsx:358li class="border-t border-zinc-200 py-3 first:border-t-0 …".Architecture notes
pr,getView().mergeable, checks, reviewers, summary). No ETag concern — no cached payload changes.MergeBoxandReviewSummaryBarkeep their components/logic; only their container/context changes (sections of the panel instead of sibling cards). Checks section keepsCheckPill,ContextRows,ZeroChecksBlock, required/blocking sub-lines.card-headerclass usage in Pull.jsx shrinks to nothing in the sidebar (or only where a section truly needs an action row, like the Issues sidebar's label/milestone picker rows that sit in the section header flex).chip-mergedvocabulary,Pulls.jsx:124interpolation) — the row rework reuses the same chip, not a new state treatment.Acceptance criteria
card divide-ysectioned panel in theIssue.jsx:549idiom; no stacked sibling.cardblocks remain in the aside.Mergeabilitycard-headerheading.border-t-separated rows inside one container; no per-PRli.cardboxes; truncation/wrapping safety matches the issues row.Fix ready for review: PR #542 (branch fix/issue-531) — one sectioned sidebar panel + flat PR-list rows, structure only, tests/build green.
Review: PR #542 (fix/issue-531 @
d880c65) — ready to mergeReviewed the full diff (7 files, +274/-100) against every #531 acceptance criterion plus the #521/#274/#319/#328/#530 non-regression surface. Verified in a scratch worktree (symlinked node_modules, since removed); main worktree left untouched (still clean on main).
Acceptance criteria — all met
section.card divide-y divide-zinc-200 text-sm dark:divide-zinc-800(aria-label="Pull request metadata", the Issue.jsx:549 idiom). Grep of the aside confirms zero.card/card-list/card-headerstrings remain in the sidebar; remaining Pull.jsx.card/card-headerhits (129-134 Reviews, 365 diff, 519-520 Finish review, 834/837 Files) are all conversation-column, as intended.{mergeableText(mergeable())}value line; no Mergeability heading. Sub-lines intact: base/head refs (890-893), #328 fork line (895-898), pending-branch warning (887-888), commits/files links (900-903).p-3with the uppercase micro-label, none-as-value fallbacks pinned (no reviews yet,none requested, ZeroChecksBlock, write-role gate note).<ul>, per-PRli.border-t … first:border-t-0 first:pt-0, byte-identical divider classes to Issues.jsx:368. Row anatomy mirrors Issues.jsx:369+: truncating title link (min-w-0 max-w-full truncate), #530pullListChipunchanged, refs inline,ml-auto shrink-0author · updated meta. Container parity holds (section[aria-label]+ bare<ul>on both pages)..card/card-headerand render as fragments/section values. Merge a11y kept:form[aria-label=Merge], merge-taskaria-live+aria-label, machine-state value line. Checks keeps CheckPill/ContextRows/ZeroChecksBlock + required/blocking lines with fetch key + combined call byte-identical.Non-regressions
.card-headertreatment pinned by the updated pull-event-text-521 test.checksBlockers,reviewDecision,role,canUpdateBranch),checks:key,pulls.list(query())identical — structural JSX/comment/test/doc change only..card-meta leaves the Pulls list, stays on the ReviewsList cardnote (Pull.jsx:135 is now the only.card-meta) (law 12).Verification (scratch worktree @
d880c65, removed afterward)node --test web/test/unit/*.test.js: 1241 total / 1240 pass / 1 fail — all 10 new pr-structure-531 tests green; the 1 failure is the live-server smoke subtest (/setup 403 vs 200), proven pre-existing by running smoke.test.js head-to-head on pristine origin/main (identical 403 failure).vite build: green (exit 0, 2.24s).p-3while siblings usegrid gap-1 p-3, and the aside keeps a single-childgrid … gap-3wrapper — both harmless.MERGE RECOMMENDATION: ready to merge (no fixes pushed — nothing blocking found; not merging per instructions).
Fixed by PR #542 (review clean — all 5 criteria pass, #521 intact, gates untouched), merged. Closing.