fix(lanes): don't auto-close Add Lane modal after setup summary
The 3s auto-close timer closed before a user reasonably had time to look at the setup results, making the feature appear to do nothing. Require an explicit dismiss (Cancel/X) instead.
This commit is contained in:
@@ -58,25 +58,6 @@ export function AddLaneModal({
|
||||
setSetupResult(null);
|
||||
};
|
||||
|
||||
// The setup summary (below) is shown for a few seconds before the modal
|
||||
// auto-closes, so the user actually sees whether profile/agents/mcp
|
||||
// succeeded instead of the modal vanishing the instant the lane exists.
|
||||
const closeTimerRef = useRef<number | null>(null);
|
||||
const finishAndClose = useCallback(() => {
|
||||
if (closeTimerRef.current !== null) {
|
||||
window.clearTimeout(closeTimerRef.current);
|
||||
closeTimerRef.current = null;
|
||||
}
|
||||
reset();
|
||||
onClose();
|
||||
}, [onClose]);
|
||||
useEffect(
|
||||
() => () => {
|
||||
if (closeTimerRef.current !== null) window.clearTimeout(closeTimerRef.current);
|
||||
},
|
||||
[]
|
||||
);
|
||||
|
||||
// Look up the repo's branches once the path settles - debounced so every
|
||||
// keystroke while typing a path doesn't fire a request against a path that
|
||||
// isn't finished yet.
|
||||
@@ -147,9 +128,11 @@ export function AddLaneModal({
|
||||
});
|
||||
}
|
||||
|
||||
// Leave the modal open so the setup summary below stays on screen; the
|
||||
// user dismisses it themselves (Cancel/X) once they've seen it, rather
|
||||
// than racing a timer that can close before they've looked at it.
|
||||
setBusy(false);
|
||||
onAdded(result.lane);
|
||||
closeTimerRef.current = window.setTimeout(finishAndClose, 3000);
|
||||
} catch (err) {
|
||||
setError(err instanceof Error ? err.message : String(err));
|
||||
setBusy(false);
|
||||
@@ -163,8 +146,9 @@ export function AddLaneModal({
|
||||
// into after the very first character. useCallback keeps the identity stable
|
||||
// across renders so only mount/unmount (and a real onClose change) refocuses.
|
||||
const handleCancel = useCallback(() => {
|
||||
finishAndClose();
|
||||
}, [finishAndClose]);
|
||||
reset();
|
||||
onClose();
|
||||
}, [onClose]);
|
||||
|
||||
return (
|
||||
<ConfirmModal
|
||||
|
||||
@@ -158,8 +158,9 @@ describe("AddLaneModal", () => {
|
||||
expect(onAdded).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ id: 9, status: "provisioning" })
|
||||
);
|
||||
// The setup summary stays on screen for a few seconds before auto-closing.
|
||||
await waitFor(() => expect(onClose).toHaveBeenCalled(), { timeout: 4000 });
|
||||
// The modal stays open showing the setup summary until dismissed - it
|
||||
// does not close itself just because the lane was added.
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("shows a server error and leaves the modal open instead of closing silently", async () => {
|
||||
@@ -208,7 +209,7 @@ describe("AddLaneModal", () => {
|
||||
await waitFor(() => expect(onAdded).toHaveBeenCalled());
|
||||
});
|
||||
|
||||
it("still calls onAdded and closes even when every setup call fails", async () => {
|
||||
it("still calls onAdded even when every setup call fails", async () => {
|
||||
vi.mocked(api.lanes.branches).mockResolvedValue({ branches: ["main"], current: "main" });
|
||||
vi.mocked(api.lanes.worktree).mockResolvedValue({
|
||||
lane: { id: 43, title: "demo2", cwd: "/lanes/demo2", status: "provisioning" } as Lane,
|
||||
@@ -229,10 +230,10 @@ describe("AddLaneModal", () => {
|
||||
await user.click(screen.getByRole("button", { name: "Add lane" }));
|
||||
|
||||
await waitFor(() => expect(onAdded).toHaveBeenCalled());
|
||||
await waitFor(() => expect(onClose).toHaveBeenCalled(), { timeout: 4000 });
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("shows the setup summary and lets the user dismiss it early instead of waiting out the auto-close timer", async () => {
|
||||
it("shows the setup summary and lets the user dismiss it manually", async () => {
|
||||
vi.mocked(api.lanes.branches).mockResolvedValue({ branches: ["main"], current: "main" });
|
||||
vi.mocked(api.lanes.worktree).mockResolvedValue({
|
||||
lane: { id: 44, title: "demo3", cwd: "/lanes/demo3", status: "provisioning" } as Lane,
|
||||
|
||||
Reference in New Issue
Block a user