-
Notifications
You must be signed in to change notification settings - Fork 402
feat: add web zoom to help widget #1893
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
Conversation
The web widget already had a zoom feature, but the help widget did not. This change ports the zoom feature so it is available on the help widget as well.
WalkthroughThe pull request introduces enhancements to the The The changes involve updating import statements to include necessary types and clients, likely supporting the new zoom functionality in the help view interface. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/app/view/helpview/helpview.tsx (2)
64-81
: LGTM with a minor suggestion for readability.The zoom factor implementation is robust with proper input validation, DOM readiness check, and persistence.
Consider extracting the zoom limits as named constants for better readability and maintainability:
+const MIN_ZOOM_FACTOR = 0.1; +const MAX_ZOOM_FACTOR = 5; +const DEFAULT_ZOOM_FACTOR = 1; setZoomFactor(factor: number | null) { // null is ok (will reset to default) - if (factor != null && factor < 0.1) { - factor = 0.1; + if (factor != null && factor < MIN_ZOOM_FACTOR) { + factor = MIN_ZOOM_FACTOR; } - if (factor != null && factor > 5) { - factor = 5; + if (factor != null && factor > MAX_ZOOM_FACTOR) { + factor = MAX_ZOOM_FACTOR; } const domReady = globalStore.get(this.domReady); if (!domReady) { return; } - this.webviewRef.current?.setZoomFactor(factor || 1); + this.webviewRef.current?.setZoomFactor(factor || DEFAULT_ZOOM_FACTOR); RpcApi.SetMetaCommand(TabRpcClient, { oref: WOS.makeORef("block", this.blockId), meta: { "web:zoom": factor }, // allow null so we can remove the zoom factor here }); }
Line range hint
84-135
: LGTM with a minor suggestion for maintainability.The zoom menu implementation is well-structured with a good range of zoom levels and proper handling of the current zoom state.
Consider extracting the zoom levels into a configuration array for better maintainability:
+const ZOOM_LEVELS = [ + { label: "25%", factor: 0.25 }, + { label: "50%", factor: 0.5 }, + { label: "70%", factor: 0.7 }, + { label: "80%", factor: 0.8 }, + { label: "90%", factor: 0.9 }, + { label: "100%", factor: 1 }, + { label: "110%", factor: 1.1 }, + { label: "120%", factor: 1.2 }, + { label: "130%", factor: 1.3 }, + { label: "150%", factor: 1.5 }, + { label: "175%", factor: 1.75 }, + { label: "200%", factor: 2 }, +]; getSettingsMenuItems(): ContextMenuItem[] { const zoomSubMenu: ContextMenuItem[] = []; let curZoom = 1; if (globalStore.get(this.domReady)) { curZoom = this.webviewRef.current?.getZoomFactor() || 1; } const model = this; function makeZoomFactorMenuItem(label: string, factor: number): ContextMenuItem { return { label: label, type: "checkbox", click: () => { model.setZoomFactor(factor); }, checked: curZoom == factor, }; } zoomSubMenu.push({ label: "Reset", click: () => { model.setZoomFactor(null); }, }); - zoomSubMenu.push(makeZoomFactorMenuItem("25%", 0.25)); - zoomSubMenu.push(makeZoomFactorMenuItem("50%", 0.5)); - zoomSubMenu.push(makeZoomFactorMenuItem("70%", 0.7)); - zoomSubMenu.push(makeZoomFactorMenuItem("80%", 0.8)); - zoomSubMenu.push(makeZoomFactorMenuItem("90%", 0.9)); - zoomSubMenu.push(makeZoomFactorMenuItem("100%", 1)); - zoomSubMenu.push(makeZoomFactorMenuItem("110%", 1.1)); - zoomSubMenu.push(makeZoomFactorMenuItem("120%", 1.2)); - zoomSubMenu.push(makeZoomFactorMenuItem("130%", 1.3)); - zoomSubMenu.push(makeZoomFactorMenuItem("150%", 1.5)); - zoomSubMenu.push(makeZoomFactorMenuItem("175%", 1.75)); - zoomSubMenu.push(makeZoomFactorMenuItem("200%", 2)); + ZOOM_LEVELS.forEach(({ label, factor }) => { + zoomSubMenu.push(makeZoomFactorMenuItem(label, factor)); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/app/view/helpview/helpview.tsx
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build for TestDriver.ai
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
frontend/app/view/helpview/helpview.tsx (1)
5-7
: LGTM!The new imports are necessary for the RPC functionality used in the zoom feature implementation.
The web widget already had a zoom feature, but the help widget did not. This change ports the zoom feature so it is available on the help widget as well.
The web widget already had a zoom feature, but the help widget did not. This change ports the zoom feature so it is available on the help widget as well.