David/WFDASH-WEBAPP
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(giellie): pin Approve/Reject above a scrolling list on long approvals

Browse source 
A long batch (e.g. 16 stock_items.create) made the approval card taller than
the page; it sits outside the message scroller, so the Approve button was
clipped and unreachable. The card now has a bounded height (60dvh), a pinned
header holding Approve/Reject, and only the list of changes scrolls.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
Dave 2026-10-01 08:32:04 +02:00
parent 6c8f63661a
commit b8116d32eb
2 changed files with 41 additions and 13 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
src/components/giellie/approval-card.test.tsx
View file

@ -64,6 +64,30 @@ describe("ApprovalCard", () => {
expect(onDecide).toHaveBeenNthCalledWith(2, false);
});
it("keeps Approve/Reject pinned outside a scrolling list on a long batch (16 changes)", () => {
const many: PendingAction = {
tool_uses: Array.from({ length: 16 }, (_, i) => ({
id: String(i),
name: "stock_items.create",
input: { name: `ITEM ${i}`, cost_price: 5.8, markup: 80 },
confirmAlways: false,
})),
completed: [],
order: Array.from({ length: 16 }, (_, i) => String(i)),
};
render(<ApprovalCard pending={many} busy={false} onDecide={() => {}} />);
const list = screen.getByTestId("approval-list");
const approve = screen.getByRole("button", { name: /approve/i });
const reject = screen.getByRole("button", { name: /reject/i });
// The list is the only scroller, and the buttons are not inside it …
expect(list.className).toContain("overflow-y-auto");
expect(list.contains(approve)).toBe(false);
expect(list.contains(reject)).toBe(false);
// … and they come BEFORE it, so a tall list can never push them off-screen.
expect(approve.compareDocumentPosition(list) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy();
expect(screen.getByText("16 changes", { exact: false })).toBeTruthy();
});
it("disables both buttons while busy", () => {
render(<ApprovalCard pending={pending} busy onDecide={() => {}} />);
expect((screen.getByRole("button", { name: /approve/i }) as HTMLButtonElement).disabled).toBe(true);

30
src/components/giellie/approval-card.tsx
View file

@ -12,11 +12,23 @@ interface Props {
export function ApprovalCard({ pending, busy, onDecide }: Props) {
return (
<div className="rounded-lg border border-amber-400/60 bg-amber-50 dark:bg-amber-950/30 p-3 space-y-3">
<p className="text-sm font-medium">
Giellie wants to make {pending.tool_uses.length === 1 ? "this change" : `${pending.tool_uses.length} changes`}:
</p>
<ul className="space-y-2">
// Bounded height + pinned header: with a long batch (e.g. a 16-item price
// list) the Approve/Reject buttons must stay on screen while only the list
// of changes scrolls. The card sits outside the message scroller, so an
// unbounded card used to push its own buttons out of the clipped page.
<div className="flex max-h-[60dvh] flex-col rounded-lg border border-amber-400/60 bg-amber-50 dark:bg-amber-950/30">
<div className="flex shrink-0 flex-wrap items-center gap-2 border-b border-amber-400/40 p-3">
<p className="mr-auto text-sm font-medium">
Giellie wants to make {pending.tool_uses.length === 1 ? "this change" : `${pending.tool_uses.length} changes`}:
</p>
<Button size="sm" disabled={busy} onClick={() => onDecide(true)}>
<Check className="mr-1 h-4 w-4" /> Approve
</Button>
<Button size="sm" variant="outline" disabled={busy} onClick={() => onDecide(false)}>
<X className="mr-1 h-4 w-4" /> Reject
</Button>
</div>
<ul data-testid="approval-list" className="min-h-0 flex-1 space-y-2 overflow-y-auto p-3">
{pending.tool_uses.map((tu) => (
<li key={tu.id} className="rounded-md border bg-background p-2">
<div className="flex items-center gap-2 text-sm font-mono font-semibold">
@ -33,14 +45,6 @@ export function ApprovalCard({ pending, busy, onDecide }: Props) {
</li>
))}
</ul>
<div className="flex gap-2">
<Button size="sm" disabled={busy} onClick={() => onDecide(true)}>
<Check className="mr-1 h-4 w-4" /> Approve
</Button>
<Button size="sm" variant="outline" disabled={busy} onClick={() => onDecide(false)}>
<X className="mr-1 h-4 w-4" /> Reject
</Button>
</div>
</div>
);
}