Skip to content

feat(desktop): stop recording overlay and stable in-progress bounds #1704

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
richiemcilroy merged 2 commits into from
Apr 1, 2026
+73 −11
Merged

feat(desktop): stop recording overlay and stable in-progress bounds #1704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5dc076c
feat(desktop): stop recording overlay and stable in-progress bounds
richiemcilroy Mar 31, 2026
47b1f93
Merge branch 'main' into feat/desktop-recording-ux
richiemcilroy Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -979,6 +979,8 @@
const { rawOptions, setOptions } = useRecordingOptions();
const currentRecording = createCurrentRecordingQuery();
const isRecording = () => !!currentRecording.data;
const isActivelyRecording = () =>
currentRecording.data?.status === "recording";
const auth = authStore.createQuery();

const [hasHiddenMainWindowForPicker, setHasHiddenMainWindowForPicker] =
Expand Down Expand Up @@ -1508,6 +1510,20 @@
const license = createLicenseQuery();

const signIn = createSignInMutation();
const stopRecording = createMutation(() => ({
mutationFn: async () => {
try {
await commands.stopRecording();
} catch (error) {
await dialog.message(
`Failed to stop recording: ${
error instanceof Error ? error.message : String(error)
}`,
{ title: "Stop Recording", kind: "error" },
);
}
},
}));
Comment on lines +1513 to +1526
Copy link
Copy Markdown

@tembo tembo bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you rethrow after showing the dialog, the mutation's error state stays accurate (useful for telemetry / future UI) instead of always resolving successfully.

Suggested change
const stopRecording = createMutation(() => ({
mutationFn: async () => {
try {
await commands.stopRecording();
} catch (error) {
await dialog.message(
`Failed to stop recording: ${
error instanceof Error ? error.message : String(error)
}`,
{ title: "Stop Recording", kind: "error" },
);
}
},
}));
const stopRecording = createMutation(() => ({
mutationFn: async () => {
try {
await commands.stopRecording();
} catch (error) {
await dialog.message(
`Failed to stop recording: ${
error instanceof Error ? error.message : String(error)
}`,
{ title: "Stop Recording", kind: "error" },
);
throw error;
}
},
}));


