Skip to content
Closed
Show file tree
Hide file tree
Changes from 10 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
2 changes: 1 addition & 1 deletion apps/desktop/src/app/DesktopLifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ describe("DesktopLifecycle", () => {
handleBackendNotReady: Effect.void,
flushMainWindowBounds: Effect.void,
dispatchMenuAction: () => Effect.void,
zoomMain: () => Effect.void,
zoomFocused: () => Effect.void,
syncAppearance: Effect.void,
});

Expand Down
2 changes: 1 addition & 1 deletion apps/desktop/src/backend/DesktopBackendPool.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ function makePoolLayer(
handleBackendNotReady: Effect.void,
flushMainWindowBounds: Effect.void,
dispatchMenuAction: () => Effect.die("unexpected menu action"),
zoomMain: () => Effect.die("unexpected zoom"),
zoomFocused: () => Effect.die("unexpected focused zoom"),
syncAppearance: Effect.void,
} satisfies DesktopWindow.DesktopWindow["Service"]),
),
Expand Down
69 changes: 66 additions & 3 deletions apps/desktop/src/preview/Manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,31 @@ describe("isPreviewRefreshShortcut", () => {
});
});

describe("getPreviewZoomShortcutDirection", () => {
const input = (overrides: Partial<Electron.Input> = {}) =>
({
type: "keyDown",
key: "-",
meta: true,
control: false,
shift: false,
alt: false,
...overrides,
}) as Electron.Input;

it("recognizes standard zoom chords without matching modified variants", () => {
expect(PreviewManager.getPreviewZoomShortcutDirection(input())).toBe("out");
expect(PreviewManager.getPreviewZoomShortcutDirection(input({ key: "=" }))).toBe("in");
expect(PreviewManager.getPreviewZoomShortcutDirection(input({ key: "+", shift: true }))).toBe(
"in",
);
expect(PreviewManager.getPreviewZoomShortcutDirection(input({ key: "0" }))).toBe("reset");
expect(PreviewManager.getPreviewZoomShortcutDirection(input({ alt: true }))).toBeNull();
expect(PreviewManager.getPreviewZoomShortcutDirection(input({ meta: false }))).toBeNull();
expect(PreviewManager.getPreviewZoomShortcutDirection(input({ type: "keyUp" }))).toBeNull();
});
});

