Add zoom slider, match official client zoom #370

Kaydax · 2024-01-31T21:14:04Z

This code is really bad, so I expect many change request. However its working fine.

This adds a zoom slider to the Vesktop settings page as well as rebinds ctrl+/- to use these zoom controls instead of the built in electron zoom controls.

Vendicated · 2024-02-18T16:27:08Z

src/main/mainWindow.ts

@@ -470,6 +503,32 @@ export async function createWindows() {
                mainWin!.maximize();
            }
        });
+
+        mainWin.on("focus", () => {


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

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

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

Vendicated · 2024-02-18T16:27:53Z

i dont think this is very useful. why would anyone use this instead of just using zoom shortcuts?

sunnniee · 2024-02-18T16:37:03Z

i dont think this is very useful. why would anyone use this instead of just using zoom shortcuts?

because they simply don't know the zoom shortcuts exist. changing zoom is a reoccurring question in the support channel

Vendicated · 2024-02-18T16:43:21Z

people also constantly ask how to change vencord location in vesktop even tho its inside vesktop settings

people are simply stupid. you cant fix that by adding pointless settings

Kaydax · 2024-02-18T18:14:50Z

i dont think this is very useful. why would anyone use this instead of just using zoom shortcuts?

I explained this in the dev channel. The current zoom shortcut buttons do not match the same zoom scale as the official discord client, thus causing totally different zoom levels between the two. It's more of a OCD thing than anything, but also makes the client feel more like the official one. It also fixes issues with fonts looking off with the way the normal zoom works, as the default electron / chrome zoom have fractional zooming, which makes fonts look blurry on some zoom levels

TomTheDragon · 2024-03-18T12:10:51Z

The zoom shortcut does not seem to work on linux (Vesktop and/or official client)
So a slider like the official client has, seems to be the only way to get zoom functionality

D3SOX · 2024-03-18T17:13:13Z

@TomTheDragon I don't why it doesn't work for you but CTRL + 0 / - / + work fine for me on Linux with KDE Plasma

TomTheDragon · 2024-03-19T21:09:23Z

@TomTheDragon I don't why it doesn't work for you but CTRL + 0 / - / + work fine for me on Linux with KDE Plasma

I just gave it a try on my other machine and zoom over the shortcut works fine over there.
Both machines run KDE6, so not sure whats the problem then.
But hey it at least shows that you can run into the problem, in which you are dependent on a slider.

Kaydax · 2024-04-11T22:30:17Z

So any updates on this? If there is still a problem with how I handle the inputs for zoom, it would be good to help find a better way if that's the only issue.

ghost · 2024-04-23T13:06:47Z

So any updates on this? If there is still a problem with how I handle the inputs for zoom, it would be good to help find a better way if that's the only issue.

I have a problem, zoom simply does not work, i am on kde6. Besides that i only have a 60% keyboard and there is no way to make those combos work. Getting a full sized keyboard is not a solution either, nor having 2 keyboards on the desk at the same time.

Kaydax · 2024-04-28T01:06:00Z

I have a problem, zoom simply does not work, i am on kde6. Besides that i only have a 60% keyboard and there is no way to make those combos work. Getting a full sized keyboard is not a solution either, nor having 2 keyboards on the desk at the same time.

Wdym the "no way to make those combos work". The only key combos used are CTRL +/-. and 60% keyboards have +/-

ghost · 2024-04-28T12:15:12Z

I have a problem, zoom simply does not work, i am on kde6. Besides that i only have a 60% keyboard and there is no way to make those combos work. Getting a full sized keyboard is not a solution either, nor having 2 keyboards on the desk at the same time.

Wdym the "no way to make those combos work". The only key combos used are CTRL +/-. and 60% keyboards have +/-

Those combos work only in english layout on the 60%keyboard, which i rarely use. Took me some time to find out.

Vendicated

you convinced me so i guess we can go forward and get this merged!

however there are some things that need to be fixed first

Vendicated · 2024-05-23T15:55:10Z

src/renderer/components/settings/WindowZoom.tsx

+                }}
+                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]}


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

Vendicated · 2024-05-23T15:55:29Z

src/renderer/components/settings/WindowZoom.tsx

+                maxValue={2}
+                markers={[0.5, 0.67, 0.75, 0.8, 0.9, 1, 1.1, 1.25, 1.5, 1.75, 2]}
+                stickToMarkers={true}
+                onMarkerRender={v => (v === 1 ? "100" : `${Math.round(v * 100)}`)}


Suggested change

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

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

Vendicated · 2024-05-23T15:55:40Z

src/renderer/components/settings/WindowZoom.tsx

+                markers={[0.5, 0.67, 0.75, 0.8, 0.9, 1, 1.1, 1.25, 1.5, 1.75, 2]}
+                stickToMarkers={true}
+                onMarkerRender={v => (v === 1 ? "100" : `${Math.round(v * 100)}`)}
+            ></Slider>


Suggested change

></Slider>

/>

Vendicated · 2024-05-23T15:55:51Z

