-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add UI font size adjustment with Ctrl+Mouse Wheel #7531
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -74,6 +74,8 @@ const App = () => { | |||||||||||||
| cloudApiUrl, | ||||||||||||||
| renderContext, | ||||||||||||||
| mdmCompliant, | ||||||||||||||
| uiFontSize, | ||||||||||||||
| setUiFontSize, | ||||||||||||||
| } = useExtensionState() | ||||||||||||||
|
|
||||||||||||||
| // Create a persistent state manager | ||||||||||||||
|
|
@@ -226,6 +228,43 @@ const App = () => { | |||||||||||||
| } | ||||||||||||||
| }, [tab]) | ||||||||||||||
|
|
||||||||||||||
| // Handle Ctrl + Mouse Wheel for font size adjustment | ||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This wheel event handler logic is duplicated. Could we extract this into a custom hook like |
||||||||||||||
| useEffect(() => { | ||||||||||||||
| const handleWheel = (e: WheelEvent) => { | ||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing test coverage for this wheel event handler. Consider adding tests to verify:
|
||||||||||||||
| // Check if Ctrl key is pressed (or Cmd on Mac) | ||||||||||||||
| if (e.ctrlKey || e.metaKey) { | ||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the Cmd+Mouse Wheel behavior on Mac intentional? The PR description and issue #7530 only mention Ctrl+Mouse Wheel, but this also responds to metaKey. If this is intended for Mac support, it should be documented in the PR description. |
||||||||||||||
| e.preventDefault() // Prevent default browser zoom | ||||||||||||||
|
|
||||||||||||||
| const currentSize = uiFontSize || 100 | ||||||||||||||
| const delta = e.deltaY > 0 ? -5 : 5 // Decrease on scroll down, increase on scroll up | ||||||||||||||
| const newSize = Math.min(200, Math.max(50, currentSize + delta)) // Clamp between 50% and 200% | ||||||||||||||
|
|
||||||||||||||
| if (newSize !== currentSize) { | ||||||||||||||
| setUiFontSize(newSize) | ||||||||||||||
| // Send message to extension to persist the setting | ||||||||||||||
| vscode.postMessage({ type: "uiFontSize", value: newSize }) | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Add event listener with passive: false to allow preventDefault | ||||||||||||||
| window.addEventListener("wheel", handleWheel, { passive: false }) | ||||||||||||||
|
|
||||||||||||||
| return () => { | ||||||||||||||
| window.removeEventListener("wheel", handleWheel) | ||||||||||||||
| } | ||||||||||||||
| }, [uiFontSize, setUiFontSize]) | ||||||||||||||
|
|
||||||||||||||
| // Apply font size to the document | ||||||||||||||
| useEffect(() => { | ||||||||||||||
| if (uiFontSize) { | ||||||||||||||
| // Apply the font size as a CSS variable that can be used throughout the app | ||||||||||||||
| document.documentElement.style.setProperty("--ui-font-scale", `${uiFontSize / 100}`) | ||||||||||||||
| // Also apply a direct font-size to the body for immediate effect | ||||||||||||||
| document.documentElement.style.fontSize = `${uiFontSize}%` | ||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Setting both a CSS variable and direct fontSize might be redundant. Consider using just the CSS variable approach:
Suggested change
|
||||||||||||||
| } | ||||||||||||||
| }, [uiFontSize]) | ||||||||||||||
|
|
||||||||||||||
| if (!didHydrateState) { | ||||||||||||||
| return null | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add runtime validation here to ensure fontSize is within the expected range?