Skip to content
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.

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

Add zoom slider, match official client zoom #370

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
4 changes: 4 additions & 0 deletions src/main/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ handle(IpcEvents.MAXIMIZE, e => {
}
});

handle(IpcEvents.SET_ZOOM, (e, zoom: number) => {
mainWin.webContents.setZoomFactor(zoom);
});

handleSync(IpcEvents.SPELLCHECK_GET_AVAILABLE_LANGUAGES, e => {
e.returnValue = session.defaultSession.availableSpellCheckerLanguages;
});
Expand Down
73 changes: 66 additions & 7 deletions src/main/mainWindow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
BrowserWindow,
BrowserWindowConstructorOptions,
dialog,
globalShortcut,
Menu,
MenuItemConstructorOptions,
nativeTheme,
Expand Down Expand Up @@ -235,14 +236,14 @@ function initMenuBar(win: BrowserWindow) {
click() {
app.quit();
}
},
// See https://github.com/electron/electron/issues/14742 and https://github.com/electron/electron/issues/5256
{
label: "Zoom in (hidden, hack for Qwertz and others)",
accelerator: "CmdOrCtrl+=",
role: "zoomIn",
visible: false
}
// See https://github.com/electron/electron/issues/14742 and https://github.com/electron/electron/issues/5256
// {
// label: "Zoom in (hidden, hack for Qwertz and others)",
// accelerator: "CmdOrCtrl+=",
// role: "zoomIn",
// visible: false
// }
] satisfies MenuItemList;

