Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions apps/studio/src/client/atoms/studio-modal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,14 @@ const openModalAtom = atom<null | {
state: unknown;
}>(null);

/**
* Whether any app-wide studio modal holds the slot. Read by hosts of a
* body-mounted browser guest, which is not inside the dialog's subtree and so
* would otherwise keep painting straight through its overlay (see
* use-guest-covered).
*/
export const isStudioModalOpenAtom = atom((get) => get(openModalAtom) !== null);

type StudioModalAtom<T> = WritableAtom<null | T, [null | T], void>;

/**
Expand Down
43 changes: 34 additions & 9 deletions apps/studio/src/client/components/task/browser-panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
import { useIsActiveTab } from "@/client/hooks/use-active-tab";
import { useBrowserFind } from "@/client/hooks/use-browser-find";
import { useBrowserSlot } from "@/client/hooks/use-browser-slot";
import { useIsGuestCovered } from "@/client/hooks/use-guest-covered";
import { getWebviewElement } from "@/client/lib/browser-pool";
import {
EMULATED_DEVICES,
Expand Down Expand Up @@ -104,17 +105,32 @@ export function TaskBrowserPanel({
// Set when a main-frame navigation fails (bad host, no network, ...). The
// guest is parked and we show a light error state over the slot instead of its
// blank error page. Cleared when a new load starts or succeeds.
const [loadError, setLoadError] = useState<null | {
//
// Stamped with the guest it happened on, and read back only for that one:
// `targetId` changes in place when the selected session changes, with no
// remount, so a bare error would survive into the next session's guest and
// both park it behind a notice and name the previous session's URL. Filtered
// on read rather than cleared in an effect, so it costs no extra render and
// no failed page is briefly shown as fine.
const [failure, setFailure] = useState<null | {
message: string;
targetId: BrowserTargetId;
url: string;
}>(null);
const loadError = failure?.targetId === targetId ? failure : null;
// While the user is editing the URL, agent-driven navigations must not
// overwrite what they're typing.
const editingUrlRef = useRef(false);

const find = useBrowserFind({ active, isActiveTab, targetId });
// Read once and give both hooks the same answer: a panel that parks its guest
// under an overlay must also stop being the Cmd+F target, or the overlay's
// own host claims the single find-opener slot, clears it on unmount, and this
// panel never re-registers.
const covered = useIsGuestCovered();
const find = useBrowserFind({ active, covered, isActiveTab, targetId });
const slotRef = useBrowserSlot({
active,
covered,
emulatedDeviceHeight: emulatedDevice?.height,
emulatedDeviceWidth: emulatedDevice?.width,
hasLoadError: Boolean(loadError),
Expand Down Expand Up @@ -165,22 +181,31 @@ export function TaskBrowserPanel({
// Not attached yet; a did-navigate will re-run sync once it is.
}
};
// Clear only a failure stamped with this listener's own guest. These
// listeners outlive `targetId` changing in place by the frame between the
// render and the effect teardown, and a bare clear arriving from the
// previous guest in that window would drop the current guest's error
// notice and unpark it over the slot. The other state these handlers write
// needs no such guard: the re-run's own `sync()` follows the teardown.
const clearFailure = () => {
setFailure((current) =>
current?.targetId === targetId ? null : current,
);
};
const onNavigate = () => {
setLoadError(null);
clearFailure();
sync();
};
const onStartLoading = () => {
setLoadError(null);
};
const onFailLoad = (event: Event) => {
const detail = event as DidFailLoadEvent;
// Ignore sub-frame failures and user-aborted navigations (ERR_ABORTED),
// which fire routinely when a new navigation supersedes an in-flight one.
if (!detail.isMainFrame || detail.errorCode === -3) {
return;
}
setLoadError({
setFailure({
message: detail.errorDescription || "This site can’t be reached",
targetId,
url: detail.validatedURL,
});
if (!editingUrlRef.current && detail.validatedURL) {
Expand All @@ -198,12 +223,12 @@ export function TaskBrowserPanel({
sync();
webview.addEventListener("did-navigate", onNavigate);
webview.addEventListener("did-navigate-in-page", onNavigate);
webview.addEventListener("did-start-loading", onStartLoading);
webview.addEventListener("did-start-loading", clearFailure);
webview.addEventListener("did-fail-load", onFailLoad);
return () => {
webview.removeEventListener("did-navigate", onNavigate);
webview.removeEventListener("did-navigate-in-page", onNavigate);
webview.removeEventListener("did-start-loading", onStartLoading);
webview.removeEventListener("did-start-loading", clearFailure);
webview.removeEventListener("did-fail-load", onFailLoad);
};
}, [active, targetId]);
Expand Down
11 changes: 9 additions & 2 deletions apps/studio/src/client/hooks/use-browser-find.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,17 @@ interface FoundInPageEvent extends Event {
*/
export function useBrowserFind({
active,
covered = false,
isActiveTab,
targetId,
}: {
active: boolean;
// This host is behind a full-window overlay, so it is not the one Cmd+F
// should reach. The opener is a single slot: without this, a host that parks
// its guest keeps claiming it, and an overlay that takes the slot and clears
// it on unmount leaves the panel -- whose own inputs never changed -- never
// re-registering, so Cmd+F stops working entirely.
covered?: boolean;
isActiveTab: boolean;
targetId: BrowserTargetId;
}) {
Expand Down Expand Up @@ -71,15 +78,15 @@ export function useBrowserFind({
// the Cmd+F app command opens (and re-focuses) its find bar. See
// browser-find-registry for why Cmd+F can't be a renderer keydown.
useEffect(() => {
if (!active || !isActiveTab) {
if (!active || !isActiveTab || covered) {
return;
}
return setBrowserFindOpener(() => {
setFindOpen(true);
findInputRef.current?.focus();
findInputRef.current?.select();
});
}, [active, isActiveTab]);
}, [active, covered, isActiveTab]);

// Focus the find input when the bar opens (its first render, when the opener
// above couldn't focus it yet). Deferred a frame so it wins over Radix
Expand Down
10 changes: 9 additions & 1 deletion apps/studio/src/client/hooks/use-browser-slot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,20 @@ interface DeviceEmulation {
*/
export function useBrowserSlot({
active,
covered = false,
emulatedDeviceHeight,
emulatedDeviceWidth,
hasLoadError,
isActiveTab,
targetId,
}: {
active: boolean;
// A full-window overlay is drawn over this slot. The guest is body-mounted,
// outside every dialog's subtree, so no overlay occludes it -- it keeps
// painting over the dim layer as though nothing opened. Opening a dialog is
// also not a tab switch, which is the only park signal this hook otherwise
// gets, so a covered host has to say so itself.
covered?: boolean;
// Device size to emulate (see emulated-devices.ts presets), or
// null/undefined for the panel's natural size. Passed as separate
// primitives rather than an object so a new object identity per render
Expand Down Expand Up @@ -64,7 +71,7 @@ export function useBrowserSlot({

// On a load error, park the guest so its blank error page doesn't cover our
// own error state rendered in the slot.
if (!isActiveTab || hasLoadError) {
if (!isActiveTab || hasLoadError || covered) {
setPaintHost(targetId, slotOwner);
syncEmulation(null);
return;
Expand Down Expand Up @@ -139,6 +146,7 @@ export function useBrowserSlot({
};
}, [
active,
covered,
isActiveTab,
hasLoadError,
slotOwner,
Expand Down
31 changes: 31 additions & 0 deletions apps/studio/src/client/hooks/use-guest-covered.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { commandMenuOpenAtom } from "@/client/atoms/command-menu";
import { filePreviewAtom } from "@/client/atoms/file-preview";
import { isStudioModalOpenAtom } from "@/client/atoms/studio-modal";
import { taskFileViewerAtom } from "@/client/atoms/task-file-viewer";
import { useAtomValue } from "jotai";

/**
* Whether a full-window overlay is currently drawn over the page, for a host
* that shows a browser guest.
*
* The guest is mounted on `document.body`, outside every dialog's subtree, so no
* overlay occludes it the way it occludes ordinary content -- it keeps painting
* over the dim layer as though nothing opened. Opening a dialog is also not a
* tab switch, which is the only park signal `useBrowserSlot` otherwise receives,
* so a covered host has to say so itself.
*
* All four sources, because they are independent slots and a menu accelerator
* can open one over another: the app-wide studio modals (settings, sign-in,
* ...), the command palette (a dialog of the same shape, but on its own atom
* rather than the studio-modal slot), the file viewer's expand modal, and the
* chat's image/diagram preview.
*/
export function useIsGuestCovered(): boolean {
const studioModalOpen = useAtomValue(isStudioModalOpenAtom);
const commandMenuOpen = useAtomValue(commandMenuOpenAtom);
const fileViewerModalOpen = useAtomValue(taskFileViewerAtom).isModalOpen;
const filePreviewOpen = useAtomValue(filePreviewAtom).isOpen;
return (
studioModalOpen || commandMenuOpen || fileViewerModalOpen || filePreviewOpen
);
}
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.