chadtmiller.com/knotless
git that lives in your Atmosphere account
1
10
Opening a pull or an issue chooses its reviewers and labels beside its buttons
docs/design/collaboration.md+2−1
| @@ -183,7 +183,8 @@ they have not reviewed it; once it is merged or closed, no longer. | ||
Both are fields of the pull record, so only its author sets them: when | ||
opening it (`knotless pull --draft --reviewer <handle>`) and with `knotless | ||
edit` (`--draft`, `--ready`, `--reviewer`, `--no-reviewer`). On the pages, | ||
the author asks the owner and collaborators to review it as it opens, and | ||
the author asks the owner and collaborators to review it as it opens, | ||
from Add beside the buttons that open it, and | ||
once it is open from its reviewers under the title (or Add, when it asks | ||
no one yet), where they also stop asking one who has not reviewed it. The | ||
author opens one as a draft with Open as draft, marks it | ||
docs/design/serve.md+5−4
| @@ -985,10 +985,11 @@ bring in, from the repository or a fork of it, shows the commits and diff | ||
it would bring (`getCompare`), with a button that unfolds the form to open | ||
it, its body started from the pull template at that base (`getTemplate`). | ||
Compare & pull links with `expand=1`, which shows the form at once. | ||
Beside the form, a sidebar | ||
chooses the labels to put on it, those the repository's viewer state says | ||
the account may (`getRepo`'s `viewer.labels`), and for a pull the | ||
reviewers to ask, from the owner and collaborators. Opening (`createIssue`, | ||
Beside the form's buttons, the | ||
facts line its page will show once open chooses the labels to put on it, | ||
those the repository's viewer state says the account may (`getRepo`'s | ||
`viewer.labels`), and for a pull the reviewers to ask, from the owner and | ||
collaborators: nothing is written until it opens. Opening (`createIssue`, | ||
`createPull`) goes through the same code as `knotless issue`, `knotless | ||
pull` and `knotless label`, so a body that follows none of the template is | ||
refused there too, and a label the account may not put on is refused | ||
harness/e2e/flows.spec.mjs+7−10
| @@ -186,11 +186,10 @@ test('opening a pull from a fork', async () => { | ||
await expect(page.getByRole('textbox', { name: 'Title' })).toHaveCount(0) | ||
await page.locator('.compare-ready').getByRole('button', { name: 'Open pull' }).click() | ||
await page.getByRole('textbox', { name: 'Title' }).fill('Bring in the fork: usage, totals and releases') | ||
// Asking the owner to review it as it opens, from the sidebar. | ||
await page.locator('.reviewer-menu summary').click() | ||
await page.locator('.reviewer-menu').getByRole('button', { name: 'alice.test' }).click() | ||
await page.locator('.reviewer-menu summary').click() | ||
await expect(page.locator('.side-people')).toContainText('alice.test') | ||
// Asking the owner to review it as it opens, from Add beside the buttons. | ||
await page.locator('.open-form .add-menu summary').click() | ||
await page.locator('.open-form .add-menu').getByRole('button', { name: 'alice.test' }).click() | ||
await expect(page.locator('.open-form .reviewers-fact')).toContainText('1 reviewer') | ||
// Opened as a draft. | ||
await page.getByRole('button', { name: 'Open as draft' }).click() | ||
await page.waitForURL(/\/alice\.test\/tally\/pull\//) | ||
| @@ -407,11 +406,9 @@ test('logging in to a second account and labelling as the owner', async () => { | ||
test('opening an issue with a label, as the owner', async () => { | ||
await go(page, '/alice.test/tally/issues/new') | ||
await page.getByRole('textbox', { name: 'Title' }).fill('Say what counts as a word in the README') | ||
const menu = page.locator('.open-side .label-menu') | ||
await menu.locator('summary').click() | ||
await menu.getByRole('button', { name: 'kind/bug' }).click() | ||
await menu.locator('summary').click() | ||
await expect(page.locator('.open-side .labels')).toContainText('kind/bug') | ||
await page.locator('.open-form .add-menu summary').click() | ||
await page.locator('.open-form .add-menu').getByRole('button', { name: 'kind/bug' }).click() | ||
await expect(page.locator('.open-form .labels')).toContainText('kind/bug') | ||
await page.getByRole('button', { name: 'Open issue' }).click() | ||
await page.waitForURL(/\/alice\.test\/tally\/issue\//) | ||
await expect(page.getByRole('heading', { name: 'Say what counts as a word in the README' })).toBeVisible() | ||
web/src/lib/molecules/ReviewerPicker.sveltedeleted+0−22
| @@ -1,22 +0,0 @@ | ||
<script lang="ts"> | ||
import type { ActorView } from '#lib/lexicons/dev/knotless/defs.ts'; | ||
import Avatar from '#lib/atoms/Avatar.svelte'; | ||
import Icon from '#lib/atoms/Icon.svelte'; | ||
/** | ||
* A menu of the `people` who may be asked to review a pull, those | ||
* asked (`on`, by DID) ticked: choosing one tells `toggle` to ask them, | ||
* or no longer. It opens from a gear beside the heading that names it. | ||
*/ | ||
let { people, on, toggle }: { people: ActorView[]; on: ActorView['did'][]; toggle: (did: ActorView['did'], removed: boolean) => void } = $props(); | ||
</script> | ||
| ||
<details class="popover filter-menu reviewer-menu"> | ||
<summary aria-label="Choose reviewers" title="Choose reviewers"><Icon name="gear" /></summary> | ||
<div class="menu"> | ||
<div class="menu-head">Ask to review</div> | ||
{#each people as p (p.did)} | ||
{@const ticked = on.includes(p.did)} | ||
<button class="menu-item" type="button" aria-pressed={ticked} onclick={() => toggle(p.did, ticked)}><span class="tick">{#if ticked}<Icon name="check" />{/if}</span><Avatar name={p.handle} did={p.did} src={p.avatar} /><span>{p.handle}</span></button> | ||
{/each} | ||
</div> | ||
</details> | ||
web/src/lib/organisms/OpenFacts.sveltenew+39−0
| @@ -0,0 +1,39 @@ | ||
<script lang="ts"> | ||
import type { ActorView } from '#lib/lexicons/dev/knotless/defs.ts'; | ||
import type * as repo from '#lib/lexicons/dev/knotless/repo.ts'; | ||
import ItemFacts from '#lib/organisms/ItemFacts.svelte'; | ||
/** | ||
* What a pull or an issue will carry as it opens, in the facts line | ||
* its page shows once open: the labels to put on it, of those | ||
* `offered`, and with `people` the reviewers to ask, from them. Nothing | ||
* is written until it opens: what is chosen is bound, `labels` by name | ||
* and `reviewers` by DID, for the form to send. | ||
*/ | ||
let { | ||
offered, | ||
labels = $bindable([]), | ||
people = [], | ||
reviewers = $bindable([]) | ||
}: { offered: repo.Label[]; labels?: string[]; people?: ActorView[]; reviewers?: ActorView['did'][] } = $props(); | ||
const chosen = $derived(offered.filter((l) => labels.includes(l.name))); | ||
const asked = $derived(people.filter((p) => reviewers.includes(p.did))); | ||
const flip = <T extends string>(list: T[], one: T, removed: boolean) => (removed ? list.filter((x) => x !== one) : [...list, one]); | ||
/** Its scope: what comes before its last `/`. */ | ||
const scope = (name: string) => name.slice(0, Math.max(0, name.lastIndexOf('/'))); | ||
/** Labels with `name` put on or taken off: an exclusive one put on takes the others of its scope off, as labelling does. */ | ||
async function label(name: string, removed: boolean) { | ||
const l = offered.find((o) => o.name === name); | ||
const rivals = (o: string) => !!l?.exclusive && !removed && !!scope(name) && scope(o) === scope(name) && !!offered.find((x) => x.name === o)?.exclusive; | ||
labels = flip( | ||
labels.filter((o) => !rivals(o)), | ||
name, | ||
removed | ||
); | ||
} | ||
/** Asks `did` to review it as it opens, or no longer. */ | ||
async function ask(did: ActorView['did'], removed: boolean) { | ||
reviewers = flip(reviewers, did, removed); | ||
} | ||
</script> | ||
| ||
<ItemFacts requested={asked} {people} ask={people.length ? ask : undefined} labels={chosen} {offered} label={offered.length ? label : undefined} /> | ||
web/src/lib/organisms/OpenForm.svelte+5−2
| @@ -2,6 +2,7 @@ | ||
import type { Image } from '#lib/lexicons/dev/knotless/defs.ts'; | ||
import { untrack } from 'svelte'; | ||
import type { AtUriString } from '@atproto/lex'; | ||
import type { Snippet } from 'svelte'; | ||
import Icon from '#lib/atoms/Icon.svelte'; | ||
import MarkdownField from '#lib/molecules/MarkdownField.svelte'; | ||
/** | ||
| @@ -10,9 +11,10 @@ | ||
* given both, with the pictures uploaded into the body, and answers why it failed, in the server's words, or | ||
* nothing. Its preview resolves relative links inside `repo`. Anyone | ||
* not `in` has no button to send it. With `draft`, a second button | ||
* of that name sends it as a draft. | ||
* of that name sends it as a draft. `facts`, drawn beside the buttons, | ||
* is what it will carry as it opens. | ||
*/ | ||
let { in: logged, label, draft, title = '', text = '', send, repo }: { in: boolean; label: string; draft?: string; title?: string; text?: string; send: (title: string, body: string, images?: Image[], draft?: boolean) => Promise<string | undefined>; repo?: AtUriString } = $props(); | ||
let { in: logged, label, draft, title = '', text = '', send, repo, facts }: { in: boolean; label: string; draft?: string; title?: string; text?: string; send: (title: string, body: string, images?: Image[], draft?: boolean) => Promise<string | undefined>; repo?: AtUriString; facts?: Snippet } = $props(); | ||
let heading = $state(untrack(() => title)); | ||
let body = $state(untrack(() => text)); | ||
let images = $state<Image[]>([]); | ||
| @@ -33,6 +35,7 @@ | ||
<MarkdownField label="Description" placeholder="What it is about, in Markdown" rows={14} {repo} bind:value={body} bind:images /> | ||
{#if problem}<p class="problem" role="alert">{problem}</p>{/if} | ||
<div class="composer-end"> | ||
{@render facts?.()} | ||
{#if logged && draft}<button class="btn" type="submit" value="draft" disabled={!!busy || !heading.trim()} aria-busy={busy === 'draft'}><Icon name="pullDraft" />{draft}</button>{/if} | ||
{#if logged}<button class="btn primary" type="submit" value="open" disabled={!!busy || !heading.trim()} aria-busy={busy === 'open'}>{label}</button>{/if} | ||
</div> | ||
web/src/lib/organisms/OpenSide.sveltedeleted+0−53
| @@ -1,53 +0,0 @@ | ||
<script lang="ts"> | ||
import type { ActorView } from '#lib/lexicons/dev/knotless/defs.ts'; | ||
import type * as repo from '#lib/lexicons/dev/knotless/repo.ts'; | ||
import Avatar from '#lib/atoms/Avatar.svelte'; | ||
import LabelPicker from '#lib/molecules/LabelPicker.svelte'; | ||
import Labels from '#lib/molecules/Labels.svelte'; | ||
import Person from '#lib/molecules/Person.svelte'; | ||
import ReviewerPicker from '#lib/molecules/ReviewerPicker.svelte'; | ||
/** | ||
* Beside the form that opens a pull or an issue: the | ||
* labels to put on it, of those `offered`, and with `people` the | ||
* reviewers to ask, from them. What is chosen is bound, `labels` by | ||
* name and `reviewers` by DID, for the form to send as it opens. | ||
*/ | ||
let { | ||
offered, | ||
labels = $bindable([]), | ||
people, | ||
reviewers = $bindable([]) | ||
}: { offered: repo.Label[]; labels?: string[]; people?: ActorView[]; reviewers?: ActorView['did'][] } = $props(); | ||
const chosen = $derived(offered.filter((l) => labels.includes(l.name))); | ||
const asked = $derived((people ?? []).filter((p) => reviewers.includes(p.did))); | ||
const flip = <T extends string>(list: T[], one: T, removed: boolean) => (removed ? list.filter((x) => x !== one) : [...list, one]); | ||
/** Its scope: what comes before its last `/`. */ | ||
const scope = (name: string) => name.slice(0, Math.max(0, name.lastIndexOf('/'))); | ||
/** Labels with `name` put on or taken off: an exclusive one put on takes the others of its scope off, as labelling does. */ | ||
function pick(name: string, removed: boolean) { | ||
const l = offered.find((o) => o.name === name); | ||
const rivals = (o: string) => !!l?.exclusive && !removed && !!scope(name) && scope(o) === scope(name) && !!offered.find((x) => x.name === o)?.exclusive; | ||
labels = flip(labels.filter((o) => !rivals(o)), name, removed); | ||
} | ||
</script> | ||
| ||
<aside class="open-side"> | ||
{#if people?.length} | ||
<section> | ||
<div class="side-head"><h3>Reviewers</h3><ReviewerPicker {people} on={reviewers} toggle={(did, removed) => (reviewers = flip(reviewers, did, removed))} /></div> | ||
{#if asked.length} | ||
<ul class="side-people"> | ||
{#each asked as p (p.did)}<li><Avatar name={p.handle} did={p.did} src={p.avatar} /><Person handle={p.handle} /></li>{/each} | ||
</ul> | ||
{:else} | ||
<p class="mute">No reviewers</p> | ||
{/if} | ||
</section> | ||
{/if} | ||
{#if offered.length} | ||
<section> | ||
<div class="side-head"><h3>Labels</h3><LabelPicker labels={offered} on={labels} gear toggle={async (name, removed) => pick(name, removed)} /></div> | ||
{#if chosen.length}<Labels labels={chosen} />{:else}<p class="mute">No labels</p>{/if} | ||
</section> | ||
{/if} | ||
</aside> | ||
web/src/routes/[authority]/[repo]/issues/new/+page.svelte+4−5
| @@ -5,7 +5,7 @@ | ||
import { call } from '#lib/data.ts'; | ||
import { itemHref, repoHref } from '#lib/format.ts'; | ||
import OpenForm from '#lib/organisms/OpenForm.svelte'; | ||
import OpenSide from '#lib/organisms/OpenSide.svelte'; | ||
import OpenFacts from '#lib/organisms/OpenFacts.svelte'; | ||
import RepoPage from '#lib/templates/RepoPage.svelte'; | ||
let { data } = $props(); | ||
const r = $derived(data.repo); | ||
| @@ -22,8 +22,7 @@ | ||
| ||
<RepoPage repo={r} tab="issues" title="New issue · {r.name}"> | ||
<h2 class="form-title">New issue</h2> | ||
<div class="open-layout"> | ||
<OpenForm repo={`at://${data.authority}/dev.knotless.repo/${r.name}`} in={!!data.viewer?.actor} label="Open issue" text={data.template} send={open} /> | ||
{#if offered.length}<OpenSide {offered} bind:labels />{/if} | ||
</div> | ||
<OpenForm repo={`at://${data.authority}/dev.knotless.repo/${r.name}`} in={!!data.viewer?.actor} label="Open issue" text={data.template} send={open}> | ||
{#snippet facts()}<OpenFacts {offered} bind:labels />{/snippet} | ||
</OpenForm> | ||
</RepoPage> | ||
web/src/routes/[authority]/[repo]/pulls/new/+page.svelte+6−7
| @@ -9,7 +9,7 @@ | ||
import CommitEvent from '#lib/organisms/CommitEvent.svelte'; | ||
import Diff from '#lib/organisms/Diff.svelte'; | ||
import OpenForm from '#lib/organisms/OpenForm.svelte'; | ||
import OpenSide from '#lib/organisms/OpenSide.svelte'; | ||
import OpenFacts from '#lib/organisms/OpenFacts.svelte'; | ||
import RepoPage from '#lib/templates/RepoPage.svelte'; | ||
import Empty from '#lib/molecules/Empty.svelte'; | ||
let { data } = $props(); | ||
| @@ -71,12 +71,11 @@ | ||
<div class="box"><Empty title="Branch not found">{`${from ? `${from.handle}/${from.name}` : 'That repository'} has no branch ${data.branch}, or ${r.name} no branch ${data.base}.`}</Empty></div> | ||
{:else} | ||
{#if writing} | ||
<div class="open-layout"> | ||
{#key `${data.head} ${data.branch} ${data.base}`} | ||
<OpenForm repo={`at://${data.authority}/dev.knotless.repo/${r.name}`} in={!!me} label="Open pull" draft="Open as draft" {title} text={data.template} send={open} /> | ||
{/key} | ||
{#if me}<OpenSide {offered} bind:labels {people} bind:reviewers />{/if} | ||
</div> | ||
{#key `${data.head} ${data.branch} ${data.base}`} | ||
<OpenForm repo={`at://${data.authority}/dev.knotless.repo/${r.name}`} in={!!me} label="Open pull" draft="Open as draft" {title} text={data.template} send={open}> | ||
{#snippet facts()}{#if me}<OpenFacts {offered} bind:labels {people} bind:reviewers />{/if}{/snippet} | ||
</OpenForm> | ||
{/key} | ||
{:else if ch.commits.length} | ||
<div class="box compare-ready"> | ||
<span>{`${plural(ch.commits.length, 'commit', 'commits')} and ${plural(ch.files.length, 'file', 'files')} changed, from ${data.branch} into ${data.base}.`}</span> | ||
web/src/app.css+3−13
| @@ -820,20 +820,10 @@ textarea.field{resize:vertical;font-family:var(--font-mono);font-size:var(--text | ||
.list-top{align-items:center} | ||
.form-title{font-size:var(--text-2xl);font-weight:var(--weight-medium);margin-bottom:var(--space-2)} | ||
.form-lead{margin-bottom:var(--space-4)} | ||
.open-form{overflow:visible} | ||
.open-form .composer-end .item-facts{flex:1 1 auto} | ||
@media (max-width:640px){.open-form .composer-end .item-facts{flex-basis:100%}} | ||
.open-form .title{font-family:var(--font-sans);font-size:var(--text-lg)} | ||
.open-layout{display:grid;grid-template-columns:minmax(0,1fr) var(--side-width);align-items:start;gap:var(--space-6)} | ||
.open-layout>:only-child{grid-column:1/-1} | ||
.open-side{display:flex;flex-direction:column;font-size:var(--text-sm)} | ||
.open-side section{display:flex;flex-direction:column;gap:var(--space-2);padding-bottom:var(--space-4);border-bottom:var(--border)} | ||
.open-side section+section{padding-top:var(--space-4)} | ||
.open-side section:last-child{padding-bottom:0;border-bottom:0} | ||
.side-head{display:flex;align-items:center;justify-content:space-between;gap:var(--space-2)} | ||
.side-head h3{font-size:var(--text-sm);font-weight:var(--weight-semibold);color:var(--mute)} | ||
.side-head .filter-menu>summary{padding:var(--space-1);border-radius:var(--radius-sm)} | ||
.side-people{display:flex;flex-direction:column;gap:var(--space-2);margin:0;padding:0;list-style:none} | ||
.side-people li{display:flex;align-items:center;gap:var(--space-2);min-width:0;overflow-wrap:anywhere} | ||
.reviewer-menu .menu-item{align-items:center;gap:var(--space-2)} | ||
@media (max-width:640px){.open-layout{grid-template-columns:minmax(0,1fr)}.open-side{order:-1}} | ||
.compare{display:flex;flex-wrap:wrap;align-items:flex-end;gap:var(--space-3);padding:var(--space-3);margin-bottom:var(--space-4);overflow:visible} | ||
.compare label{display:flex;flex-direction:column;gap:var(--space-1);font-size:var(--text-sm);font-weight:var(--weight-medium)} | ||
.compare>.i{color:var(--mute);margin-bottom:var(--space-2)} | ||