const menu = Menu.buildFromTemplate([
Expand Down Expand Up @@ -467,6 +468,36 @@ function createMainWindow() {

const runVencordMain = once(() => require(join(VENCORD_FILES_DIR, "vencordDesktopMain.js")));

const allowedZoomFactors = [0.5, 0.67, 0.75, 0.8, 0.9, 1, 1.1, 1.25, 1.5, 1.75, 2];

function handleZoomIn() {
const zoomFactor = Settings.store.zoomFactor ?? 1;
const currentIndex = allowedZoomFactors.indexOf(zoomFactor);
if (currentIndex < allowedZoomFactors.length - 1) {
const newZoomFactor = allowedZoomFactors[currentIndex + 1];
Settings.setData({ zoomFactor: newZoomFactor });
mainWin.webContents.setZoomFactor(newZoomFactor);
mainWin.webContents.send("zoomChanged", newZoomFactor);
}
}

function handleZoomOut() {
const zoomFactor = Settings.store.zoomFactor ?? 1;
const currentIndex = allowedZoomFactors.indexOf(zoomFactor);
Copy link
Member

Choose a reason for hiding this comment

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

if you just want to check for existence of an element, includes should be used over indexOf

Suggested change
const currentIndex = allowedZoomFactors.indexOf(zoomFactor);
const currentIndex = allowedZoomFactors.includes(zoomFactor);

if (currentIndex > 0) {
const newZoomFactor = allowedZoomFactors[currentIndex - 1];
Settings.setData({ zoomFactor: newZoomFactor });
Copy link
Member

@Vendicated Vendicated May 23, 2024

Choose a reason for hiding this comment

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

setData changes the entire underlying settings data. this will effectively reset all settings. what you instead want is to set the property on store

Suggested change
Settings.setData({ zoomFactor: newZoomFactor });
Settings.store.zoomFactor = newZoomFactor;

there are more locations with the same error, those also need fixing

mainWin.webContents.setZoomFactor(newZoomFactor);
mainWin.webContents.send("zoomChanged", newZoomFactor);
}
}

function resetZoom() {
Settings.setData({ zoomFactor: 1 });
mainWin.webContents.setZoomFactor(1);
mainWin.webContents.send("zoomChanged", 1);
}

export async function createWindows() {
const startMinimized = process.argv.includes("--start-minimized");
const splash = createSplashWindow(startMinimized);
Expand All @@ -480,6 +511,8 @@ export async function createWindows() {
mainWin.webContents.on("did-finish-load", () => {
splash.destroy();

mainWin.webContents.setZoomFactor(Settings.store.zoomFactor ?? 1);

if (!startMinimized) {
mainWin!.show();
if (State.store.maximized && !isDeckGameMode) mainWin!.maximize();
Expand All @@ -497,6 +530,32 @@ export async function createWindows() {
mainWin!.maximize();
}
});

mainWin.on("focus", () => {
Copy link
Member

Choose a reason for hiding this comment

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

this is a really bad way to do this. why not use menu items with accelerators like previously?

Copy link
Author

Choose a reason for hiding this comment

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

Because it refused to work with the menu items after hours of trying. If anyone is able to get it working that way instead that would be super helpful

Copy link
Member

Choose a reason for hiding this comment

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

it works very much the same. did you maybe forget to remove the default zoom handlers? they might take priority over your accelerators

globalShortcut.register("CommandOrControl+0", () => {
resetZoom();
});
globalShortcut.register("CommandOrControl+plus", () => {
handleZoomIn();
});
globalShortcut.register("CommandOrControl+=", () => {
handleZoomIn();
});
globalShortcut.register("CommandOrControl+-", () => {
handleZoomOut();
});
globalShortcut.register("CommandOrControl+_", () => {
handleZoomOut();
});
});

mainWin.on("blur", () => {
globalShortcut.unregister("CommandOrControl+0");
globalShortcut.unregister("CommandOrControl+plus");
globalShortcut.unregister("CommandOrControl+=");
globalShortcut.unregister("CommandOrControl+-");
globalShortcut.unregister("CommandOrControl+_");
});
});

// evil hack to fix electron 32 regression that makes devtools always light theme
Expand Down
3 changes: 2 additions & 1 deletion src/preload/VesktopNative.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ export const VesktopNative = {
focus: () => invoke<void>(IpcEvents.FOCUS),
close: (key?: string) => invoke<void>(IpcEvents.CLOSE, key),
minimize: () => invoke<void>(IpcEvents.MINIMIZE),
maximize: () => invoke<void>(IpcEvents.MAXIMIZE)
maximize: () => invoke<void>(IpcEvents.MAXIMIZE),
zoom: (zoom: number) => invoke<void>(IpcEvents.SET_ZOOM, zoom)
},
capturer: {
getLargeThumbnail: (id: string) => invoke<string>(IpcEvents.CAPTURER_GET_LARGE_THUMBNAIL, id)
Expand Down
4 changes: 4 additions & 0 deletions src/preload/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ require(ipcRenderer.sendSync(IpcEvents.GET_VENCORD_PRELOAD_FILE));
webFrame.executeJavaScript(ipcRenderer.sendSync(IpcEvents.GET_VENCORD_RENDERER_SCRIPT));
webFrame.executeJavaScript(ipcRenderer.sendSync(IpcEvents.GET_RENDERER_SCRIPT));

ipcRenderer.on("zoomChanged", (event, newZoomFactor) => {
window.dispatchEvent(new CustomEvent("zoomChanged", { detail: newZoomFactor }));
});

// #region css
const rendererCss = ipcRenderer.sendSync(IpcEvents.GET_RENDERER_CSS_FILE);

Expand Down
4 changes: 3 additions & 1 deletion src/renderer/components/settings/Settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { DiscordBranchPicker } from "./DiscordBranchPicker";
import { NotificationBadgeToggle } from "./NotificationBadgeToggle";
import { VencordLocationPicker } from "./VencordLocationPicker";
import { WindowsTransparencyControls } from "./WindowsTransparencyControls";
import { WindowZoom } from "./WindowZoom";

interface BooleanSetting {
key: keyof typeof Settings.store;
Expand Down Expand Up @@ -65,7 +66,8 @@ const SettingsOptions: Record<string, Array<BooleanSetting | SettingsComponent>>
description: "Adapt the splash window colors to your custom theme",
defaultValue: false
},
WindowsTransparencyControls
WindowsTransparencyControls,
WindowZoom
],
Behaviour: [
{
Expand Down
48 changes: 48 additions & 0 deletions src/renderer/components/settings/WindowZoom.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* SPDX-License-Identifier: GPL-3.0
* Vesktop, a desktop app aiming to give you a snappier Discord Experience
* Copyright (c) 2023 Vendicated and Vencord contributors
*/

import { Margins } from "@vencord/types/utils";
import { Forms, Slider, useEffect, useState } from "@vencord/types/webpack/common";

import { SettingsComponent } from "./Settings";

export const WindowZoom: SettingsComponent = ({ settings }) => {
const [zoomFactor, setZoomFactor] = useState(settings.zoomFactor ?? 1);

useEffect(() => {
const handleZoomChange = event => {
console.log("zoom changed", event.detail);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
console.log("zoom changed", event.detail);

setZoomFactor(event.detail);
};

window.addEventListener("zoomChanged", handleZoomChange);
Copy link
Member

Choose a reason for hiding this comment

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

you shouldn't need this event. changing settings will already update your state & rerender the ui

Copy link
Author

Choose a reason for hiding this comment

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

I removed the listener, and the UI element does not update when the zoom gets changed, even with a listener


return () => {
window.removeEventListener("zoomChanged", handleZoomChange);
};
}, []);

return (
<>
<Forms.FormTitle className={Margins.top16 + " " + Margins.bottom8}>Zoom Level</Forms.FormTitle>
<Slider
className={Margins.top20}
initialValue={zoomFactor}
defaultValue={1}
onValueChange={v => {
settings.zoomFactor = v;
VesktopNative.win.zoom(v);
Copy link
Member

Choose a reason for hiding this comment

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

you don't need this ipc call. you can register a settings change listener in the native process, look at the initSettingsListener function in mainWindow.ts

setZoomFactor(v);
Copy link
Member

Choose a reason for hiding this comment

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

you don't need this state. via useSettings(), you can get a stateful settings object that will automatically rerender your component on change

the parent component already uses this hook so you should not need to call it yourself

}}
minValue={0.5}
maxValue={2}
markers={[0.5, 0.67, 0.75, 0.8, 0.9, 1, 1.1, 1.25, 1.5, 1.75, 2]}
Copy link
Member

Choose a reason for hiding this comment

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

this constant seems duplicated across multiple files. for better maintainability, it should be moved to the shared/ folder

stickToMarkers={true}
onMarkerRender={v => (v === 1 ? "100" : `${Math.round(v * 100)}`)}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
onMarkerRender={v => (v === 1 ? "100" : `${Math.round(v * 100)}`)}
onMarkerRender={v => (v === 1 ? "100" : String(Math.round(v * 100)))}

></Slider>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
></Slider>
/>

</>
);
};
1 change: 1 addition & 0 deletions src/shared/IpcEvents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export const enum IpcEvents {
FOCUS = "VCD_FOCUS",
MINIMIZE = "VCD_MINIMIZE",
MAXIMIZE = "VCD_MAXIMIZE",
SET_ZOOM = "VCD_ZOOM",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
SET_ZOOM = "VCD_ZOOM",
SET_ZOOM = "VCD_SET_ZOOM",


SHOW_ITEM_IN_FOLDER = "VCD_SHOW_ITEM_IN_FOLDER",
GET_SETTINGS = "VCD_GET_SETTINGS",
Expand Down
1 change: 1 addition & 0 deletions src/shared/settings.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export interface Settings {
arRPC?: boolean;
appBadge?: boolean;
disableMinSize?: boolean;
zoomFactor?: number;
Copy link
Member

Choose a reason for hiding this comment

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

wouldn't this make a lot more sense in State than in Settings?

Copy link
Author

Choose a reason for hiding this comment

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

Then how would it persist after restarting?

Copy link
Member

Choose a reason for hiding this comment

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

state is also persisted!

clickTrayToShowHide?: boolean;
customTitleBar?: boolean;

Expand Down