const BaseControls = () => (
<div class="space-y-2">
Expand Down Expand Up @@ -1777,7 +1793,7 @@
<div class="flex items-center space-x-1">
<a
class="*:w-[92px] *:h-auto text-[--text-primary]"
target="_blank"

Check failure on line 1796 in apps/desktop/src/routes/(window-chrome)/new-main/index.tsx

View workflow job for this annotation

GitHub Actions / Lint (Biome)

lint/security/noBlankTarget 

Avoid using target="_blank" without rel="noopener" or rel="noreferrer".
href={
auth.data
? `${import.meta.env.VITE_SERVER_URL}/dashboard`
Expand Down Expand Up @@ -1994,6 +2010,26 @@
</Show>
</Show>
</div>
<Show when={isActivelyRecording()}>
<div class="absolute inset-0 z-10 flex flex-col justify-end bg-gray-1/80 px-6 pb-8 backdrop-blur-sm">
Copy link
Copy Markdown

@tembo tembo bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This overlay uses z-10; RecoveryToast doesn’t set a z-index and will likely render underneath it while recording. If the toast needs to stay actionable, consider bumping its z-index (or adjusting the overlay layering) so it isn’t blocked.

<div class="pointer-events-auto">
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Redundant pointer-events-auto class

The parent div has no pointer-events-none class, so it already captures all pointer events by default (Tailwind's base is pointer-events-auto). The inner wrapper's pointer-events-auto class has no effect. The typical pattern would be to mark the overlay pointer-events-none so it's transparent to clicks, and only apply pointer-events-auto to the interactive button area — but since the design intent here is to block all background interaction while recording, the outer div is correct as-is; the inner class is just redundant.

Suggested change
<div class="pointer-events-auto">
<div>
Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/desktop/src/routes/(window-chrome)/new-main/index.tsx
Line: 2015

Comment:
**Redundant `pointer-events-auto` class**

The parent `div` has no `pointer-events-none` class, so it already captures all pointer events by default (Tailwind's base is `pointer-events-auto`). The inner wrapper's `pointer-events-auto` class has no effect. The typical pattern would be to mark the overlay `pointer-events-none` so it's transparent to clicks, and only apply `pointer-events-auto` to the interactive button area — but since the design intent here is to block all background interaction while recording, the outer div is correct as-is; the inner class is just redundant.

```suggestion
					<div>
```

How can I resolve this? If you propose a fix, please make it concise.

<button
type="button"
disabled={stopRecording.isPending}
onClick={() => stopRecording.mutate()}
class="flex h-11 w-full items-center justify-center gap-2 rounded-xl bg-red-9 px-4 text-sm font-medium text-white transition hover:bg-red-10 disabled:cursor-not-allowed disabled:opacity-60"
>
<Show
when={!stopRecording.isPending}
fallback={<IconLucideLoader2 class="size-4 animate-spin" />}
>
<IconCapStopCircle class="size-4" />
</Show>
<span>Stop Recording</span>
</button>
</div>
</div>
</Show>
<RecoveryToast />
</div>
);
Expand Down
48 changes: 37 additions & 11 deletions apps/desktop/src/routes/in-progress-recording.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { createElementBounds } from "@solid-primitives/bounds";
import { createTimer } from "@solid-primitives/timer";
import { createMutation } from "@tanstack/solid-query";
import { LogicalPosition } from "@tauri-apps/api/dpi";
Expand Down Expand Up @@ -111,8 +110,8 @@ function InProgressRecordingInner() {
const [startingDismissed, setStartingDismissed] = createSignal(false);
const [interactiveAreaRef, setInteractiveAreaRef] =
createSignal<HTMLDivElement | null>(null);
const interactiveBounds = createElementBounds(interactiveAreaRef);
let settingsButtonRef: HTMLButtonElement | undefined;
let lastInteractiveBoundsKey = "";
const recordingMode = createMemo(
() => currentRecording.data?.mode ?? optionsQuery.rawOptions.mode,
);
Expand Down Expand Up @@ -292,30 +291,55 @@ function InProgressRecordingInner() {
void refreshCameraWindowState();
});

createEffect(() => {
const syncInteractiveAreaBounds = () => {
const element = interactiveAreaRef();
if (!element) {
void commands.removeFakeWindow(FAKE_WINDOW_BOUNDS_NAME);
if (lastInteractiveBoundsKey !== "") {
lastInteractiveBoundsKey = "";
void commands.removeFakeWindow(FAKE_WINDOW_BOUNDS_NAME);
}
return;
}

const left = interactiveBounds.left ?? 0;
const top = interactiveBounds.top ?? 0;
const width = interactiveBounds.width ?? 0;
const height = interactiveBounds.height ?? 0;
const rect = element.getBoundingClientRect();
if (rect.width === 0 || rect.height === 0) return;
Copy link
Copy Markdown

@tembo tembo bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the interactive area stays mounted but becomes non-visible (0x0 rect), we keep the last fake window bounds. Clearing the fake window here too might avoid stale hit targets.

Suggested change
if (rect.width === 0 || rect.height === 0) return;
const rect = element.getBoundingClientRect();
if (rect.width === 0 || rect.height === 0) {
if (lastInteractiveBoundsKey !== "") {
lastInteractiveBoundsKey = "";
void commands.removeFakeWindow(FAKE_WINDOW_BOUNDS_NAME);
}
return;
}


if (width === 0 || height === 0) return;
const key = [rect.left, rect.top, rect.width, rect.height]
.map((value) => value.toFixed(2))
.join(":");
if (key === lastInteractiveBoundsKey) return;

lastInteractiveBoundsKey = key;
void commands.setFakeWindowBounds(FAKE_WINDOW_BOUNDS_NAME, {
Copy link
Copy Markdown

@tembo tembo bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

void commands.setFakeWindowBounds(...) (and the removeFakeWindow call above) will surface as unhandled promise rejections if the backend fails. Since this runs frequently, it’s probably worth attaching a .catch(...) (even just console.error) so failures don’t spam/tear down the webview.

position: { x: left, y: top },
size: { width, height },
position: { x: rect.left, y: rect.top },
size: { width: rect.width, height: rect.height },
});
};

createEffect(() => {
interactiveAreaRef();
queueMicrotask(syncInteractiveAreaBounds);
});

createEffect(() => {
state();
issuePanelVisible();
queueMicrotask(syncInteractiveAreaBounds);
});

onCleanup(() => {
lastInteractiveBoundsKey = "";
void commands.removeFakeWindow(FAKE_WINDOW_BOUNDS_NAME);
});

onMount(() => {
const onResize = () => syncInteractiveAreaBounds();
window.addEventListener("resize", onResize);
onCleanup(() => window.removeEventListener("resize", onResize));
requestAnimationFrame(() => syncInteractiveAreaBounds());
setTimeout(() => syncInteractiveAreaBounds(), 150);
});
Comment on lines +335 to +341
Copy link
Copy Markdown

@tembo tembo bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requestAnimationFrame / setTimeout here aren't cleaned up; if this route unmounts quickly you can still call syncInteractiveAreaBounds() after cleanup.

Suggested change
onMount(() => {
const onResize = () => syncInteractiveAreaBounds();
window.addEventListener("resize", onResize);
onCleanup(() => window.removeEventListener("resize", onResize));
requestAnimationFrame(() => syncInteractiveAreaBounds());
setTimeout(() => syncInteractiveAreaBounds(), 150);
});
onMount(() => {
const onResize = () => syncInteractiveAreaBounds();
window.addEventListener("resize", onResize);
const rafId = requestAnimationFrame(() => syncInteractiveAreaBounds());
const timeoutId = window.setTimeout(() => syncInteractiveAreaBounds(), 150);
onCleanup(() => {
window.removeEventListener("resize", onResize);
cancelAnimationFrame(rafId);
clearTimeout(timeoutId);
});
});


createTimer(
() => {
void refreshCameraWindowState();
Expand All @@ -324,6 +348,8 @@ function InProgressRecordingInner() {
setInterval,
);

createTimer(syncInteractiveAreaBounds, 250, setInterval);
Copy link
Copy Markdown

@tembo tembo bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 250ms polling loop calls getBoundingClientRect() continuously, which can force layout. If you want the same stability without the steady cost, a ResizeObserver on interactiveAreaRef() + rAF-throttled sync (and/or window.visualViewport events) can replace the interval in most cases.


createEffect(() => {
if (
state().variant === "stopped" &&
Expand Down
Loading