fix(canvases): dismiss previews and enable PR card dragging

Close review board previews after successful item actions and outside clicks without dismissing a different item during in-flight requests. Explicitly enable native dragging on editable PR cards and add regression coverage.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: cd29e26b-0e56-48bf-a046-ac749aef86ce
This commit is contained in:
Aaron Powell
2026-10-14 11:25:57 +11:00
co-authored by Copilot App
parent 764b3829bf
commit a10206b7f6
6 changed files with 283 additions and 9 deletions
@@ -26,6 +26,8 @@ Open it by asking Copilot to "open the plugin review board" (canvas id `external
Fast-track never overrides an existing review or a bucket you chose; drag a card to *Unreviewed* to opt it
out and have it reviewed normally. Cards show *First party* / *Third party* pills from the intake labels.
- **Details panel** shows the AI review, the rendered issue body, and all comments.
Clicking outside the panel or successfully submitting an item action closes it;
failed actions leave it open so you can correct or retry them.
- **Quick actions** post `/approve` (with an optional note) or `/reject <reason>` straight to the issue
after a confirm click. The reject reason is prefilled from the AI's suggested comment.
- **Column actions** approve, reject, or re-review every eligible item in a column, including
@@ -60,3 +62,4 @@ Run `node --experimental-vm-modules --test eng/board-state-storage.test.mjs` fro
to check both canvases' session-scoped persistence without accessing GitHub.
Run `node --test eng/board-bulk-actions.test.mjs` to check column confirmation, staged feedback,
queue exclusions, and partial failures using in-process fakes.
Run `node --test eng/board-preview.test.mjs` to check preview dismissal and in-flight actions.
@@ -261,6 +261,7 @@ async function bulkAction(columnId, kind) {
}
await api("POST", "/api/decision", { ...item, kind });
completed++;
if (selected === item.number) closeDrawer();
} catch (error) {
failures.push(`#${item.number}: ${error.message}`);
}
@@ -343,6 +344,7 @@ async function moveItem(number, column) {
const hadReview = column === "unreviewed" && Boolean(findItem(number)?.review);
try {
await api("POST", "/api/move", { number, column });
if (selected === number) closeDrawer();
if (hadReview) toast(`Cleared the AI review for #${number}; it will be included in the next review.`);
} catch (error) {
toast(error.message, { error: true });
@@ -616,10 +618,11 @@ async function decide(kind) {
: `Posted ${result.body.split("\n")[0]} on #${item.number}`,
{ link: result.commentUrl },
);
if (selected === item.number) closeDrawer();
} catch (error) {
toast(error.message, { error: true });
} finally {
resetConfirm();
if (selected === item.number) resetConfirm();
}
}
@@ -672,6 +675,9 @@ ui.search.addEventListener("input", () => {
});
ui.drawerClose.addEventListener("click", closeDrawer);
document.addEventListener("pointerdown", (event) => {
if (selected !== null && !ui.drawer.contains(event.target)) closeDrawer();
}, true);
ui.drawerMove.addEventListener("change", () => moveItem(selected, ui.drawerMove.value));
function hideRereview() {
ui.rereviewPanel.hidden = true;
@@ -711,7 +717,7 @@ ui.rereviewPanel.addEventListener("submit", async (event) => {
try {
const result = await api("POST", "/api/rereview", { number, guidance: ui.rereviewGuidance.value });
toast(result.message);
hideRereview();
if (selected === number) closeDrawer();
} catch (error) {
toast(error.message, { error: true });
} finally {
@@ -32,6 +32,8 @@ authenticated `gh` CLI.
- **Detail drawer** — AI review, description, conversation, changed files, and checks.
Keyboard focus stays inside the modal drawer and returns to its card on close.
Use Left/Right arrows or Home/End to switch detail tabs.
Clicking outside the drawer or successfully submitting an item action closes it;
failed actions leave it open so you can correct or retry them.
- **GitHub actions** — submit feedback, request changes, close a PR, or approve it and
enable auto-merge with a merge commit. Each action needs two clicks to confirm and
is also recorded in local board history. Approval is pinned to the board's commit
@@ -92,5 +94,7 @@ Run `node --test eng/open-pr-board.test.mjs` from the repository root. GitHub wr
are tested with injected in-process fakes, never the authenticated CLI.
Run `node --test eng/board-bulk-actions.test.mjs` to check column confirmation, staged feedback,
queue exclusions, and partial failures using in-process fakes.
Run `node --test eng/board-preview.test.mjs` to check preview dismissal and in-flight actions.
Run `node --test eng/open-pr-board-drag.test.mjs` to check native card dragging and transfer data.
Run `node --experimental-vm-modules --test eng/board-state-storage.test.mjs` to check
both canvases' session-scoped persistence without accessing GitHub.
+16 -7
View File
@@ -241,7 +241,7 @@ function renderCard(item) {
"article",
{
class: item.number === selectedNumber ? "card selected" : "card",
draggable: !item.decision,
draggable: item.decision ? "false" : "true",
dataset: { number: String(item.number) },
tabindex: "0",
role: "button",
@@ -402,6 +402,7 @@ async function bulkAction(columnId, kind) {
method: "POST", body: { number: item.number, kind, comment: item.comment },
});
completed++;
if (selectedNumber === item.number) closeDrawer();
if (result.warning) failures.push(`#${item.number}: ${result.warning}`);
} catch (error) {
failures.push(`#${item.number}: ${error.message}`);
@@ -436,12 +437,14 @@ function reviewTab(item) {
{
class: "btn btn-small btn-primary",
type: "button",
onclick: (event) =>
run(
onclick: async (event) => {
const result = await run(
event.currentTarget,
() => api("/api/review", { method: "POST", body: { numbers: [item.number] } }),
(result) => result.message ?? "Review requested.",
),
);
if (result && selectedNumber === item.number) closeDrawer();
},
},
"Review this PR",
),
@@ -728,12 +731,12 @@ async function submitDecision(button, kind) {
api("/api/decision", { method: "POST", body: { number: item.number, kind, comment } }),
);
decisionsInFlight.delete(item.number);
renderDrawerHeader();
if (selectedNumber === item.number) renderDrawerHeader();
button.textContent = DECISION_BUTTONS[kind];
if (result) {
if (result.warning) toast(result.warning, { error: true, url: result.url });
else toast(`#${item.number}: ${result.status}.`, { url: result.url });
if (currentItem()) loadDetail(item.number, { force: true });
if (selectedNumber === item.number) closeDrawer();
}
}
@@ -778,6 +781,10 @@ el.review.addEventListener("click", (event) =>
el.drawerClose.addEventListener("click", closeDrawer);
document.addEventListener("pointerdown", (event) => {
if (selectedNumber !== null && !el.drawer.contains(event.target)) closeDrawer();
}, true);
document.addEventListener("keydown", (event) => {
if (!el.drawer.classList.contains("open")) return;
if (event.key === "Escape") {
@@ -806,6 +813,7 @@ el.drawerColumn.addEventListener("change", async (event) => {
if (column === item.column) return;
try {
await api("/api/move", { method: "POST", body: { number: item.number, column } });
if (selectedNumber === item.number) closeDrawer();
} catch (error) {
toast(error.message, { error: true });
event.target.value = item.column;
@@ -855,9 +863,10 @@ el.rereviewStart.addEventListener("click", async (event) => {
}),
(value) => value.message ?? "Re-review requested.",
);
if (result) {
if (result && selectedNumber === item.number) {
el.rereviewGuidance.value = "";
el.rereviewPanel.hidden = true;
closeDrawer();
}
});
+185
View File
@@ -0,0 +1,185 @@
import assert from "node:assert/strict";
import { readFile } from "node:fs/promises";
import vm from "node:vm";
import { test } from "node:test";
async function createUI(boardName, request = async () => ({
status: "submitted", body: "/approve", pending: true, message: "Queued",
})) {
const nodes = new Map();
function node() {
const classes = new Set();
return {
content: "", value: "", dataset: {}, listeners: {}, attributes: {},
classList: {
add: (name) => classes.add(name),
remove: (name) => classes.delete(name),
contains: (name) => classes.has(name),
},
addEventListener(name, handler) { this.listeners[name] = handler; },
setAttribute(name, value) { this.attributes[name] = value; },
querySelectorAll: () => [],
querySelector: () => null,
contains(target) { return target === this || target.insideDrawer === true; },
focus() { this.focused = true; },
};
}
const listeners = new Map();
const document = {
querySelector: node,
querySelectorAll: () => [],
getElementById(id) {
if (!nodes.has(id)) nodes.set(id, node());
return nodes.get(id);
},
addEventListener(name, handler, capture) { listeners.set(name, { handler, capture }); },
};
const messages = [];
const context = vm.createContext({
document, setTimeout: () => 1, clearTimeout() {}, confirm: () => true,
});
const isPR = boardName === "open-pr-board";
const source = await readFile(new URL(`../.github/extensions/${boardName}/public/app.js`, import.meta.url), "utf8");
const end = isPR ? "const events = new EventSource" : "function connect()";
vm.runInContext(source.slice(0, source.indexOf(end)), context);
context.request = request;
context.notify = (message) => messages.push(message);
vm.runInContext(`
board = { items: [1, 2].map(number => ({
number, column: "approve", headRefOid: "first", reviewEligible: true,
review: { suggestedComment: "Feedback" },
})), columns: [] };
renderBoard = renderDrawer = renderDrawerHeader = renderDrawerBody = () => {};
api = request;
toast = notify;
`, context);
const drawer = nodes.get("drawer");
function select(number) {
vm.runInContext(`${isPR ? "selectedNumber" : "selected"} = ${number}`, context);
drawer.classList.add("open");
drawer.setAttribute("aria-hidden", "false");
drawer.inert = false;
nodes.get("board").inert = isPR;
if (isPR) {
vm.runInContext(`drawerOriginNumber = ${number}`, context);
vm.runInContext("el.toolbar.inert = true", context);
}
nodes.get("decision-comment").value = "Feedback";
}
select(1);
async function decide(kind = "approve") {
if (isPR) {
const button = nodes.get("decision-approve") ?? node();
await context.submitDecision(button, kind);
return context.submitDecision(button, kind);
}
await context.decide(kind);
return context.decide(kind);
}
return {
context, nodes, drawer, listeners, messages, select, decide,
selected: () => vm.runInContext(isPR ? "selectedNumber" : "selected", context),
};
}
for (const boardName of ["open-pr-board", "external-plugin-review-board"]) {
test(`${boardName}: clicking inside retains the preview; clicking outside closes it`, async () => {
const ui = await createUI(boardName);
const { handler, capture } = ui.listeners.get("pointerdown");
assert.equal(capture, true);
handler({ target: { insideDrawer: true } });
assert.equal(ui.selected(), 1);
handler({ target: {} });
assert.equal(ui.selected(), null);
assert.equal(ui.drawer.classList.contains("open"), false);
assert.equal(ui.drawer.attributes["aria-hidden"], "true");
assert.equal(ui.nodes.get("board").inert, false);
if (boardName === "open-pr-board") {
assert.equal(ui.drawer.inert, true);
assert.equal(ui.nodes.get("filter").focused, true);
assert.equal(vm.runInContext("el.toolbar.inert", ui.context), false);
}
handler({ target: {} });
assert.equal(ui.selected(), null);
});
const kinds = boardName === "open-pr-board"
? ["approve", "close", "request-changes", "comment"] : ["approve", "reject"];
for (const kind of kinds) {
test(`${boardName}: successful ${kind} closes the preview`, async () => {
const ui = await createUI(boardName);
await ui.decide(kind);
assert.equal(ui.selected(), null);
assert.equal(ui.drawer.classList.contains("open"), false);
assert.equal(ui.messages.length, 1);
});
}
test(`${boardName}: a failed action leaves the preview open`, async () => {
const ui = await createUI(boardName, async () => { throw new Error("Offline"); });
await ui.decide();
assert.equal(ui.selected(), 1);
assert.equal(ui.drawer.classList.contains("open"), true);
assert.equal(ui.messages[0], "Offline");
});
test(`${boardName}: completion does not close a different preview opened in flight`, async () => {
let release;
const ui = await createUI(boardName, () => new Promise(resolve => { release = resolve; }));
const pending = ui.decide();
await new Promise(resolve => setImmediate(resolve));
ui.listeners.get("pointerdown").handler({ target: {} });
ui.select(2);
release({ status: "submitted", body: "/approve" });
await pending;
assert.equal(ui.selected(), 2);
assert.equal(ui.drawer.classList.contains("open"), true);
});
test(`${boardName}: successful bucket moves close the preview`, async () => {
const ui = await createUI(boardName);
if (boardName === "open-pr-board") {
await ui.nodes.get("drawer-column").listeners.change({ target: { value: "close" } });
} else {
await ui.context.moveItem(1, "straight-reject");
}
assert.equal(ui.selected(), null);
});
test(`${boardName}: successful re-review closes the preview`, async () => {
const ui = await createUI(boardName);
if (boardName === "open-pr-board") {
await ui.nodes.get("rereview-start").listeners.click({ currentTarget: ui.nodes.get("rereview-start") });
} else {
await ui.nodes.get("rereview-panel").listeners.submit({ preventDefault() {} });
}
assert.equal(ui.selected(), null);
});
test(`${boardName}: failed bucket moves and re-reviews leave the preview open`, async () => {
const ui = await createUI(boardName, async () => { throw new Error("Offline"); });
if (boardName === "open-pr-board") {
await ui.nodes.get("drawer-column").listeners.change({ target: { value: "close" } });
await ui.nodes.get("rereview-start").listeners.click({ currentTarget: ui.nodes.get("rereview-start") });
} else {
await ui.context.moveItem(1, "straight-reject");
await ui.nodes.get("rereview-panel").listeners.submit({ preventDefault() {} });
}
assert.equal(ui.selected(), 1);
assert.equal(ui.drawer.classList.contains("open"), true);
assert.deepEqual(ui.messages, ["Offline", "Offline"]);
});
test(`${boardName}: bulk decisions close a preview of a successfully actioned item`, async () => {
const ui = await createUI(boardName);
await ui.context.bulkAction("approve", "approve");
assert.equal(ui.selected(), null);
});
}
test("open-pr-board: partial approval success closes the preview and keeps the warning visible", async () => {
const ui = await createUI("open-pr-board", async () => ({ warning: "Auto-merge could not be enabled" }));
await ui.decide();
assert.equal(ui.selected(), null);
assert.equal(ui.messages[0], "Auto-merge could not be enabled");
});
+67
View File
@@ -0,0 +1,67 @@
import assert from "node:assert/strict";
import { readFile } from "node:fs/promises";
import vm from "node:vm";
import { test } from "node:test";
const source = await readFile(
new URL("../.github/extensions/open-pr-board/public/app.js", import.meta.url), "utf8",
);
class Node {
constructor() {
this.attributes = {};
this.dataset = {};
this.listeners = {};
const classes = new Set();
this.classList = {
add: (name) => classes.add(name),
remove: (name) => classes.delete(name),
contains: (name) => classes.has(name),
};
}
setAttribute(name, value) { this.attributes[name] = value; }
addEventListener(name, handler) { this.listeners[name] = handler; }
append() {}
get draggable() { return this.attributes.draggable === "true"; }
}
function createContext() {
const context = vm.createContext({
Node,
document: { createElement: () => new Node(), createTextNode: () => new Node() },
selectedNumber: null,
relativeTime: () => "now",
statusPills: () => [],
});
for (const [start, end] of [
["function h(", "/**"],
["function renderCard(", "function renderBoard("],
]) {
vm.runInContext(source.slice(source.indexOf(start), source.indexOf(end, source.indexOf(start))), context);
}
return context;
}
test("PR cards explicitly enable native dragging; actioned cards explicitly disable it", () => {
const context = createContext();
const item = { number: 42, title: "Example", author: "contributor" };
const card = context.renderCard(item);
assert.equal(card.attributes.draggable, "true");
assert.equal(card.draggable, true);
const actioned = context.renderCard({ ...item, decision: { kind: "approve" } });
assert.equal(actioned.attributes.draggable, "false");
assert.equal(actioned.draggable, false);
});
test("dragging transfers the PR number for moving and clears its visual state on completion", () => {
const context = createContext();
const card = context.renderCard({ number: 42, title: "Example", author: "contributor" });
const values = new Map();
const dataTransfer = { setData: (type, value) => values.set(type, value) };
card.listeners.dragstart({ dataTransfer });
assert.equal(values.get("text/plain"), "42");
assert.equal(dataTransfer.effectAllowed, "move");
assert.equal(card.classList.contains("dragging"), true);
card.listeners.dragend();
assert.equal(card.classList.contains("dragging"), false);
});