src/shared/IpcEvents.ts

@@ -18,6 +18,7 @@ export const enum IpcEvents {
    FOCUS = "VCD_FOCUS",
    MINIMIZE = "VCD_MINIMIZE",
    MAXIMIZE = "VCD_MAXIMIZE",
+    SET_ZOOM = "VCD_ZOOM",


Suggested change

SET_ZOOM = "VCD_ZOOM",

SET_ZOOM = "VCD_SET_ZOOM",

Vendicated · 2024-05-23T15:56:17Z

src/shared/settings.d.ts

@@ -20,6 +20,7 @@ export interface Settings {
    arRPC?: boolean;
    appBadge?: boolean;
    disableMinSize?: boolean;
+    zoomFactor?: number;


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

Then how would it persist after restarting?

state is also persisted!

Vendicated · 2024-05-23T15:59:58Z

src/main/mainWindow.ts

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


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);

Vendicated · 2024-05-23T16:01:56Z

src/renderer/components/settings/WindowZoom.tsx

+                defaultValue={1}
+                onValueChange={v => {
+                    settings.zoomFactor = v;
+                    VesktopNative.win.zoom(v);


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

Vendicated · 2024-05-23T16:03:14Z

src/renderer/components/settings/WindowZoom.tsx

+                onValueChange={v => {
+                    settings.zoomFactor = v;
+                    VesktopNative.win.zoom(v);
+                    setZoomFactor(v);


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

Vendicated · 2024-05-23T16:03:28Z

src/renderer/components/settings/WindowZoom.tsx

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


Suggested change

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

Vendicated · 2024-05-23T16:05:51Z

src/renderer/components/settings/WindowZoom.tsx

+            setZoomFactor(event.detail);
+        };
+
+        window.addEventListener("zoomChanged", handleZoomChange);


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

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

Kaydax added 2 commits January 31, 2024 16:00

Add zoom slider, match offical client zoom

54dd33d

Forgot about resetZoom

d516595

Vendicated reviewed Feb 18, 2024

View reviewed changes

This comment was marked as spam.

Sign in to view

Kaydax added 2 commits March 12, 2024 05:13

Update to latest vesktop

47aff14

Merge branch 'main' into zoom-parity

1e333fa

Merge branch 'main' into zoom-parity

9f9038c

Merge branch 'Vencord:main' into zoom-parity

1cb5680

Vendicated requested changes May 23, 2024

View reviewed changes

Merge branch 'main' into zoom-parity

2bbc8d3

Vendicated marked this pull request as draft July 4, 2024 16:52

Kaydax added 3 commits July 10, 2024 21:31

Merge branch 'Vencord:main' into zoom-parity

436837d

Merge branch 'Vencord:main' into zoom-parity

5b66750

Merge branch 'Vencord:main' into zoom-parity

2412da5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add zoom slider, match official client zoom #370

Add zoom slider, match official client zoom #370

Kaydax commented Jan 31, 2024

Vendicated Feb 18, 2024

Kaydax Feb 18, 2024

Vendicated May 23, 2024

Vendicated commented Feb 18, 2024 •

edited

Loading

sunnniee commented Feb 18, 2024

Vendicated commented Feb 18, 2024

Kaydax commented Feb 18, 2024 •

edited

Loading

This comment was marked as spam.

TomTheDragon commented Mar 18, 2024

D3SOX commented Mar 18, 2024

TomTheDragon commented Mar 19, 2024

Kaydax commented Apr 11, 2024

ghost commented Apr 23, 2024 •

edited by ghost

Loading

Kaydax commented Apr 28, 2024

ghost commented Apr 28, 2024

Vendicated left a comment

Vendicated May 23, 2024

Vendicated May 23, 2024

Vendicated May 23, 2024

Vendicated May 23, 2024

Vendicated May 23, 2024

Kaydax May 24, 2024

Vendicated May 28, 2024

Vendicated May 23, 2024

Vendicated May 23, 2024

Vendicated May 23, 2024

Vendicated May 23, 2024

Vendicated May 23, 2024

Kaydax Jun 19, 2024

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

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

Add zoom slider, match official client zoom #370

Are you sure you want to change the base?

Add zoom slider, match official client zoom #370

Conversation

Kaydax commented Jan 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vendicated commented Feb 18, 2024 • edited Loading

sunnniee commented Feb 18, 2024

Vendicated commented Feb 18, 2024

Kaydax commented Feb 18, 2024 • edited Loading

This comment was marked as spam.

TomTheDragon commented Mar 18, 2024

D3SOX commented Mar 18, 2024

TomTheDragon commented Mar 19, 2024

Kaydax commented Apr 11, 2024

ghost commented Apr 23, 2024 • edited by ghost Loading

Kaydax commented Apr 28, 2024

ghost commented Apr 28, 2024

Vendicated left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Vendicated commented Feb 18, 2024 •

edited

Loading

Kaydax commented Feb 18, 2024 •

edited

Loading

ghost commented Apr 23, 2024 •

edited by ghost

Loading