diff --git a/apps/desktop/bundle-budget.json b/apps/desktop/bundle-budget.json index de348eff2e..064ce5ed82 100644 --- a/apps/desktop/bundle-budget.json +++ b/apps/desktop/bundle-budget.json @@ -1,29 +1,29 @@ { "slack": 0.05, "gzipBytes": { - "renderer/panel.js": 858173, - "renderer/voice.js": 340950, - "renderer/call.js": 385392, - "renderer/whiteboard.js": 441173, + "renderer/panel.js": 901571, + "renderer/voice.js": 345922, + "renderer/call.js": 402844, + "renderer/whiteboard.js": 441172, "renderer/file-tree.js": 72151, - "renderer/plan-editor.js": 176496, + "renderer/plan-editor.js": 177053, "renderer/session-recorder.js": 61865, - "renderer/terminal.js": 146927, + "renderer/terminal.js": 146931, "renderer/pcm-tee-worklet.js": 413, "renderer/avatar-session.js": 149485, - "renderer/diagram.js": 195519, + "renderer/diagram.js": 195523, "renderer/diff-view.js": 178690, - "renderer/factory-world.js": 161750, + "renderer/factory-world.js": 161840, "renderer/factory-world-dev.js": 1745, - "renderer/factory-world-station.js": 31732, - "renderer/factory-world-citadel.js": 31274, - "renderer/factory-world-railway.js": 41051, - "renderer/factory-world-archipelago.js": 35105, - "renderer/factory-world-neon.js": 29957, - "renderer/factory-world-factory.js": 38318, - "renderer/factory-world-colony.js": 31066, - "renderer/factory-world-atlantis.js": 37101, - "renderer/factory-world-tree.js": 34653, - "renderer/factory-world-planet.js": 30555 + "renderer/factory-world-station.js": 32657, + "renderer/factory-world-citadel.js": 31901, + "renderer/factory-world-railway.js": 41644, + "renderer/factory-world-archipelago.js": 35693, + "renderer/factory-world-neon.js": 30543, + "renderer/factory-world-factory.js": 38317, + "renderer/factory-world-colony.js": 32366, + "renderer/factory-world-atlantis.js": 37955, + "renderer/factory-world-tree.js": 34651, + "renderer/factory-world-planet.js": 30877 } } diff --git a/apps/desktop/src/renderer/styles/tasks.css b/apps/desktop/src/renderer/styles/tasks.css index cf4b333890..b66199ca8e 100644 --- a/apps/desktop/src/renderer/styles/tasks.css +++ b/apps/desktop/src/renderer/styles/tasks.css @@ -2107,6 +2107,44 @@ user-select: text; } +/* The title edited in place (`pull-request-writes.tsx`): its pencil stands + faint after the number until the title is hovered or the pencil focused, + and the field takes the title's own type and room. */ +.pull-request-title-edit { + width: 22px; + height: 22px; + margin-inline-start: var(--luke-space-2); + vertical-align: middle; + opacity: 0; +} + +.pull-request-title-edit svg { + width: 13px; + height: 13px; +} + +.pull-request-title-editable:hover .pull-request-title-edit, +.pull-request-title-edit:focus-visible { + opacity: 1; +} + +.pull-request-title-field { + display: flex; + align-items: center; + gap: var(--luke-space-2); +} + +.pull-request-title-input { + min-width: 0; + flex: 1 1 auto; + padding: var(--luke-space-2) var(--luke-space-4); + border: 1px solid var(--luke-color-border-strong); + border-radius: var(--luke-control-radius); + background: var(--luke-color-surface); + color: inherit; + font: inherit; +} + .pull-request-number { color: var(--luke-color-text-subtle); font-weight: var(--luke-weight-regular); @@ -2151,8 +2189,8 @@ } /* The merge box's presses at the line's end (`merge-box.tsx`): what stands - of a merge already asked for, quiet, then the one press, a method's - press joined to the chevron that chooses the method. */ + of a merge already asked for, quiet, then the one press, `@luke/ui`'s + Button, or its SplitButton where a chevron chooses the method. */ .merge-box-actions { display: inline-flex; flex: 0 0 auto; @@ -2167,41 +2205,6 @@ color: var(--luke-color-text-muted); } -.merge-box-split { - display: inline-flex; - align-items: stretch; -} - -.merge-box-press { - height: 26px; - padding: 0 var(--luke-space-8); - gap: var(--luke-space-4); - font-size: 12px; -} - -.merge-box-press svg { - width: 13px; - height: 13px; -} - -.merge-box-split > .merge-box-press:not(:last-child) { - border-start-end-radius: 0; - border-end-end-radius: 0; -} - -.merge-box-picker { - height: 26px; - padding: 0 var(--luke-space-4); - border-start-start-radius: 0; - border-end-start-radius: 0; - margin-inline-start: -1px; -} - -.merge-box-picker svg { - width: 13px; - height: 13px; -} - [data-merge-tone="ready"] { --merge-tone: var(--luke-color-complete); } diff --git a/apps/desktop/src/renderer/tasks/agent-pull-request-tab.test.ts b/apps/desktop/src/renderer/tasks/agent-pull-request-tab.test.ts index f72daab913..7cb0c157fe 100644 --- a/apps/desktop/src/renderer/tasks/agent-pull-request-tab.test.ts +++ b/apps/desktop/src/renderer/tasks/agent-pull-request-tab.test.ts @@ -1663,13 +1663,13 @@ test("a draft's pill offers Ready for review, which is asked of the service for ), ); assert.equal(pill(tab).textContent, "Draft"); - assert.deepEqual(offered(tab), ["Ready for review"]); + assert.deepEqual(offered(tab), ["Ready for review", "Close pull request"]); await pressAction(tab, "Ready for review"); assert.deepEqual(changes, [[AGENT.id, { action: PULL_REQUEST_ACTION.READY_FOR_REVIEW }]]); assert.equal(pill(tab).textContent, "Open"); - assert.deepEqual(offered(tab), ["Convert to draft"]); + assert.deepEqual(offered(tab), ["Convert to draft", "Close pull request"]); assert.equal(tab.querySelector(".pull-request-write-note"), null); }); @@ -1765,13 +1765,13 @@ function merging(merge: MergeState, merged: Partial = {}): C /** The merge box's presses, by their words. */ function boxPresses(tab: HTMLElement): string[] { - return words(tab, ".merge-box-press"); + return words(tab, ".merge-box-actions .luke-button-words"); } /** The merge box's press of the words given, pressed. */ async function pressBox(tab: HTMLElement, label: string): Promise { const press = - [...tab.querySelectorAll(".merge-box-press")].find( + [...tab.querySelectorAll(".merge-box-actions button")].find( (each) => each.textContent === label, ) ?? assert.fail(`the merge box offers ${label}`); act(() => press.click()); @@ -2416,6 +2416,96 @@ test("a reply GitHub refuses says so under its thread in GitHub's own words, its assert.equal(withoutId.querySelector(".pull-request-thread-actions"), null); }); +test("an open pull request's pill offers Close and a closed one's Reopen, each asked of the service for the agent", async () => { + const open = await draw(controlAnswering()); + await pressAction(open, "Close pull request"); + act(() => root?.unmount()); + root = undefined; + const closed = await draw( + controlAnswering({ + pullRequest: () => + Promise.resolve({ pullRequest: { ...PULL_REQUEST, state: PULL_REQUEST_STATE.CLOSED } }), + }), + ); + assert.deepEqual(offered(closed), ["Reopen pull request"]); + await pressAction(closed, "Reopen pull request"); + + assert.deepEqual(changes, [ + [AGENT.id, { action: PULL_REQUEST_ACTION.CLOSE }], + [AGENT.id, { action: PULL_REQUEST_ACTION.REOPEN }], + ]); +}); + +/** The title's field, opened by its pencil. */ +function editTitle(tab: HTMLElement): HTMLInputElement { + act(() => + ( + tab.querySelector('[aria-label="Edit title"]') ?? + assert.fail("the title has a pencil") + ).click(), + ); + return ( + tab.querySelector('[aria-label="Pull request title"]') ?? + assert.fail("the pencil opens a field") + ); +} + +/** Words typed into a field, as React hears them. */ +function type(field: HTMLInputElement, words: string): void { + const setter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, "value")?.set; + act(() => { + setter?.call(field, words); + field.dispatchEvent(new Event("input", { bubbles: true })); + }); +} + +/** A key pressed in a field, while an input method composes a word where `composing` says. */ +async function key(field: HTMLInputElement, name: string, composing = false): Promise { + act(() => { + field.dispatchEvent( + new KeyboardEvent("keydown", { key: name, bubbles: true, isComposing: composing }), + ); + }); + await settle(); +} + +test("the title is edited in place: Enter keeps new words trimmed, and Escape, or words unchanged, sends nothing", async () => { + const tab = await draw(controlAnswering()); + const escaped = editTitle(tab); + assert.equal(document.activeElement, escaped, "the field takes the keys as it opens"); + type(escaped, "Something else"); + await key(escaped, "Enter", true); + await key(escaped, "Escape", true); + assert.equal(escaped.isConnected, true, "keys an input method takes leave the field as it is"); + const heard: string[] = []; + const hear = (event: KeyboardEvent) => heard.push(event.key); + window.addEventListener("keydown", hear); + await key(escaped, "Escape"); + window.removeEventListener("keydown", hear); + assert.equal(tab.querySelector('[aria-label="Pull request title"]'), null); + assert.deepEqual(heard, [], "the field's own Escape never reaches the window"); + + const unchanged = editTitle(tab); + assert.equal(unchanged.value, PULL_REQUEST.title); + await key(unchanged, "Enter"); + assert.deepEqual(changes, [], "neither Escape nor the same words change anything"); + + const renamed = editTitle(tab); + type(renamed, " Expire invitations after a week "); + await key(renamed, "Enter"); + assert.deepEqual(changes, [ + [ + AGENT.id, + { action: PULL_REQUEST_ACTION.EDIT_TITLE, title: "Expire invitations after a week" }, + ], + ]); + assert.equal( + tab.querySelector('[aria-label="Pull request title"]'), + null, + "the field closes once GitHub answered", + ); +}); + test("Files changed draws each review thread under its line, its comments through the overview's own drawing, folded while Minimize inline comments is on", async () => { const review = { kind: CONVERSATION_ENTRY.REVIEW, @@ -2534,3 +2624,33 @@ test("a review thread on its line in Files changed is replied on and resolved fr [AGENT.id, { action: PULL_REQUEST_ACTION.RESOLVE_THREAD, threadId: "PRRT_flag" }], ]); }); + +test("a merge asked about goes with its panel: hidden, the question is let go and nothing merges, and shown again it asks nothing", async () => { + let show: (shown: boolean) => void = () => undefined; + function Hiding() { + const [shown, setShown] = useState(true); + show = setShown; + return createElement(AgentPullRequestTab, { + agentId: AGENT.id, + control: merging(MERGE_STATE.CLEAN), + shown, + repository: "acme/relay", + tree: { open: false, onToggle: () => undefined }, + view: PULL_REQUEST_VIEW.CONVERSATION, + onView: () => undefined, + }); + } + const tab = await drawn(createElement(Hiding)); + // Note that the question is read as whether it stands, so a failure prints a word rather than a whole DOM node. + const question = () => document.querySelector('[role="dialog"], [role="alertdialog"]') !== null; + + await pressBox(tab, "Squash and merge"); + assert.equal(question(), true, "the merge is asked about"); + act(() => show(false)); + assert.equal(question(), false, "the question goes as its panel hides"); + act(() => show(true)); + await settle(); + + assert.equal(question(), false, "the panel comes back asking nothing"); + assert.deepEqual(changes, []); +}); diff --git a/apps/desktop/src/renderer/tasks/agent-pull-request-tab.tsx b/apps/desktop/src/renderer/tasks/agent-pull-request-tab.tsx index c6231f978e..2ec48d19d5 100644 --- a/apps/desktop/src/renderer/tasks/agent-pull-request-tab.tsx +++ b/apps/desktop/src/renderer/tasks/agent-pull-request-tab.tsx @@ -99,6 +99,7 @@ import { PullRequestStack } from "./pull-request-stack"; import { ThreadActions, ThreadWritesProvider } from "./pull-request-thread-actions"; import { PullRequestStateMenu, + PullRequestTitle, PullRequestWriteNote, type PullRequestWrites, usePullRequestWrites, @@ -1019,12 +1020,15 @@ function PullRequestHead({ parts, now, repository, + shown, writes, }: { parts: OverviewParts; now: number; /** The agent's repository, which the merge box keeps the method chosen by. */ repository: string | null; + /** Whether the panel is on screen, which a merge asked about goes with. */ + shown: boolean; writes: PullRequestWrites; }): React.JSX.Element { const { pullRequest: part, conversation } = parts; @@ -1075,7 +1079,12 @@ function PullRequestHead({ + } /> ) : null} @@ -1285,8 +1294,10 @@ function ConversationPart({ /** The fixed header's title and number, as the pull request's read says or, until it lands, as Luke held them; a skeleton where neither has. */ function HeaderTitle({ title, + writes, }: { title: Pick | undefined; + writes: PullRequestWrites; }): React.JSX.Element { if (title === undefined) { return ( @@ -1298,9 +1309,7 @@ function HeaderTitle({ ); } return ( -

- {title.title} {pullRequestNumberLabel(title)} -

+ ); } @@ -1656,8 +1665,14 @@ export function AgentPullRequestTab({ ) : ( <>
- - + +
label="Pull request" diff --git a/apps/desktop/src/renderer/tasks/merge-box.tsx b/apps/desktop/src/renderer/tasks/merge-box.tsx index f0d5f084b2..17e32fae07 100644 --- a/apps/desktop/src/renderer/tasks/merge-box.tsx +++ b/apps/desktop/src/renderer/tasks/merge-box.tsx @@ -12,10 +12,19 @@ import { type MergeMethod, PULL_REQUEST_ACTION, } from "@luke/features/pull-request-write-wire"; -import { ConfirmDialog, DIALOG_TONE, useConfirmDialog, useMenu } from "@luke/ui"; +import { + BUTTON_VARIANT, + Button, + type ButtonVariant, + ConfirmDialog, + DIALOG_TONE, + SplitButton, + useConfirmDialog, + useMenu, +} from "@luke/ui"; import { ACTION_RESULT_STATUS } from "@luke/wire"; import { Schema } from "effect"; -import { ChevronDownIcon, GitMergeIcon } from "lucide-react"; +import { GitMergeIcon } from "lucide-react"; import { useState } from "react"; import { ActionMenu, type MenuAction } from "../desktop/action-menu"; import { readKept, writeKept } from "../desktop/folded-groups"; @@ -133,19 +142,37 @@ function useMergeMethod( return [method, choose]; } -/** The chevron beside a press that names a method, opening the methods the repository allows. */ -function MethodPicker({ +/** + * A press that acts by the method chosen, joined to the chevron that opens + * the methods the repository allows (`@luke/ui`'s SplitButton); the press + * alone where the repository allows one method. + */ +function MethodPress({ + variant, + label, methods, method, disabled, + onPress, onChoose, }: { + variant: ButtonVariant; + label: string; methods: readonly MergeMethod[]; method: MergeMethod; disabled: boolean; + onPress: () => void; onChoose: (method: MergeMethod) => void; }): React.JSX.Element { - const menu = useMenu(true); + const menu = useMenu(methods.length > 1); + const icon = variant === BUTTON_VARIANT.PRIMARY ?