{#if requestFor && requester}
requested by {requester.name} · {stamp(requestFor.value.createdAt)}
@@ -424,7 +520,54 @@
{/if}
{#if version}
-
+
+
+ {#if passage}
+
event.stopPropagation()}
+ onpointerup={(event) => event.stopPropagation()}
+ >
+
+ Suggest a change
+
+
+
{passage.text}
+
+
+
+
+
+
+ {/if}
+
{/if}
{#if version?.artifact.value.criteria?.length}
@@ -645,7 +788,14 @@
{#if judging && version}
{#key version.artifact.cid}
-
+
{/key}
{:else if asking && version}
diff --git a/packages/ui/src/lib/components/UnitDetail.svelte.test.ts b/packages/ui/src/lib/components/UnitDetail.svelte.test.ts
index 38c466c..5332c0b 100644
--- a/packages/ui/src/lib/components/UnitDetail.svelte.test.ts
+++ b/packages/ui/src/lib/components/UnitDetail.svelte.test.ts
@@ -7,7 +7,7 @@ import {
type ReviewRecord,
type UnitView,
} from '@radial/core'
-import { fixtureSpace } from '@radial/core/fixture'
+import { FIXTURE_DIDS, fixtureSpace } from '@radial/core/fixture'
import { flushSync, mount, unmount } from 'svelte'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { buildDirectory } from '$lib/directory.js'
@@ -24,9 +24,10 @@ import UnitDetail from './UnitDetail.svelte'
// and `timeline()` rather than through a shape invented for this test.
const resolveImage = vi.hoisted(() => vi.fn())
+const mockAccount = vi.hoisted(() => ({ status: 'signed-out', did: '' }))
vi.mock('$lib/image-resolve.js', () => ({ resolveImage }))
vi.mock('$lib/auth.svelte.js', () => ({
- account: { status: 'signed-out', did: '' },
+ account: mockAccount,
}))
const IMAGE = 'at://did:plc:reviewer/com.disnetdev.radial.image/3mrvxfdae4l2m'
@@ -88,8 +89,8 @@ let host: HTMLDivElement
let component: Record
| undefined
/** Open the drawer on v1 — the version the fixture's `request_changes` review pins. */
-function open(bodies: string[]): void {
- const space = spaceWithFindings(bodies)
+function open(bodies: string[], live = false): void {
+ const space = { ...spaceWithFindings(bodies), ...(live ? { fixture: false } : {}) }
const view = goalOf(space)
component = mount(UnitDetail, {
target: host,
@@ -109,6 +110,8 @@ const findingWith = (severity: string): HTMLElement => {
beforeEach(() => {
resolveImage.mockReset()
+ mockAccount.status = 'signed-out'
+ mockAccount.did = ''
host = document.createElement('div')
document.body.append(host)
})
@@ -192,3 +195,56 @@ describe('a review finding on screen', () => {
expect(error.querySelector('.im')?.textContent).toContain('the wrapped bar')
})
})
+
+describe('reviewing selected artifact prose', () => {
+ it('opens beside the selection and carries the quote into a normal review finding', async () => {
+ mockAccount.status = 'signed-in'
+ mockAccount.did = FIXTURE_DIDS.tim
+ open([], true)
+
+ const prose = host.querySelector('.artifact-prose .body')
+ const walker = prose
+ ? document.createTreeWalker(prose, NodeFilter.SHOW_TEXT, {
+ acceptNode: (node) =>
+ node.textContent?.trim() ? NodeFilter.FILTER_ACCEPT : NodeFilter.FILTER_SKIP,
+ })
+ : undefined
+ const text = walker?.nextNode()
+ if (!text?.textContent) throw new Error('artifact prose missing')
+ const stop = Math.min(28, text.textContent.length)
+ const quote = text.textContent.slice(0, stop)
+ const range = document.createRange()
+ range.setStart(text, 0)
+ range.setEnd(text, stop)
+ Object.defineProperty(range, 'getBoundingClientRect', {
+ value: () => ({ left: 40, right: 180, top: 20, bottom: 38, width: 140, height: 18 }),
+ })
+ const selection = window.getSelection()
+ selection?.removeAllRanges()
+ selection?.addRange(range)
+ window.dispatchEvent(new PointerEvent('pointerup', { bubbles: true }))
+ flushSync()
+
+ const popover = host.querySelector('.inline-review')
+ expect(popover?.querySelector('blockquote')?.textContent).toBe(quote)
+
+ const suggestion = popover?.querySelector('textarea') as HTMLTextAreaElement
+ suggestion.value = 'State the observable outcome.'
+ suggestion.dispatchEvent(new InputEvent('input', { bubbles: true }))
+ flushSync()
+ const add = popover?.querySelector('button.btn-bad') as HTMLButtonElement
+ expect(add.disabled).toBe(false)
+ add.click()
+ flushSync()
+
+ await vi.waitFor(() => {
+ flushSync()
+ const editor = host.querySelector('.fedit .cm-content, .fedit textarea')
+ const value =
+ editor instanceof HTMLTextAreaElement ? editor.value : editor?.textContent
+ expect(value).toContain(`> ${quote}`)
+ expect(value).toContain('State the observable outcome.')
+ })
+ expect((host.querySelector('.fedit select') as HTMLSelectElement).value).toBe('warning')
+ })
+})
diff --git a/packages/ui/src/lib/components/Verdict.svelte b/packages/ui/src/lib/components/Verdict.svelte
index eb8d237..37a00bb 100644
--- a/packages/ui/src/lib/components/Verdict.svelte
+++ b/packages/ui/src/lib/components/Verdict.svelte
@@ -29,15 +29,23 @@
space: Space
/** Which button opened it. It decides what the card opens *holding*, never what it will write. */
initial: Verdict
+ /** A passage-level suggestion captured beside the artifact before this full review opened. */
+ initialFinding?: FindingRow | undefined
done: () => void
}
- const { unit, version, space, initial, done }: Props = $props()
+ const { unit, version, space, initial, initialFinding, done }: Props = $props()
// Request-changes opens with a row waiting: a review that sends work back and says nothing about
// why is the one shape of review that is worse than no review. Approve opens with none, and an
// approval with a note on it is a click away.
let rows = $state(
- untrack(() => (initial === 'request_changes' ? [blankFinding()] : [])),
+ untrack(() =>
+ initialFinding
+ ? [initialFinding]
+ : initial === 'request_changes'
+ ? [blankFinding()]
+ : [],
+ ),
)
// Which verdict is being written, or null. Not a chosen-verdict flag: nothing is chosen until a
// button is pressed, and the button that is pressed is the one that writes.
diff --git a/packages/ui/src/lib/verdicts.test.ts b/packages/ui/src/lib/verdicts.test.ts
index 29f3b1e..6f83c4b 100644
--- a/packages/ui/src/lib/verdicts.test.ts
+++ b/packages/ui/src/lib/verdicts.test.ts
@@ -9,6 +9,7 @@ import {
askReviewArgs,
blankFinding,
findingsOf,
+ inlineFinding,
judgeable,
mayJudge,
owedBy,
@@ -66,6 +67,15 @@ describe('what the editor produces', () => {
{ severity: 'info', path: 'src/a.ts', line: 3, body: 'Fix.' },
])
})
+
+ it('keeps a selected passage and its suggestion together as portable markdown', () => {
+ expect(inlineFinding('First line\nSecond line', 'Make this measurable.', 'info')).toEqual({
+ severity: 'info',
+ path: '',
+ line: '',
+ body: '> First line\n> Second line\n\nMake this measurable.',
+ })
+ })
})
describe('what a review pins', () => {
diff --git a/packages/ui/src/lib/verdicts.ts b/packages/ui/src/lib/verdicts.ts
index 91b88f9..3c77b89 100644
--- a/packages/ui/src/lib/verdicts.ts
+++ b/packages/ui/src/lib/verdicts.ts
@@ -66,6 +66,32 @@ export interface FindingDraft {
body: string
}
+/**
+ * Turn a passage selected in rendered artifact prose into an ordinary review finding draft.
+ *
+ * The quote lives in the finding body rather than in observer-local UI state, so it survives the
+ * write, reaches revision-turn bundles, and remains pinned by the review's subject CID. Prefix every
+ * line separately: a multi-paragraph selection is one markdown blockquote, not a first quoted line
+ * followed by reviewer prose that merely looks adjacent.
+ */
+export function inlineFinding(
+ quote: string,
+ suggestion: string,
+ severity: Severity = 'warning',
+): FindingDraft {
+ const quoted = quote
+ .trim()
+ .split('\n')
+ .map((line) => `> ${line}`)
+ .join('\n')
+ return {
+ severity,
+ path: '',
+ line: '',
+ body: `${quoted}\n\n${suggestion.trim()}`,
+ }
+}
+
/**
* One ROW of the findings editor: a draft, plus the two things that are true of the row rather than
* of the finding.