const {
browserWindowConstructor,
createFromPath,
Expand Down Expand Up @@ -160,6 +185,8 @@ interface TestCapturedPreviewImage {
const makeTestPreviewWebContents = (
capturePage: () => Promise<TestCapturedPreviewImage>,
id = 42,
listeners?: Map<string, (...args: unknown[]) => void>,
setZoomFactor = vi.fn(),
) =>
({
id,
Expand All @@ -169,9 +196,13 @@ const makeTestPreviewWebContents = (
getTitle: () => "Example",
isLoading: () => false,
getZoomFactor: () => 1,
setZoomFactor: vi.fn(),
on: vi.fn(),
off: vi.fn(),
setZoomFactor,
on: vi.fn((event: string, listener: (...args: unknown[]) => void) => {
listeners?.set(event, listener);
}),
off: vi.fn((event: string, listener: (...args: unknown[]) => void) => {
if (listeners?.get(event) === listener) listeners.delete(event);
}),
ipc: { on: vi.fn(), off: vi.fn() },
send: webviewSend,
navigationHistory: { canGoBack: () => false, canGoForward: () => false },
Expand Down Expand Up @@ -476,6 +507,38 @@ describe("PreviewManager", () => {
),
);

effectIt.effect("routes menu zoom from focus events instead of stale global focus", () =>
withManager((manager) =>
Effect.gen(function* () {
const listeners = new Map<string, (...args: unknown[]) => void>();
const setZoomFactor = vi.fn();
const previewWebContents = makeTestPreviewWebContents(
async () => ({
toJPEG: () => Buffer.from("unused"),
getSize: () => ({ width: 1, height: 1 }),
}),
42,
listeners,
setZoomFactor,
);
fromId.mockReturnValue(previewWebContents);

yield* manager.createTab("tab_focus_zoom");
yield* manager.registerWebview("tab_focus_zoom", 42);

listeners.get("focus")?.();
expect(yield* manager.zoomFocusedPreview("out")).toBe(true);
expect(setZoomFactor).toHaveBeenCalledOnce();
expect(setZoomFactor).toHaveBeenCalledWith(0.9);

getFocusedWebContents.mockReturnValue(previewWebContents);
listeners.get("blur")?.();
expect(yield* manager.zoomFocusedPreview("out")).toBe(false);
expect(setZoomFactor).toHaveBeenCalledOnce();
}),
),
);

effectIt.effect("emulates prefers-color-scheme and re-applies it across webview swaps", () =>
withManager((manager) =>
Effect.gen(function* () {
Expand Down
102 changes: 83 additions & 19 deletions apps/desktop/src/preview/Manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,18 @@ export const isPreviewRefreshShortcut = (input: Electron.Input): boolean =>
!input.shift &&
!input.alt;

export type PreviewZoomShortcutDirection = "in" | "out" | "reset";

export const getPreviewZoomShortcutDirection = (
input: Electron.Input,
): PreviewZoomShortcutDirection | null => {
if (input.type !== "keyDown" || (!input.meta && !input.control) || input.alt) return null;
if (input.key === "+" || input.key === "=") return "in";
if (!input.shift && input.key === "-") return "out";
if (!input.shift && input.key === "0") return "reset";
return null;
};

const isPreviewInputSignal = (value: unknown): value is PreviewInputSignal => {
if (typeof value !== "object" || value === null || !("kind" in value)) return false;
if (value.kind === "pointer") {
Expand Down Expand Up @@ -504,6 +516,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
{ readonly semaphore: Semaphore.Semaphore; users: number }
>();
const tabLifecycleGenerations = new Map<string, number>();
let focusedPreviewTabId: string | null = null;

const attempt = <A>(errorContext: PreviewOperationContext, evaluate: () => A) =>
Effect.try({
Expand Down Expand Up @@ -1263,6 +1276,45 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
);
});

const applyZoom = Effect.fn("PreviewManager.applyZoom")(function* (
tabId: string,
transform: (current: number) => number,
) {
yield* withTabLifecycleLock(
tabId,
Effect.gen(function* () {
const tab = (yield* SynchronizedRef.get(tabsRef)).get(tabId);
if (!tab) return;
const next = transform(tab.zoomFactor);
if (Math.abs(next - tab.zoomFactor) < ZOOM_EPSILON) return;
if (tab.webContentsId != null) {
const wc = webContents.fromId(tab.webContentsId);
if (wc && !wc.isDestroyed()) {
yield* attempt({ operation: "applyZoom", tabId, webContentsId: wc.id }, () =>
wc.setZoomFactor(next),
);
}
}
yield* update(tabId, { zoomFactor: next });
}),
);
});

const zoomFocusedPreview = Effect.fn("PreviewManager.zoomFocusedPreview")(function* (
direction: PreviewZoomShortcutDirection,
) {
const tabId = focusedPreviewTabId;
if (tabId === null || !(yield* SynchronizedRef.get(tabsRef)).has(tabId)) {
focusedPreviewTabId = null;
return false;
}

yield* applyZoom(tabId, (current) =>
direction === "reset" ? DEFAULT_ZOOM_FACTOR : nextZoomLevel(current, direction),
);
return true;
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
});
Comment thread
SunkenInTime marked this conversation as resolved.

const attachListeners = Effect.fn("PreviewManager.attachListeners")(function* (
tabId: string,
wc: Electron.WebContents,
Expand Down Expand Up @@ -1382,17 +1434,36 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
);
return;
}
const zoomDirection = getPreviewZoomShortcutDirection(input);
if (zoomDirection !== null) {
event.preventDefault();
runFork(
applyZoom(tabId, (current) =>
zoomDirection === "reset" ? DEFAULT_ZOOM_FACTOR : nextZoomLevel(current, zoomDirection),
),
);
return;
}
runFork(forwardShortcut(event, input));
};
const focused = () => {
focusedPreviewTabId = tabId;
};
const blurred = () => {
if (focusedPreviewTabId === tabId) focusedPreviewTabId = null;
};
yield* Scope.addFinalizer(
scope,
attempt({ operation: "detachListeners", tabId, webContentsId: wc.id }, () => {
blurred();
Comment thread
cursor[bot] marked this conversation as resolved.
Outdated
wc.off("did-navigate", syncNavigation);
wc.off("did-navigate-in-page", syncNavigation);
wc.off("page-title-updated", sync);
wc.off("did-start-loading", sync);
wc.off("did-stop-loading", sync);
wc.off("did-fail-load", failed as never);
wc.off("focus", focused);
wc.off("blur", blurred);
wc.off("before-input-event", beforeInput);
wc.ipc.off(HUMAN_INPUT_CHANNEL, humanInput);
}).pipe(Effect.ignore),
Expand All @@ -1405,6 +1476,8 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
wc.on("did-start-loading", sync);
wc.on("did-stop-loading", sync);
wc.on("did-fail-load", failed as never);
wc.on("focus", focused);
wc.on("blur", blurred);
wc.ipc.on(HUMAN_INPUT_CHANNEL, humanInput);
wc.setWindowOpenHandler(({ url }) => {
runFork(
Expand All @@ -1428,8 +1501,13 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
const setMainWindow = Effect.fn("PreviewManager.setMainWindow")(function* (
window: BrowserWindow,
) {
const focused = () => {
focusedPreviewTabId = null;
};
yield* Ref.set(mainWindowRef, Option.some(window));
window.webContents.on("focus", focused);
window.once("closed", () => {
focused();
runFork(closeAllPictureInPicture());
});
});
Expand Down Expand Up @@ -1916,25 +1994,6 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
);
});

const applyZoom = Effect.fn("PreviewManager.applyZoom")(function* (
tabId: string,
transform: (current: number) => number,
) {
const tab = (yield* SynchronizedRef.get(tabsRef)).get(tabId);
if (!tab) return;
const next = transform(tab.zoomFactor);
if (Math.abs(next - tab.zoomFactor) < ZOOM_EPSILON) return;
if (tab.webContentsId != null) {
const wc = webContents.fromId(tab.webContentsId);
if (wc && !wc.isDestroyed()) {
yield* attempt({ operation: "applyZoom", tabId, webContentsId: wc.id }, () =>
wc.setZoomFactor(next),
);
}
}
yield* update(tabId, { zoomFactor: next });
});

// Emulated media lives on the CDP debugger session, not the WebContents, so
// it is lost whenever the session detaches (webview swap, DevTools
// open/close) and must be re-applied after every (re)attach.
Expand Down Expand Up @@ -3309,6 +3368,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
subscribeRecordingFrames: (listener: RecordingFrameListener) =>
subscribe(recordingFrameListenersRef, listener),
subscribeStateChanges: (listener: Listener) => subscribe(listenersRef, listener),
zoomFocusedPreview,
zoomIn: (tabId: string) => applyZoom(tabId, (current) => nextZoomLevel(current, "in")),
zoomOut: (tabId: string) => applyZoom(tabId, (current) => nextZoomLevel(current, "out")),
};
Expand Down Expand Up @@ -3588,6 +3648,9 @@ export class PreviewManager extends Context.Service<
readonly goBack: (tabId: string) => Effect.Effect<void, PreviewManagerError>;
readonly goForward: (tabId: string) => Effect.Effect<void, PreviewManagerError>;
readonly refresh: (tabId: string) => Effect.Effect<void, PreviewManagerError>;
readonly zoomFocusedPreview: (
direction: PreviewZoomShortcutDirection,
) => Effect.Effect<boolean, PreviewManagerError>;
readonly zoomIn: (tabId: string) => Effect.Effect<void, PreviewManagerError>;
readonly zoomOut: (tabId: string) => Effect.Effect<void, PreviewManagerError>;
readonly resetZoom: (tabId: string) => Effect.Effect<void, PreviewManagerError>;
Expand Down Expand Up @@ -3688,6 +3751,7 @@ export const make = Effect.gen(function* PreviewManagerMake() {
goBack: operations.goBack,
goForward: operations.goForward,
refresh: operations.refresh,
zoomFocusedPreview: operations.zoomFocusedPreview,
zoomIn: operations.zoomIn,
zoomOut: operations.zoomOut,
resetZoom: operations.resetZoom,
Expand Down
7 changes: 2 additions & 5 deletions apps/desktop/src/window/DesktopApplicationMenu.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ const makeDesktopWindowLayer = (selectedAction: Deferred.Deferred<string>) =>
handleBackendNotReady: Effect.void,
flushMainWindowBounds: Effect.void,
dispatchMenuAction: (action) => Deferred.succeed(selectedAction, action).pipe(Effect.asVoid),
zoomMain: (direction) =>
zoomFocused: (direction) =>
Deferred.succeed(selectedAction, `zoom-${direction}`).pipe(Effect.asVoid),
syncAppearance: Effect.void,
} satisfies DesktopWindow.DesktopWindow["Service"]);
Expand Down Expand Up @@ -146,10 +146,7 @@ describe("DesktopApplicationMenu", () => {
}),
);

// Zoom must route through DesktopWindow.zoomMain instead of the Electron
// zoom roles: the roles zoom whichever webContents has focus, which breaks
// app zoom while an embedded preview WebContentsView holds focus.
it.effect("routes View menu zoom to the main window instead of zoom roles", () =>
it.effect("routes zoom to the focused surface without native zoom roles", () =>
Effect.gen(function* () {
const selectedAction = yield* Deferred.make<string>();
const applicationMenuTemplate =
Expand Down
16 changes: 6 additions & 10 deletions apps/desktop/src/window/DesktopApplicationMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ const dispatchMenuAction = Effect.fn("desktop.menu.dispatchMenuAction")(function
yield* desktopWindow.dispatchMenuAction(action);
});

const zoomMainWindow = Effect.fn("desktop.menu.zoomMainWindow")(function* (
const zoomFocusedSurface = Effect.fn("desktop.menu.zoomFocusedSurface")(function* (
direction: DesktopWindow.MainWindowZoomDirection,
): Effect.fn.Return<void, never, DesktopWindow.DesktopWindow> {
): Effect.fn.Return<void, DesktopWindow.DesktopWindowError, DesktopWindow.DesktopWindow> {
const desktopWindow = yield* DesktopWindow.DesktopWindow;
yield* desktopWindow.zoomMain(direction);
yield* desktopWindow.zoomFocused(direction);
});

const checkForUpdatesFromMenu = Effect.gen(function* () {
Expand Down Expand Up @@ -135,7 +135,7 @@ export const make = Effect.gen(function* () {
runMenuEffect("open-settings", dispatchMenuAction("open-settings"));
};
const zoomClick = (direction: DesktopWindow.MainWindowZoomDirection) => () => {
runMenuEffect(`zoom-${direction}`, zoomMainWindow(direction));
runMenuEffect(`zoom-${direction}`, zoomFocusedSurface(direction));
};
const template: Electron.MenuItemConstructorOptions[] = [];

Expand Down Expand Up @@ -191,12 +191,8 @@ export const make = Effect.gen(function* () {
{ role: "forceReload" },
{ role: "toggleDevTools" },
{ type: "separator" },
/*
Not the zoom roles: those act on the focused webContents, so with
an embedded preview WebContentsView focused they zoom the guest
page and the app UI appears stuck. These always zoom the main
window (see DesktopWindow.zoomMain).
*/
// Do not use Electron's zoom roles: route explicitly to the managed
// preview or the T3 window based on which surface owns focus.
{ label: "Actual Size", accelerator: "CmdOrCtrl+0", click: zoomClick("reset") },
{ label: "Zoom In", accelerator: "CmdOrCtrl+=", click: zoomClick("in") },
{
Expand Down
29 changes: 29 additions & 0 deletions apps/desktop/src/window/DesktopWindow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,35 @@ describe("DesktopWindow", () => {
);
});

it.effect("starts preview webviews at 100% independently of the app zoom", () =>
Effect.gen(function* () {
const fakeWindow = makeFakeBrowserWindow();
const createCount = yield* Ref.make(0);
const mainWindow = yield* Ref.make<Option.Option<Electron.BrowserWindow>>(Option.none());
const layer = makeTestLayer({
window: fakeWindow.window,
createCount,
mainWindow,
});

yield* Effect.gen(function* () {
const desktopWindow = yield* DesktopWindow.DesktopWindow;
yield* desktopWindow.handleBackendReady(new URL("http://127.0.0.1:3773"));

const willAttachWebview = fakeWindow.webContentsListeners.get("will-attach-webview");
if (!willAttachWebview) {
return yield* Effect.die("will-attach-webview listener was not registered");
}
const webPreferences = { zoomFactor: 0.8 } as Electron.WebPreferences;
willAttachWebview({ preventDefault: vi.fn() }, webPreferences, {
partition: "persist:t3code-preview-test",
});

assert.equal(webPreferences.zoomFactor, 1);
}).pipe(Effect.provide(layer));
}),
);

it.effect("does not open a development window until the backend is ready", () =>
Effect.gen(function* () {
const fakeWindow = makeFakeBrowserWindow();
Expand Down
Loading
Loading