Public: anyone can read this; the crew can change it.
A conflicted pull's resolve page speaks the home page's language #9852
Check · Review · Land · Live done — Landed and live in production.
scout/forge:scout/9842-a-conflicted-pull-s-resolve-page-speaks → main 2 commits fork
4 files
changed +16
−26
docs/design/areas/pull.md +1 −1
| @@ -18,4 +18,4 @@ The pull page's cards (merge box, review, commits, slices, run-this-code) are `. | ||
| 18 | 18 | |
| 19 | 19 | ## The conflict resolver (#9713) |
| 20 | 20 | |
| 21 | `/:owner/:name/pulls/:number/resolve`: each conflict block, the crew proposal and the empty states are `.card.bare` (#9588), a heading over rows on the one background. The two sides lose their hairline box; the taps keep theirs (a control). The ours/theirs/both/Mine taps, the text box and `#crew-proposal` are unchanged. No new class; `test/conflicts.test.ts` holds it. | |
| 21 | `/:owner/:name/pulls/:number/resolve`: each conflict block, the crew proposal and the empty states are a `<section>` with an `<h2>` over rows (no `card`), on the one background. The two sides lose their hairline box; the taps keep theirs (a control). The ours/theirs/both/Mine taps, the text box and `#crew-proposal` are unchanged. No new class; `test/conflicts.test.ts` holds it. |
src/web/pages/conflicts.ts +9 −19
| @@ -33,7 +33,7 @@ import { field, readForm } from "../form.ts"; | ||
| 33 | 33 | import { html, type Html } from "../html.ts"; |
| 34 | 34 | import { flashTo, page } from "../layout.ts"; |
| 35 | 35 | import { renderMarkdown } from "../markdown.ts"; |
| 36 | import { btn, card } from "../ui.ts"; | |
| 36 | import { btn } from "../ui.ts"; | |
| 37 | 37 | import { changeHead, changeTabs, pullContext } from "../studio/change.ts"; |
| 38 | 38 | |
| 39 | 39 | /** More blocks or longer words than this are not a hand's work. */ |
| @@ -126,7 +126,7 @@ async function proposalCard( | ||
| 126 | 126 | </form>`); |
| 127 | 127 | if (parts.length === 0) return html``; |
| 128 | 128 | // A live region: the run's events re-read the card, so nobody reloads. |
| 129 | return html`<div id="crew-proposal" data-live="run.queued,run.started,run.finished,pull.*,comment.*">${card(html`${parts}`, { extra: "bare", head: html`<h2>The crew</h2>` })}</div>`; | |
| 129 | return html`<div id="crew-proposal" data-live="run.queued,run.started,run.finished,pull.*,comment.*"><section><h2>The crew</h2>${parts}</section></div>`; | |
| 130 | 130 | } |
| 131 | 131 | |
| 132 | 132 | const CSS = ` |
| @@ -173,13 +173,10 @@ export async function conflictsPage(ctx: Ctx): Promise<Response> { | ||
| 173 | 173 | palette: repoPalette(ctx, repo), |
| 174 | 174 | body: html`${repoHeader(ctx, repo, "pulls", { compact: true, ownTitle: true })} |
| 175 | 175 | ${changeHead(ctx, repo, pull)}${changeTabs(repo, pull, "files")} |
| 176 | ${card( | |
| 177 | html`<p>${unknown && pull.state === "open" | |
| 176 | <p class="mt">${unknown && pull.state === "open" | |
| 178 | 177 | ? "No conflict in that file." |
| 179 | 178 | : "This pull has no conflicts."} |
| 180 | <a href="${back}">Back to the files</a>.</p>`, | |
| 181 | { extra: "bare" }, | |
| 182 | )} | |
| 179 | <a href="${back}">Back to the files</a>.</p> | |
| 183 | 180 | ${crew}`, |
| 184 | 181 | }); |
| 185 | 182 | notePhase(req, "render", performance.now() - rendering); |
| @@ -206,8 +203,8 @@ export async function conflictsPage(ctx: Ctx): Promise<Response> { | ||
| 206 | 203 | ><input type="radio" name="b${i}" value="${kind}" ${pick === kind ? html`checked` : ""} />${label}</label |
| 207 | 204 | >`; |
| 208 | 205 | const own = choices[i]; |
| 209 | return card( | |
| 210 | html`<div class="rs-sides"> | |
| 206 | return html`<section class="rs-card"><h2>Block ${i + 1} of ${total}</h2> | |
| 207 | <div class="rs-sides"> | |
| 211 | 208 | <div class="rs-side"> |
| 212 | 209 | <span class="rs-label small mono">${pull.baseBranch}</span> |
| 213 | 210 | <pre>${block.ours}</pre> |
| @@ -224,12 +221,7 @@ export async function conflictsPage(ctx: Ctx): Promise<Response> { | ||
| 224 | 221 | ${tap("ours", `Keep ${pull.baseBranch}`)}${tap("theirs", `Keep ${pull.headBranch}`)}${tap("both", "Keep both")}${tap("text", "Write my own")} |
| 225 | 222 | </div> |
| 226 | 223 | <textarea class="rs-text" name="t${i}" rows="3" maxlength="${MAX_TEXT}" |
| 227 | aria-label="Your own words for block ${i + 1}">${own?.kind === "text" ? own.text : ""}</textarea>`, | |
| 228 | { | |
| 229 | extra: "bare rs-card", | |
| 230 | head: html`<h2>Block ${i + 1} of ${total}</h2>`, | |
| 231 | }, | |
| 232 | ); | |
| 224 | aria-label="Your own words for block ${i + 1}">${own?.kind === "text" ? own.text : ""}</textarea></section>`; | |
| 233 | 225 | }); |
| 234 | 226 | |
| 235 | 227 | const body = html`${repoHeader(ctx, repo, "pulls", { compact: true, ownTitle: true })} |
| @@ -250,10 +242,8 @@ export async function conflictsPage(ctx: Ctx): Promise<Response> { | ||
| 250 | 242 | : ""} |
| 251 | 243 | ${crew} |
| 252 | 244 | ${total === 0 |
| 253 | ? card( | |
| 254 | html`<p>This file has no text to choose between. It may be deleted on | |
| 255 | one side. Fix it in your own checkout.</p>`, | |
| 256 | ) | |
| 245 | ? html`<p class="mt">This file has no text to choose between. It may be deleted on | |
| 246 | one side. Fix it in your own checkout.</p>` | |
| 257 | 247 | : html`<form method="get" action="${here}"> |
| 258 | 248 | <input type="hidden" name="file" value="${file}" /> |
| 259 | 249 | <input type="hidden" name="head" value="${status.headOid}" /> |
test/conflicts.test.ts +4 −5
| @@ -166,8 +166,8 @@ describe("the Resolve page", () => { | ||
| 166 | 166 | |
| 167 | 167 | test("conflict blocks are headings over rows, not boxed cards", async () => { |
| 168 | 168 | const page = await (await fetch(`${base}/dev/app/pulls/1/resolve`, { headers })).text(); |
| 169 | expect(page).toMatch(/<div\s+class="card bare rs-card"/); | |
| 170 | expect(page).not.toMatch(/<div\s+class="card rs-card"/); | |
| 169 | expect(page).toContain('<section class="rs-card">'); | |
| 170 | expect(page).not.toContain('class="card'); | |
| 171 | 171 | expect(page).toContain("<h2>Block 1 of 2</h2>"); |
| 172 | 172 | expect(page).toContain('name="b0"'); |
| 173 | 173 | }); |
| @@ -191,10 +191,9 @@ describe("the Resolve page", () => { | ||
| 191 | 191 | expect(page).not.toContain(w); |
| 192 | 192 | }); |
| 193 | 193 | |
| 194 | test("the empty states and the crew card are bare, in plain words", async () => { | |
| 194 | test("the empty states are plain text, no card, in plain words", async () => { | |
| 195 | 195 | const none = await (await fetch(`${base}/dev/app/pulls/1/resolve?file=nope.txt`, { headers })).text(); |
| 196 | expect(none).toMatch(/<div\s+class="card bare"/); | |
| 197 | expect(none).not.toMatch(/<div\s+class="card"/); | |
| 196 | expect(none).not.toContain('class="card'); | |
| 198 | 197 | expect(none).toContain("No conflict in that file."); |
| 199 | 198 | expect(none).not.toContain("Nothing to resolve"); |
| 200 | 199 | }); |
test/crew-proposal.test.ts +2 −1
| @@ -179,7 +179,8 @@ echo '{"type":"result","subtype":"success","is_error":false,"result":"Wrote note | ||
| 179 | 179 | { headers: { authorization: `Bearer ${token}` } }, |
| 180 | 180 | ).then((r) => r.text()); |
| 181 | 181 | expect(page).toContain("Ask the crew to fix it"); |
| 182 | expect(page).toMatch(/id="crew-proposal"[^>]*>\s*<div\s+class="card bare"/); | |
| 182 | expect(page).toMatch(/id="crew-proposal" data-live="[^"]*"><section>\s*<h2>The crew<\/h2>/); | |
| 183 | expect(page).not.toContain('class="card'); | |
| 183 | 184 | expect(page).toContain("<h2>The crew</h2>"); |
| 184 | 185 | expect(page).toContain(`pulls/${pull.number}/resolve/propose`); |
| 185 | 186 | const stranger = await fetch( |