Conversation
…iles and scripts in package json file
…s over the frontend
WalkthroughThis PR refactors the application from a LitElement-based web component architecture to a React + Vite frontend with modernized build configuration. The backend electron-forge setup is enhanced with new plugin configurations, preload scripts, and utility modules for IPC-based storage and multi-platform media capture. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Key areas requiring careful review:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
done with basic conversion |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/assets/renderer.js (2)
338-341: Critical: Undefined method call in error handler.Line 340 calls
cheatingDaddy.setStatus('error')butsetStatusis not defined in thecheatingDaddyobject (lines 933-960). This will throw a runtime error whenstartCapturefails.Apply this diff to remove the broken call:
} catch (err) { console.error('Error starting capture:', err); - cheatingDaddy.setStatus('error'); + // Status updates now handled by React components via IPC events }
611-614: Critical: Undefined method call in error handler.Line 613 calls
cheatingDaddy.addNewResponse()butaddNewResponseis not defined in thecheatingDaddyobject (lines 933-960). This will throw a runtime error when image capture fails.Apply this diff to remove the broken call:
} else { console.error('Failed to get image response:', result.error); - cheatingDaddy.addNewResponse(`Error: ${result.error}`); + // Error already logged; React components handle error display via IPC events }src/main/utils/window.js (1)
224-226:executeJavaScriptbypassescontextIsolationsecurity boundary.Using
executeJavaScriptto call renderer code undermines the security benefits ofcontextIsolation: true. This allows arbitrary code execution in the renderer context. Use IPC messaging instead.- // Use the new handleShortcut function - mainWindow.webContents.executeJavaScript(` - cheatingDaddy.handleShortcut('${shortcutKey}'); - `); + // Use IPC to communicate with renderer + mainWindow.webContents.send('shortcut-triggered', shortcutKey);Then handle
'shortcut-triggered'in your preload/renderer viaipcRenderer.on().
🧹 Nitpick comments (31)
forge.config.js (1)
19-28: Redundant makers for macOS (darwin).Both
@electron-forge/maker-zip(line 19) and@electron-forge/maker-dmg(line 23) target thedarwinplatform. This will produce both a ZIP and DMG for macOS, which is typically unnecessary.Consider removing the ZIP maker from darwin platforms:
{ name: '@electron-forge/maker-zip', - platforms: ['darwin', 'win32'] + platforms: ['win32'] },src/renderer/styles/global.css (1)
1-71: Consider making transparency configurable.Multiple background properties hardcode
0.8alpha values. Consider defining transparency as a CSS custom property for easier theming::root { + --background-alpha: 0.8; /* Backgrounds - with default 0.8 transparency */ --background-transparent: transparent; - --bg-primary: rgba(30, 30, 30, 0.8); + --bg-primary: rgba(30, 30, 30, var(--background-alpha)); /* ... apply to other rgba values ... */ }vite.config.js (1)
14-16: Verify Rollup input path is correct.With
rootset to'./src/renderer'(line 8), the input path on line 15 constructs an absolute path that may be redundant. Since Vite operates relative to theroot, consider simplifying:input: { - main: path.resolve(__dirname, 'src/renderer/index.html') + main: path.resolve(__dirname, './src/renderer/index.html') }Or even simpler, since
rootis already./src/renderer:input: { - main: path.resolve(__dirname, 'src/renderer/index.html') + main: 'index.html' }Verify the build works correctly and adjust if needed.
src/renderer/components/views/HistoryView.css (1)
74-84: Extract message type colors to CSS custom properties.The colors for user (
#3b82f6), AI (#ef4444), and screen (#22c55e) messages are hardcoded in two places (lines 74-84 and 140-150). This creates duplication and prevents theme customization.Add to
global.css::root { /* Message type colors */ --message-user-color: #3b82f6; --message-ai-color: #ef4444; --message-screen-color: #22c55e; }Then update this file:
.message.user { - border-left-color: #3b82f6; + border-left-color: var(--message-user-color); } .message.ai { - border-left-color: #ef4444; + border-left-color: var(--message-ai-color); } .message.screen { - border-left-color: #22c55e; + border-left-color: var(--message-screen-color); } .legend-dot.user { - background-color: #3b82f6; + background-color: var(--message-user-color); } .legend-dot.ai { - background-color: #ef4444; + background-color: var(--message-ai-color); } .legend-dot.screen { - background-color: #22c55e; + background-color: var(--message-screen-color); }Also applies to: 140-150
src/renderer/index.html (2)
6-6: CSP allowsunsafe-inlinewhich weakens script security.The
'unsafe-inline'directive permits inline scripts, reducing XSS protection. If inline scripts aren't required in the React app, consider removing this or using nonces/hashes instead. If it's necessary for third-party libraries or Vite dev mode, document the rationale.
13-15: Move<link>stylesheet to<head>for proper document structure.While placing
<link>in<body>is valid HTML5, it's unconventional and can cause a brief flash of unstyled content. Moving it to<head>ensures styles are loaded before rendering begins.<head> <meta charset="UTF-8" /> <meta name="viewport" content="width=device-width, initial-scale=1.0" /> <meta http-equiv="content-security-policy" content="script-src 'self' 'unsafe-inline'" /> <title>Cheating Daddy</title> + <link rel="stylesheet" href="/highlight-vscode-dark.min.css" /> </head> <body> <div id="root"></div> <!-- Load markdown and syntax highlighting libraries --> <script src="/marked-4.3.0.min.js"></script> <script src="/highlight-11.9.0.min.js"></script> - <link rel="stylesheet" href="/highlight-vscode-dark.min.css" />src/renderer/components/views/HistoryView.jsx (4)
27-36: Missing loading state when loading selected session.
loadSelectedSessiondoesn't set a loading indicator, so users see no feedback while the session loads. Consider adding a loading state similar toloadSessions.const loadSelectedSession = async (sessionId) => { try { + setLoading(true); const session = await window.cheatingDaddy.storage.getSession(sessionId); if (session) { setSelectedSession(session); } } catch (error) { console.error('Error loading session:', error); + } finally { + setLoading(false); } };
38-53: Hardcoded'en-US'locale may not respect user preferences.Consider using
undefinedas the locale parameter to use the user's default locale, or make locale configurable.const formatDate = (timestamp) => { const date = new Date(timestamp); - return date.toLocaleDateString('en-US', { + return date.toLocaleDateString(undefined, { month: 'short', day: 'numeric', year: 'numeric', }); }; const formatTime = (timestamp) => { const date = new Date(timestamp); - return date.toLocaleTimeString('en-US', { + return date.toLocaleTimeString(undefined, { hour: '2-digit', minute: '2-digit', }); };
189-193: Using array index as React key can cause rendering issues.Using
indexas the key for messages may cause incorrect reconciliation if messages are added, removed, or reordered. Consider using a unique identifier (e.g., combining timestamp with message type and index).- return messages.map((message, index) => ( - <div key={index} className={`message ${message.type}`}> + return messages.map((message, index) => ( + <div key={`${message.timestamp}-${message.type}-${index}`} className={`message ${message.type}`}> {message.content} </div> ));
203-210: Using array index as React key for screen analysis items.Same issue as with messages—if
screenAnalysisHistoryitems are reordered or filtered, using index as key can cause rendering bugs.- return screenAnalysisHistory.map((analysis, index) => ( - <div key={index} className="message screen"> + return screenAnalysisHistory.map((analysis, index) => ( + <div key={`${analysis.timestamp}-${index}`} className="message screen"> <div className="analysis-meta"> {formatTimestamp(analysis.timestamp)} • {analysis.model || 'unknown model'} </div> {analysis.response} </div> ));src/renderer/components/views/HelpView.jsx (2)
21-36: MovegetDefaultKeybindsoutside the component or memoize it.This function is defined inside the component and recreated on every render. Since it only depends on
window.cheatingDaddy?.isMacOSwhich doesn't change during component lifecycle, define it outside the component or wrap withuseCallback.+const getDefaultKeybinds = () => { + const isMac = window.cheatingDaddy?.isMacOS || navigator.platform.includes('Mac'); + return { + moveUp: isMac ? 'Alt+Up' : 'Ctrl+Up', + moveDown: isMac ? 'Alt+Down' : 'Ctrl+Down', + moveLeft: isMac ? 'Alt+Left' : 'Ctrl+Left', + moveRight: isMac ? 'Alt+Right' : 'Ctrl+Right', + toggleVisibility: isMac ? 'Cmd+\\' : 'Ctrl+\\', + toggleClickThrough: isMac ? 'Cmd+M' : 'Ctrl+M', + nextStep: isMac ? 'Cmd+Enter' : 'Ctrl+Enter', + previousResponse: isMac ? 'Cmd+[' : 'Ctrl+[', + nextResponse: isMac ? 'Cmd+]' : 'Ctrl+]', + scrollUp: isMac ? 'Cmd+Shift+Up' : 'Ctrl+Shift+Up', + scrollDown: isMac ? 'Cmd+Shift+Down' : 'Ctrl+Shift+Down', + }; +}; + function HelpView() { const [keybinds, setKeybinds] = useState(getDefaultKeybinds()); - - // ... rest of component - - function getDefaultKeybinds() { - // ... function body - }
44-49: Add error handling for external link click.If
ipcRenderer.invokefails, the error will be unhandled. Consider adding try-catch for resilience.const handleExternalLinkClick = async (url) => { if (window.require) { - const { ipcRenderer } = window.require('electron'); - await ipcRenderer.invoke('open-external', url); + try { + const { ipcRenderer } = window.require('electron'); + await ipcRenderer.invoke('open-external', url); + } catch (error) { + console.error('Failed to open external link:', error); + } } };src/renderer/components/views/MainView.jsx (2)
4-4: Remove unusedonLayoutModeChangeprop.The
onLayoutModeChangeprop is declared in the function signature but never used within the component.-function MainView({ onStart, onLayoutModeChange }) { +function MainView({ onStart }) {
51-59: Consider debouncing API key persistence.The API key is saved to storage on every keystroke. For a password input, consider debouncing the save operation or saving on blur instead to reduce storage writes.
+import { useCallback, useRef } from 'react'; + +// Inside component: +const saveTimeoutRef = useRef(null); + const handleInput = async (e) => { const value = e.target.value; setApiKey(value); - await window.cheatingDaddy.storage.setApiKey(value); + + // Debounce storage write + if (saveTimeoutRef.current) { + clearTimeout(saveTimeoutRef.current); + } + saveTimeoutRef.current = setTimeout(async () => { + try { + await window.cheatingDaddy.storage.setApiKey(value); + } catch (error) { + console.error('Failed to save API key:', error); + } + }, 300); if (showApiKeyError) { setShowApiKeyError(false); } };src/renderer/components/AppHeader/AppHeader.css (1)
88-110: Consider using a CSS variable for the update button accent color.The update button uses hardcoded
#f14c4cwhile other colors use CSS variables. For consistency and easier theming, consider defining a variable like--update-coloror--danger-color..update-button { background: transparent; - color: #f14c4c; - border: 1px solid #f14c4c; + color: var(--danger-color, #f14c4c); + border: 1px solid var(--danger-color, #f14c4c); /* ... rest of styles */ } .update-button:hover { - background: rgba(241, 76, 76, 0.1); + background: color-mix(in srgb, var(--danger-color, #f14c4c) 10%, transparent); }src/main/preload.js (1)
4-4: Consider removing or conditionalizing debug logs.Console logs for IPC operations and initialization will appear in production builds. Consider removing them or guarding with a debug flag.
+const DEBUG = process.env.NODE_ENV === 'development'; + -console.log('✅ Preload script executing...'); +if (DEBUG) console.log('✅ Preload script executing...'); // In ipcRenderer wrappers: send: (channel, ...args) => { - console.log(`IPC Send: ${channel}`, args); + if (DEBUG) console.log(`IPC Send: ${channel}`, args); return ipcRenderer.send(channel, ...args); },Also applies to: 10-11, 22-23, 47-47
src/renderer/components/views/OnboardingView.jsx (2)
16-43: MovecolorSchemesconstant outside the component.This large static array is recreated on every render. Define it outside the component for better performance.
+// Define outside component +const COLOR_SCHEMES = [ + // Slide 1 - Welcome (Very dark purple/gray) + [ + [25, 25, 35], [20, 20, 30], [30, 25, 40], + [15, 15, 25], [35, 30, 45], [10, 10, 20], + ], + // ... rest of schemes +]; + function OnboardingView({ onComplete, onClose }) { - // ... - const colorSchemes = [ - // ... - ]; + // Use COLOR_SCHEMES instead of colorSchemes
173-179: Replace magic number withslides.length.The hardcoded
4should reference the slides array length for maintainability. If slides are added/removed, these checks would break.+const TOTAL_SLIDES = 5; // Or move slides array outside and use slides.length const nextSlide = () => { - if (currentSlide < 4) { + if (currentSlide < TOTAL_SLIDES - 1) { startColorTransition(currentSlide + 1); } else { completeOnboarding(); } }; // And in JSX: <button className="nav-button" onClick={nextSlide}> - {currentSlide === 4 ? ( + {currentSlide === TOTAL_SLIDES - 1 ? ( 'Get Started' ) : (Also applies to: 300-308
package.json (1)
32-32: Moveconcurrentlyto devDependencies.
concurrentlyis only used for npm scripts during development and is not needed at runtime. It should be indevDependenciesto reduce production bundle size."dependencies": { "@google/genai": "^1.2.0", - "concurrently": "^9.2.1", "electron-squirrel-startup": "^1.0.1", "react": "^19.2.3", "react-dom": "^19.2.3" }, "devDependencies": { "@electron-forge/cli": "^7.8.1", + "concurrently": "^9.2.1",src/renderer/App.jsx (1)
138-151: Address TODO: Missing error handling for empty API key.The TODO comment indicates missing error feedback when the API key is empty.
Would you like me to generate an implementation that shows an error notification or modal when the API key is missing? I can open an issue to track this.
src/renderer/components/views/CustomizeView.jsx (2)
61-76: Consider movinggetDefaultKeybindsoutside the component.This function is recreated on every render but doesn't depend on any props or state. Moving it outside the component or memoizing it would improve performance.
+const getDefaultKeybinds = () => { + const isMac = typeof window !== 'undefined' && + (window.cheatingDaddy?.isMacOS || navigator.platform.includes('Mac')); + return { + moveUp: isMac ? 'Alt+Up' : 'Ctrl+Up', + moveDown: isMac ? 'Alt+Down' : 'Ctrl+Down', + moveLeft: isMac ? 'Alt+Left' : 'Ctrl+Left', + moveRight: isMac ? 'Alt+Right' : 'Ctrl+Right', + toggleVisibility: isMac ? 'Cmd+\\' : 'Ctrl+\\', + toggleClickThrough: isMac ? 'Cmd+M' : 'Ctrl+M', + nextStep: isMac ? 'Cmd+Enter' : 'Ctrl+Enter', + previousResponse: isMac ? 'Cmd+[' : 'Ctrl+[', + nextResponse: isMac ? 'Cmd+]' : 'Ctrl+]', + scrollUp: isMac ? 'Cmd+Shift+Up' : 'Ctrl+Shift+Up', + scrollDown: isMac ? 'Cmd+Shift+Down' : 'Ctrl+Shift+Down', + }; +}; + function CustomizeView({Then remove the inner function definition at lines 61-76.
273-281: Nested setTimeout pattern is fragile.The nested
setTimeoutcalls create a timing dependency that's hard to follow and could cause issues if the component unmounts during the delay. Consider using a cleaner async pattern.+const delay = (ms) => new Promise(resolve => setTimeout(resolve, ms)); + const clearLocalData = async () => { if (isClearing) return; setIsClearing(true); setClearStatusMessage(''); setClearStatusType(''); try { await window.cheatingDaddy.storage.clearAll(); setClearStatusMessage('Successfully cleared all local data'); setClearStatusType('success'); - setTimeout(() => { - setClearStatusMessage('Closing application...'); - setTimeout(async () => { - if (window.require) { - const { ipcRenderer } = window.require('electron'); - await ipcRenderer.invoke('quit-application'); - } - }, 1000); - }, 2000); + await delay(2000); + setClearStatusMessage('Closing application...'); + await delay(1000); + if (window.require) { + const { ipcRenderer } = window.require('electron'); + await ipcRenderer.invoke('quit-application'); + } } catch (error) {src/renderer/components/AppHeader/AppHeader.jsx (2)
42-61: Update check has no error boundary and runs on every mount.The update check fetches from GitHub on every component mount without caching. Consider adding rate limiting or caching the result in session/localStorage to avoid unnecessary network requests.
useEffect(() => { const checkForUpdates = async () => { try { + // Check cache first + const cached = sessionStorage.getItem('updateCheckResult'); + if (cached) { + const { available, timestamp } = JSON.parse(cached); + // Cache for 1 hour + if (Date.now() - timestamp < 3600000) { + setUpdateAvailable(available); + return; + } + } + const currentVersion = await window.cheatingDaddy.getVersion(); const response = await fetch('https://raw.githubusercontent.com/sohzm/cheating-daddy/refs/heads/master/package.json'); if (!response.ok) return; const remotePackage = await response.json(); const remoteVersion = remotePackage.version; - if (isNewerVersion(remoteVersion, currentVersion)) { - setUpdateAvailable(true); - } + const available = isNewerVersion(remoteVersion, currentVersion); + setUpdateAvailable(available); + sessionStorage.setItem('updateCheckResult', JSON.stringify({ available, timestamp: Date.now() })); } catch (err) { console.log('Update check failed:', err.message); } }; checkForUpdates(); }, []);
63-74: MoveisNewerVersionoutside the component.This pure utility function doesn't depend on component state or props. Moving it outside prevents recreation on each render and makes the code clearer.
+const isNewerVersion = (remote, current) => { + const remoteParts = remote.split('.').map(Number); + const currentParts = current.split('.').map(Number); + + for (let i = 0; i < Math.max(remoteParts.length, currentParts.length); i++) { + const r = remoteParts[i] || 0; + const c = currentParts[i] || 0; + if (r > c) return true; + if (r < c) return false; + } + return false; +}; + function AppHeader({Then remove the inner function definition at lines 63-74.
src/renderer/components/views/AssistantView.jsx (2)
138-147: DirectinnerHTMLassignment bypasses React's DOM reconciliation.Setting
innerHTMLdirectly on a ref'd element can cause React reconciliation issues and memory leaks if the content contains event listeners.Consider using
dangerouslySetInnerHTMLpattern which is more React-idiomatic:- const updateResponseContent = () => { - console.log('updateResponseContent called'); - if (responseContainerRef.current) { - const currentResponse = getCurrentResponse(); - console.log('Current response:', currentResponse); - const renderedResponse = renderMarkdown(currentResponse); - console.log('Rendered response:', renderedResponse); - responseContainerRef.current.innerHTML = renderedResponse; - } - }; + const renderedContent = useMemo(() => { + const currentResponse = getCurrentResponse(); + return renderMarkdown(currentResponse); + }, [responses, currentResponseIndex, selectedProfile]); ... - <div className="response-container" ref={responseContainerRef}></div> + <div + className="response-container" + ref={responseContainerRef} + dangerouslySetInnerHTML={{ __html: renderedContent }} + />This also removes the need for the
updateResponseContentuseEffect.
199-201: Magic number:getTotalAvailablereturns hardcoded 40.The hardcoded value makes the code less maintainable. Consider extracting as a constant.
+const FLASH_LIMIT = 20; +const FLASH_LITE_LIMIT = 20; +const TOTAL_DAILY_LIMIT = FLASH_LIMIT + FLASH_LITE_LIMIT; + function AssistantView({ ... const getTotalAvailable = () => { - return 40; // 20 flash + 20 flash-lite + return TOTAL_DAILY_LIMIT; };src/renderer/components/views/OnboardingView.css (3)
9-36: Consider adding explicit focus styles for keyboard accessibility.The
.close-buttonhas hover states but lacks a:focusor:focus-visiblestyle. Keyboard users navigating with Tab won't see a visible focus indicator..close-button:hover { background: rgba(255, 255, 255, 0.12); border-color: rgba(255, 255, 255, 0.2); color: rgba(255, 255, 255, 0.9); } + +.close-button:focus-visible { + outline: 2px solid rgba(255, 255, 255, 0.6); + outline-offset: 2px; +}
148-183: Add focus-visible state for keyboard navigation on nav buttons.Similar to the close button,
.nav-buttonshould have a visible focus indicator for accessibility compliance..nav-button:disabled:hover { background: rgba(255, 255, 255, 0.08); border-color: rgba(255, 255, 255, 0.1); transform: none; } + +.nav-button:focus-visible { + outline: 2px solid rgba(255, 255, 255, 0.6); + outline-offset: 2px; +}
191-206: Add focus state for interactive progress dots.The dots have
cursor: pointerindicating they're clickable, but lack keyboard focus styling..dot:hover { background: rgba(255, 255, 255, 0.4); } + +.dot:focus-visible { + outline: 2px solid rgba(255, 255, 255, 0.6); + outline-offset: 2px; +}src/main/utils/renderer.js (2)
147-155: Consider using chunk-based conversion for large buffers.String concatenation in a loop can be slow for large audio buffers. For improved performance with large data:
function arrayBufferToBase64(buffer) { - let binary = ''; const bytes = new Uint8Array(buffer); - const len = bytes.byteLength; - for (let i = 0; i < len; i++) { - binary += String.fromCharCode(bytes[i]); - } - return btoa(binary); + const CHUNK_SIZE = 0x8000; + let binary = ''; + for (let i = 0; i < bytes.length; i += CHUNK_SIZE) { + binary += String.fromCharCode.apply(null, bytes.subarray(i, i + CHUNK_SIZE)); + } + return btoa(binary); }
534-584: Consider extracting shared screenshot logic into helper functions.
captureManualScreenshotduplicates much ofcaptureScreenshot(video/canvas initialization, readyState check, quality mapping). Extracting these into helpers would reduce maintenance burden.// Helper for lazy video/canvas initialization async function ensureVideoCapture() { if (!hiddenVideo) { hiddenVideo = document.createElement('video'); hiddenVideo.srcObject = mediaStream; hiddenVideo.muted = true; hiddenVideo.playsInline = true; await hiddenVideo.play(); await new Promise(resolve => { if (hiddenVideo.readyState >= 2) return resolve(); hiddenVideo.onloadedmetadata = () => resolve(); }); offscreenCanvas = document.createElement('canvas'); offscreenCanvas.width = hiddenVideo.videoWidth; offscreenCanvas.height = hiddenVideo.videoHeight; offscreenContext = offscreenCanvas.getContext('2d'); } return hiddenVideo.readyState >= 2; } // Helper for quality value mapping function getQualityValue(quality) { const qualityMap = { high: 0.9, medium: 0.7, low: 0.5 }; return qualityMap[quality] ?? 0.7; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (38)
forge.config.js(1 hunks)package.json(3 hunks)src/assets/renderer.js(4 hunks)src/components/app/AppHeader.js(0 hunks)src/components/app/CheatingDaddyApp.js(0 hunks)src/components/index.js(0 hunks)src/components/views/AssistantView.js(0 hunks)src/components/views/CustomizeView.js(0 hunks)src/components/views/HelpView.js(0 hunks)src/components/views/HistoryView.js(0 hunks)src/components/views/MainView.js(0 hunks)src/components/views/OnboardingView.js(0 hunks)src/main/index.js(1 hunks)src/main/preload.js(1 hunks)src/main/utils/gemini.js(1 hunks)src/main/utils/renderer.js(1 hunks)src/main/utils/window.js(2 hunks)src/preload.js(0 hunks)src/renderer/App.css(1 hunks)src/renderer/App.jsx(1 hunks)src/renderer/components/AppHeader/AppHeader.css(1 hunks)src/renderer/components/AppHeader/AppHeader.jsx(1 hunks)src/renderer/components/views/AssistantView.css(1 hunks)src/renderer/components/views/AssistantView.jsx(1 hunks)src/renderer/components/views/CustomizeView.css(1 hunks)src/renderer/components/views/CustomizeView.jsx(1 hunks)src/renderer/components/views/HelpView.css(1 hunks)src/renderer/components/views/HelpView.jsx(1 hunks)src/renderer/components/views/HistoryView.css(1 hunks)src/renderer/components/views/HistoryView.jsx(1 hunks)src/renderer/components/views/MainView.css(1 hunks)src/renderer/components/views/MainView.jsx(1 hunks)src/renderer/components/views/OnboardingView.css(1 hunks)src/renderer/components/views/OnboardingView.jsx(1 hunks)src/renderer/index.html(1 hunks)src/renderer/index.jsx(1 hunks)src/renderer/styles/global.css(1 hunks)vite.config.js(1 hunks)
💤 Files with no reviewable changes (10)
- src/components/views/HistoryView.js
- src/components/index.js
- src/components/views/HelpView.js
- src/components/app/AppHeader.js
- src/components/views/MainView.js
- src/preload.js
- src/components/views/CustomizeView.js
- src/components/app/CheatingDaddyApp.js
- src/components/views/AssistantView.js
- src/components/views/OnboardingView.js
🧰 Additional context used
🧬 Code graph analysis (10)
src/renderer/components/views/HelpView.jsx (1)
src/main/utils/windowResize.js (1)
window(4-4)
vite.config.js (5)
src/main/utils/window.js (1)
path(2-2)src/main/index.js (1)
path(6-6)forge.config.js (1)
path(1-1)src/main/utils/gemini.js (1)
path(423-423)src/main/storage.js (1)
path(2-2)
src/renderer/components/views/AssistantView.jsx (2)
src/main/utils/windowResize.js (1)
window(4-4)src/renderer/App.jsx (3)
responses(19-19)currentResponseIndex(20-20)selectedProfile(17-17)
src/renderer/components/views/HistoryView.jsx (2)
src/renderer/components/views/CustomizeView.jsx (1)
customPrompt(22-22)src/main/utils/gemini.js (2)
conversationHistory(11-11)screenAnalysisHistory(12-12)
src/renderer/components/views/CustomizeView.jsx (3)
src/main/utils/windowResize.js (1)
window(4-4)src/main/utils/gemini.js (2)
value(161-176)currentProfile(13-13)src/renderer/App.jsx (4)
selectedProfile(17-17)layoutMode(23-23)selectedLanguage(18-18)selectedImageQuality(22-22)
src/renderer/components/views/MainView.jsx (5)
src/assets/renderer.js (1)
apiKey(158-158)src/main/utils/renderer.js (1)
apiKey(158-158)src/main/utils/windowResize.js (1)
window(4-4)src/main/utils/window.js (1)
isMac(113-113)src/main/utils/gemini.js (1)
value(161-176)
src/renderer/components/views/OnboardingView.jsx (1)
src/main/utils/windowResize.js (1)
window(4-4)
src/main/preload.js (3)
src/main/utils/window.js (2)
require(1-1)require(46-46)src/main/index.js (3)
require(5-5)require(7-7)require(8-8)src/main/utils/gemini.js (7)
require(1-1)require(2-2)require(3-3)require(4-4)require(5-5)require(6-6)require(422-422)
src/main/index.js (4)
src/main/utils/window.js (3)
path(2-2)require(1-1)require(46-46)forge.config.js (1)
path(1-1)src/main/utils/gemini.js (8)
path(423-423)require(1-1)require(2-2)require(3-3)require(4-4)require(5-5)require(6-6)require(422-422)src/main/storage.js (1)
path(2-2)
src/main/utils/renderer.js (3)
src/assets/renderer.js (11)
electronModule(22-22)electronModule(23-23)audioBuffer(30-30)audioBuffer(350-350)audioBuffer(383-383)audioBuffer(413-413)SAMPLE_RATE(31-31)storage(45-124)i(139-139)i(151-151)s(141-141)src/main/utils/gemini.js (3)
audioBuffer(456-456)SAMPLE_RATE(451-451)i(502-502)src/main/storage.js (2)
config(114-114)config(170-170)
🔇 Additional comments (38)
src/assets/renderer.js (5)
1-19: Good defensive programming for non-Electron context.The fallback dummy API prevents crashes when running outside Electron and provides helpful console messages to guide developers.
157-166: Status updates now delegated to React.The simplified
initializeGeminicorrectly returns a boolean and lets React components handle UI updates via IPC listeners.
168-172: Status listener comment is accurate.The comment correctly indicates React components now handle status updates through their own IPC listeners, removing the tight coupling.
734-741: Simplified shortcut handler aligns with React architecture.The comment correctly notes React components now manage their own shortcuts, and this handler is kept for compatibility with main process.
933-960: Consolidated API surface is cleaner.The
cheatingDaddyobject now exposes a focused API without UI-coupled methods, aligning well with the React migration.forge.config.js (2)
35-45: Plugin-fuses configuration added.The new
@electron-forge/plugin-fusesconfiguration adds security hardening with appropriate defaults (runAsNode disabled, cookie encryption enabled, etc.).
14-16: This is a newly created file, not a name change.The
forge.config.jsfile was created in the latest commit and has no prior history. There is no change fromcheating-daddytocheating_daddy—cheating_daddy(underscore) is the initial value. Therefore, this is not a breaking change affecting existing installations.If consistency with the repository name is desired, consider whether
cheating-daddy(hyphen) would be more appropriate for the Windows installer name.Likely an incorrect or invalid review comment.
src/main/index.js (2)
19-27: Well-structured initialization flow.The
whenReadyhandler properly initializes storage before creating windows and setting up IPC handlers in a logical sequence.
46-235: Comprehensive storage IPC handlers.The storage IPC setup provides a complete, consistent API for config, credentials, preferences, keybinds, sessions, limits, and clear operations with proper error handling.
src/renderer/index.jsx (1)
12-14: React.StrictMode is appropriate for development.Note that StrictMode causes intentional double-rendering in development to help detect side effects. This is expected behavior in React 19.
vite.config.js (1)
5-27: Vite configuration follows Electron + React patterns.The configuration properly separates the renderer process with appropriate root, public dir, and build output settings for an Electron app.
src/renderer/components/views/HistoryView.css (1)
1-256: Well-structured component styles.The stylesheet provides comprehensive, organized styling with consistent use of CSS custom properties for most theming needs, proper spacing, and custom scrollbar integration.
src/main/utils/gemini.js (1)
4-4: Import path change is correct. The fileaudioUtils.jsexists atsrc/audioUtils.js, and the import insrc/main/utils/gemini.jscorrectly resolves to it using the relative path../../audioUtils. No consistency issues detected—this is the only import of audioUtils in the codebase.src/renderer/App.css (1)
1-43: LGTM!Clean CSS structure with proper use of CSS variables for theming, flexbox layout, and view-specific modifiers. The transition on
.view-containerprovides smooth view changes.src/renderer/components/views/HelpView.css (1)
1-190: LGTM!Well-structured CSS with consistent use of CSS variables, responsive grid layouts using
auto-fit, and elegant CSS counter-based step numbering. The hover states and transitions provide good interactivity feedback.src/renderer/components/views/MainView.css (1)
1-114: LGTM!Clean CSS with appropriate styling for the main view's input and button elements. The
blink-redanimation provides clear visual feedback for API key errors, and the.initializingstate properly disables interaction.src/renderer/components/views/HistoryView.jsx (1)
116-133: Session list usessessionIdas key—this is correct.Good use of
session.sessionIdas a stable unique key for the sessions list.src/renderer/components/views/AssistantView.css (1)
1-309: LGTM!Comprehensive styling for the AssistantView with well-organized markdown rendering rules, proper scrollbar customization, and a clean CSS-only tooltip implementation. The typography and spacing are consistent.
src/renderer/components/views/CustomizeView.css (1)
1-519: LGTM!Thorough and well-organized settings layout CSS with responsive handling, comprehensive form controls styling (inputs, selects, sliders, color pickers), and consistent use of CSS variables for theming.
src/main/utils/window.js (2)
31-35: Good security configuration forwebPreferences.Using
contextIsolation: trueandnodeIntegration: falsewith a preload script is the recommended secure pattern for Electron apps.
39-44: Production file path is correctly configured.The path
../../dist/renderer/index.htmlresolves correctly to the Vite build output. The vite.config.js confirms the renderer builds to../../dist/rendereras the output directory, so the relative path fromsrc/main/utils/window.jsproperly targets the built renderer HTML file in production.src/renderer/components/views/HelpView.jsx (1)
51-243: LGTM!The JSX structure is clean and well-organized with good separation of UI sections. The dynamic keybind rendering and conditional formatting work correctly.
src/renderer/components/views/MainView.jsx (1)
61-114: LGTM!The button handlers, keyboard shortcut logic, and JSX rendering are well-implemented. The keyboard shortcut correctly handles both Mac and non-Mac platforms.
src/renderer/components/AppHeader/AppHeader.css (1)
1-86: LGTM!The CSS structure is clean with proper use of CSS variables, flex layouts, and
-webkit-app-regionfor Electron window dragging. Good separation between draggable and non-draggable areas.src/main/preload.js (1)
37-45: LGTM!Good security practice exposing only the
electronmodule via therequirewrapper, and proper use ofcontextBridgeto safely expose APIs to the renderer process.src/renderer/components/views/OnboardingView.jsx (2)
4-4: UnusedonCloseprop; verify quit behavior is intentional.The
onCloseprop is declared but never used. Instead,handleClosequits the entire application. If this is intentional (users must complete onboarding), consider removing the unused prop. Otherwise, useonClose()instead of quitting.-function OnboardingView({ onComplete, onClose }) { +function OnboardingView({ onComplete }) {Or if close should dismiss without quitting:
const handleClose = async () => { - if (window.require) { - const { ipcRenderer } = window.require('electron'); - await ipcRenderer.invoke('quit-application'); - } + onClose?.(); };Also applies to: 187-192
194-310: LGTM!The onboarding completion flow, slide rendering, and navigation controls are well-implemented. The canvas-based gradient animation with color transitions between slides provides a polished UX.
src/renderer/App.jsx (1)
153-170: MissingawaitforstopCaptureif it's async.Verify whether
window.cheatingDaddy.stopCapture()returns a Promise. If so, it should be awaited to ensure capture stops before the IPC call.#!/bin/bash # Search for stopCapture implementation to check if it's async rg -n "stopCapture" --type js --type ts -C 3package.json (1)
51-51: Vite version 7.3.0 is valid and is the current stable release. No action required.src/main/utils/renderer.js (9)
1-23: LGTM!Good defensive programming with clear developer guidance when running outside Electron context.
25-42: LGTM!Module-level state is appropriately scoped. The
SAMPLE_RATEconstant correctly matches the main process value (24000) shown in the relevant snippets.
43-124: LGTM with minor suggestion.The storage API wrapper is well-structured with consistent patterns. The default return values when
result.successis false prevent undefined errors, though silent failures could make debugging harder in production.
126-135: LGTM!The initial
loadPreferencesCache()call without await is acceptable sincestartCaptureproperly awaitsloadPreferencesCache()before accessing the cache, ensuring data is available when needed.
469-478: Single-pixel blank detection may miss edge cases.Sampling only pixel (0,0) to detect blank screenshots could miss cases where only that corner is black but the rest has content, or vice versa. Consider sampling multiple points if false positives/negatives become an issue.
626-668: LGTM!Comprehensive cleanup handling across all platforms with appropriate error handling for the macOS audio stop.
691-741: LGTM!IPC listeners are properly wrapped with try/catch and error logging. The session persistence pattern is consistent across all handlers.
745-931: LGTM!Well-structured theme system with good fallback handling, proper color manipulation utilities, and consistent CSS variable application.
933-967: LGTM with minor note.The consolidated API object is well-organized. Note that
window.captureManualScreenshot(line 624) is exposed separately while also accessible viacheatingDaddyindirectly. Consider whether the separate global is needed.
| } | ||
|
|
||
| const { app, BrowserWindow, shell, ipcMain } = require('electron'); | ||
| const path = require('path'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if 'path' variable is used in src/main/index.js
echo "Checking for usage of 'path' in src/main/index.js..."
grep -n "path\." src/main/index.js | grep -v "const path = require" || echo "No usage of 'path.' found (excluding the import line)"Repository: sohzm/cheating-daddy
Length of output: 170
Remove the unused path import.
The path module at line 6 is not used anywhere in this file and should be removed for clarity.
🤖 Prompt for AI Agents
In src/main/index.js around line 6, the require('path') import is unused; remove
the unused const path = require('path'); line and ensure any remaining code does
not reference path so lint/compile errors are avoided.
| on: (channel, func) => { | ||
| const subscription = (event, ...args) => func(event, ...args); | ||
| ipcRenderer.on(channel, subscription); | ||
| return () => ipcRenderer.removeListener(channel, subscription); | ||
| }, |
There was a problem hiding this comment.
removeListener won't work due to wrapped subscription function.
The on method wraps the callback in a new subscription function, but removeListener expects the original function reference. The returned unsubscribe function from on works correctly, but direct removeListener calls will fail to find the listener.
To fix this, either:
- Document that callers should use the returned unsubscribe function from
on - Track the mapping between original functions and subscriptions:
+const listenerMap = new WeakMap();
+
const electronAPI = {
ipcRenderer: {
// ...
on: (channel, func) => {
const subscription = (event, ...args) => func(event, ...args);
+ listenerMap.set(func, subscription);
ipcRenderer.on(channel, subscription);
return () => ipcRenderer.removeListener(channel, subscription);
},
// ...
removeListener: (channel, func) => {
- return ipcRenderer.removeListener(channel, func);
+ const subscription = listenerMap.get(func);
+ if (subscription) {
+ listenerMap.delete(func);
+ return ipcRenderer.removeListener(channel, subscription);
+ }
+ return ipcRenderer.removeListener(channel, func);
},Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/preload.js around lines 13 to 17, the current on wrapper creates a
new subscription function so external calls to ipcRenderer.removeListener(fn)
won't match and thus won't remove listeners; fix by tracking the mapping between
the original callback and the wrapped subscription (e.g., use a WeakMap keyed by
original function, optionally nested by channel) in the module scope, have on
store original->wrapped before calling ipcRenderer.on, update the returned
unsubscribe to call ipcRenderer.removeListener with the wrapped function and
remove the map entry, and implement a matching removeListener wrapper that looks
up the wrapped subscription from the map and passes that to
ipcRenderer.removeListener so both the unsubscribe function and direct
removeListener calls work.
| function setupLinuxMicProcessing(micStream) { | ||
| // Setup microphone audio processing for Linux | ||
| const micAudioContext = new AudioContext({ sampleRate: SAMPLE_RATE }); | ||
| const micSource = micAudioContext.createMediaStreamSource(micStream); | ||
| const micProcessor = micAudioContext.createScriptProcessor(BUFFER_SIZE, 1, 1); | ||
|
|
||
| let audioBuffer = []; | ||
| const samplesPerChunk = SAMPLE_RATE * AUDIO_CHUNK_DURATION; | ||
|
|
||
| micProcessor.onaudioprocess = async e => { | ||
| const inputData = e.inputBuffer.getChannelData(0); | ||
| audioBuffer.push(...inputData); | ||
|
|
||
| // Process audio in chunks | ||
| while (audioBuffer.length >= samplesPerChunk) { | ||
| const chunk = audioBuffer.splice(0, samplesPerChunk); | ||
| const pcmData16 = convertFloat32ToInt16(chunk); | ||
| const base64Data = arrayBufferToBase64(pcmData16.buffer); | ||
|
|
||
| await ipcRenderer.invoke('send-mic-audio-content', { | ||
| data: base64Data, | ||
| mimeType: 'audio/pcm;rate=24000', | ||
| }); | ||
| } | ||
| }; | ||
|
|
||
| micSource.connect(micProcessor); | ||
| micProcessor.connect(micAudioContext.destination); | ||
|
|
||
| // Store processor reference for cleanup | ||
| micAudioProcessor = micProcessor; | ||
| } |
There was a problem hiding this comment.
Migrate from deprecated createScriptProcessor to AudioWorklet.
ScriptProcessorNode is deprecated and has been replaced by AudioWorklets and the AudioWorkletNode interface. ScriptProcessorNode is deprecated from the specification and replaced with AudioWorklet.
Additionally, the await ipcRenderer.invoke() calls inside onaudioprocess create a backpressure risk. While the onaudioprocess handler completes immediately (not awaiting the Promise), slow IPC responses allow audio chunks to accumulate in the buffer as subsequent events continue firing, potentially causing audio drops or stuttering under load.
| } else { | ||
| console.error('Failed to get image response:', result.error); | ||
| cheatingDaddy.addNewResponse(`Error: ${result.error}`); | ||
| } |
There was a problem hiding this comment.
cheatingDaddy.addNewResponse is undefined and will throw.
The cheatingDaddy object (defined at lines 934-957) does not include an addNewResponse method. This will cause a TypeError when an image response fails.
Either add the method to cheatingDaddy or use an alternative error handling approach:
} else {
console.error('Failed to get image response:', result.error);
- cheatingDaddy.addNewResponse(`Error: ${result.error}`);
+ // Error handling should be done via IPC events or React state
+ ipcRenderer.send('display-error', result.error);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/utils/renderer.js around lines 611-614, the call
cheatingDaddy.addNewResponse(...) will throw because cheatingDaddy has no
addNewResponse method; fix this by either adding an addNewResponse
implementation to the cheatingDaddy object (implement it to accept a string
error, append it to whatever responses store the object uses and trigger any
UI/update hooks) or replace the call here with a safe fallback: check for typeof
cheatingDaddy.addNewResponse === 'function' before calling and otherwise use the
existing error-handling path (e.g., console.error plus the app's user-facing
error/UI update function) so no TypeError can occur.
| // Get current view and layout mode from storage (React manages its own state) | ||
| let viewName = 'main'; | ||
| let layoutMode = 'normal'; | ||
| try { | ||
| viewName = await event.sender.executeJavaScript('cheatingDaddy.getCurrentView()'); | ||
| layoutMode = await event.sender.executeJavaScript('cheatingDaddy.getLayoutMode()'); | ||
| const config = storage.getConfig(); | ||
| const prefs = storage.getPreferences(); | ||
| layoutMode = config.layout || 'normal'; | ||
| // Note: React manages currentView internally, use default 'main' for window sizing | ||
| } catch (error) { | ||
| console.warn('Failed to get view/layout from renderer, using defaults:', error); | ||
| viewName = 'main'; | ||
| layoutMode = 'normal'; | ||
| console.warn('Failed to get layout from storage, using defaults:', error); | ||
| } |
There was a problem hiding this comment.
viewName is always 'main', so window sizing ignores the actual view.
The code retrieves layoutMode from storage but viewName is hardcoded to 'main'. The comment suggests React manages currentView, but this means the window won't resize appropriately for different views (customize, help, history, etc.).
Consider storing the current view in storage or using IPC to query the renderer for the active view, so window sizing adapts correctly:
- let viewName = 'main';
let layoutMode = 'normal';
try {
const config = storage.getConfig();
const prefs = storage.getPreferences();
layoutMode = config.layout || 'normal';
- // Note: React manages currentView internally, use default 'main' for window sizing
+ viewName = prefs.currentView || 'main';
} catch (error) {
console.warn('Failed to get layout from storage, using defaults:', error);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Get current view and layout mode from storage (React manages its own state) | |
| let viewName = 'main'; | |
| let layoutMode = 'normal'; | |
| try { | |
| viewName = await event.sender.executeJavaScript('cheatingDaddy.getCurrentView()'); | |
| layoutMode = await event.sender.executeJavaScript('cheatingDaddy.getLayoutMode()'); | |
| const config = storage.getConfig(); | |
| const prefs = storage.getPreferences(); | |
| layoutMode = config.layout || 'normal'; | |
| // Note: React manages currentView internally, use default 'main' for window sizing | |
| } catch (error) { | |
| console.warn('Failed to get view/layout from renderer, using defaults:', error); | |
| viewName = 'main'; | |
| layoutMode = 'normal'; | |
| console.warn('Failed to get layout from storage, using defaults:', error); | |
| } | |
| // Get current view and layout mode from storage (React manages its own state) | |
| let viewName = 'main'; | |
| let layoutMode = 'normal'; | |
| try { | |
| const config = storage.getConfig(); | |
| const prefs = storage.getPreferences(); | |
| layoutMode = config.layout || 'normal'; | |
| viewName = prefs.currentView || 'main'; | |
| } catch (error) { | |
| console.warn('Failed to get layout from storage, using defaults:', error); | |
| } |
🤖 Prompt for AI Agents
In src/main/utils/window.js around lines 443 to 453, viewName is hardcoded to
'main' so window sizing ignores the actual active view; replace the hardcoded
value by reading the current view from a persisted preference or by querying the
renderer via IPC: attempt to read a stored currentView (e.g.
storage.getPreferences().currentView) and fall back to 'main', and if your app
does not persist view state implement an IPC call (ipcMain.handle/invoke) to ask
the renderer for the active view before sizing; ensure the code handles errors
and falls back safely to 'main' while still setting layoutMode from storage.
| // Listen for initialization status | ||
| useEffect(() => { | ||
| if (!window.electron?.ipcRenderer) return; | ||
|
|
||
| const handleSessionInit = (event, isInit) => { | ||
| setIsInitializing(isInit); | ||
| }; | ||
|
|
||
| window.electron.ipcRenderer.on('session-initializing', handleSessionInit); | ||
|
|
||
| return () => { | ||
| window.electron.ipcRenderer.removeAllListeners('session-initializing'); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
Use specific listener removal instead of removeAllListeners.
removeAllListeners('session-initializing') removes all listeners on this channel, not just the one added by this component. This could break other components listening to the same event. Store the handler reference and use removeListener instead.
useEffect(() => {
if (!window.electron?.ipcRenderer) return;
const handleSessionInit = (event, isInit) => {
setIsInitializing(isInit);
};
window.electron.ipcRenderer.on('session-initializing', handleSessionInit);
return () => {
- window.electron.ipcRenderer.removeAllListeners('session-initializing');
+ window.electron.ipcRenderer.removeListener('session-initializing', handleSessionInit);
};
}, []);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Listen for initialization status | |
| useEffect(() => { | |
| if (!window.electron?.ipcRenderer) return; | |
| const handleSessionInit = (event, isInit) => { | |
| setIsInitializing(isInit); | |
| }; | |
| window.electron.ipcRenderer.on('session-initializing', handleSessionInit); | |
| return () => { | |
| window.electron.ipcRenderer.removeAllListeners('session-initializing'); | |
| }; | |
| }, []); | |
| // Listen for initialization status | |
| useEffect(() => { | |
| if (!window.electron?.ipcRenderer) return; | |
| const handleSessionInit = (event, isInit) => { | |
| setIsInitializing(isInit); | |
| }; | |
| window.electron.ipcRenderer.on('session-initializing', handleSessionInit); | |
| return () => { | |
| window.electron.ipcRenderer.removeListener('session-initializing', handleSessionInit); | |
| }; | |
| }, []); |
🤖 Prompt for AI Agents
In src/renderer/components/views/MainView.jsx around lines 18 to 31, the effect
currently calls removeAllListeners('session-initializing') which will remove
listeners for other components; instead, keep the handler reference used when
calling ipcRenderer.on and call
window.electron.ipcRenderer.removeListener('session-initializing',
handleSessionInit) in the cleanup so only this component's listener is removed;
ensure handleSessionInit is declared as a stable constant inside the effect (or
memoized) so the same function reference is passed to both on and
removeListener.
| useEffect(() => { | ||
| const canvas = canvasRef.current; | ||
| if (!canvas) return; | ||
|
|
||
| const ctx = canvas.getContext('2d'); | ||
| resizeCanvas(); | ||
|
|
||
| const handleResize = () => resizeCanvas(); | ||
| window.addEventListener('resize', handleResize); | ||
|
|
||
| startGradientAnimation(ctx); | ||
|
|
||
| return () => { | ||
| if (animationIdRef.current) { | ||
| cancelAnimationFrame(animationIdRef.current); | ||
| } | ||
| window.removeEventListener('resize', handleResize); | ||
| }; | ||
| }, [currentSlide, isTransitioning]); |
There was a problem hiding this comment.
Animation cleanup may cause issues on re-render.
The useEffect that starts the gradient animation runs when currentSlide or isTransitioning changes. On each change, it cancels the previous animation and starts a new one. However, the cleanup runs asynchronously after the new effect starts, which could cause race conditions. Consider using a ref to track the latest animation state.
Also, consider moving the animation start outside of the effect and letting drawGradient read current state from refs:
useEffect(() => {
const canvas = canvasRef.current;
if (!canvas) return;
const ctx = canvas.getContext('2d');
resizeCanvas();
const handleResize = () => resizeCanvas();
window.addEventListener('resize', handleResize);
- startGradientAnimation(ctx);
+ // Start animation only once
+ if (!animationIdRef.current) {
+ startGradientAnimation(ctx);
+ }
return () => {
- if (animationIdRef.current) {
- cancelAnimationFrame(animationIdRef.current);
- }
window.removeEventListener('resize', handleResize);
};
-}, [currentSlide, isTransitioning]);
+}, []);
+
+// Separate cleanup effect
+useEffect(() => {
+ return () => {
+ if (animationIdRef.current) {
+ cancelAnimationFrame(animationIdRef.current);
+ }
+ };
+}, []);🤖 Prompt for AI Agents
In src/renderer/components/views/OnboardingView.jsx around lines 45 to 63, the
effect starts and cancels the gradient animation on every change of currentSlide
or isTransitioning causing potential race conditions; change to use a ref (e.g.,
latestAnimationRef / isMountedRef) to track the active animation id/state and
only cancel the animation id stored in that ref during cleanup, move the actual
startGradient call outside the effect (or into a single-mounted effect) so it
isn't restarted on every slide change, refactor drawGradient to read
currentSlide/isTransitioning from refs instead of closure values, and ensure the
resize listener cleanup remains but is attached only once to avoid duplicate
handlers.
| // Wait for window.cheatingDaddy to be available before rendering | ||
| function initApp() { | ||
| if (window.cheatingDaddy) { | ||
| console.log('✅ window.cheatingDaddy is available'); | ||
| const root = ReactDOM.createRoot(document.getElementById('root')); | ||
| root.render( | ||
| <React.StrictMode> | ||
| <App /> | ||
| </React.StrictMode> | ||
| ); | ||
| } else { | ||
| console.log('⏳ Waiting for window.cheatingDaddy...'); | ||
| setTimeout(initApp, 50); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add timeout to prevent infinite polling loop.
The initApp function polls indefinitely every 50ms for window.cheatingDaddy. If the preload script fails or takes too long, this creates an infinite loop with no user feedback.
Apply this diff to add a timeout:
+const MAX_WAIT_TIME = 5000; // 5 seconds
+const startTime = Date.now();
+
function initApp() {
if (window.cheatingDaddy) {
console.log('✅ window.cheatingDaddy is available');
const root = ReactDOM.createRoot(document.getElementById('root'));
root.render(
<React.StrictMode>
<App />
</React.StrictMode>
);
} else {
+ if (Date.now() - startTime > MAX_WAIT_TIME) {
+ console.error('❌ Timeout waiting for window.cheatingDaddy');
+ document.body.innerHTML = '<div style="color: red; padding: 20px;">Failed to initialize. Please restart the application.</div>';
+ return;
+ }
console.log('⏳ Waiting for window.cheatingDaddy...');
setTimeout(initApp, 50);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Wait for window.cheatingDaddy to be available before rendering | |
| function initApp() { | |
| if (window.cheatingDaddy) { | |
| console.log('✅ window.cheatingDaddy is available'); | |
| const root = ReactDOM.createRoot(document.getElementById('root')); | |
| root.render( | |
| <React.StrictMode> | |
| <App /> | |
| </React.StrictMode> | |
| ); | |
| } else { | |
| console.log('⏳ Waiting for window.cheatingDaddy...'); | |
| setTimeout(initApp, 50); | |
| } | |
| } | |
| // Wait for window.cheatingDaddy to be available before rendering | |
| const MAX_WAIT_TIME = 5000; // 5 seconds | |
| const startTime = Date.now(); | |
| function initApp() { | |
| if (window.cheatingDaddy) { | |
| console.log('✅ window.cheatingDaddy is available'); | |
| const root = ReactDOM.createRoot(document.getElementById('root')); | |
| root.render( | |
| <React.StrictMode> | |
| <App /> | |
| </React.StrictMode> | |
| ); | |
| } else { | |
| if (Date.now() - startTime > MAX_WAIT_TIME) { | |
| console.error('❌ Timeout waiting for window.cheatingDaddy'); | |
| document.body.innerHTML = '<div style="color: red; padding: 20px;">Failed to initialize. Please restart the application.</div>'; | |
| return; | |
| } | |
| console.log('⏳ Waiting for window.cheatingDaddy...'); | |
| setTimeout(initApp, 50); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/renderer/index.jsx around lines 6 to 20, the initApp polling loop waits
forever for window.cheatingDaddy and needs a timeout to avoid infinite retries;
modify initApp to accept and track a timeout or maxAttempts (e.g., maxAttempts
or deadline timestamp), increment attempts on each retry (or compare Date.now()
to deadline), and if exceeded stop retrying and either render a safe fallback UI
or display a clear error message to the user (and log the failure); ensure the
polling is cleared/stopped and avoid further setTimeout calls after timeout is
reached so the loop cannot run indefinitely.
| height: 100%; | ||
| overflow: hidden; | ||
| background: transparent; | ||
| font-family: 'Inter', -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if Inter font is imported anywhere
echo "Checking for Inter font imports..."
rg -n "@import.*inter|@font-face.*Inter" --type css
# Check index.html for font links
if [ -f "src/renderer/index.html" ]; then
echo ""
echo "Checking index.html for font links..."
grep -n "fonts.googleapis.com.*inter\|fonts.gstatic.com" src/renderer/index.html || echo "No Google Fonts link found"
fiRepository: sohzm/cheating-daddy
Length of output: 164
🏁 Script executed:
#!/bin/bash
# Comprehensive font loading check
echo "=== Checking all HTML files ==="
find . -name "*.html" -type f | head -20
echo ""
echo "=== Checking package.json for font packages ==="
if [ -f "package.json" ]; then
grep -i "font\|inter" package.json
fi
echo ""
echo "=== Checking for font files in project ==="
find . -type f \( -name "*.woff*" -o -name "*.ttf" -o -name "*.otf" \) | head -20
echo ""
echo "=== Checking for CSS @import statements ==="
rg "@import" --type css -A 2 | head -30
echo ""
echo "=== Checking for font loading in JS/TS files ==="
rg "Inter|font" src/renderer --type ts --type js | grep -i "import\|require\|font-family" | head -20Repository: sohzm/cheating-daddy
Length of output: 433
🏁 Script executed:
#!/bin/bash
# Final verification checks
echo "=== Content of src/renderer/index.html ==="
cat src/renderer/index.html
echo ""
echo "=== Content of global.css around line 86 ==="
sed -n '80,92p' src/renderer/styles/global.cssRepository: sohzm/cheating-daddy
Length of output: 1223
Load the Inter font or remove it from the font stack.
The stylesheet references 'Inter' on line 86 of src/renderer/styles/global.css but provides no @import, @font-face, or <link> tag to load it. The app will silently fall back to system fonts instead of using Inter.
Add one of the following:
- A
<link>tag insrc/renderer/index.html(e.g., Google Fonts:<link rel="stylesheet" href="https://fonts.googleapis.com/css2?family=Inter:wght@400;600;700&display=swap">) - An
@importrule in the CSS file - A font package like
@fontsource/interwith corresponding import - Local font files with
@font-facedeclarations
🤖 Prompt for AI Agents
In src/renderer/styles/global.css around line 86 the font-family references
'Inter' but the font is not being loaded; fix by loading Inter: either add a
Google Fonts <link> tag into src/renderer/index.html head (e.g., include the
Inter stylesheet URL), or add an @import at the top of
src/renderer/styles/global.css to load Inter, or install and import a package
like @fontsource/inter in your renderer entry, or add local @font-face
declarations pointing to bundled font files; choose one approach and ensure the
import/link is included before the stylesheet that uses the font so 'Inter' is
available at runtime.
| font-family: 'Inter', -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif; | ||
| color: var(--text-color); | ||
| cursor: default; | ||
| user-select: none; |
There was a problem hiding this comment.
User-select: none prevents text selection globally.
Setting user-select: none on the body prevents users from selecting any text, which can be frustrating when they want to copy error messages, responses, or other content.
Consider applying this selectively to UI chrome (buttons, headers) rather than globally:
body,
#root {
width: 100%;
height: 100%;
overflow: hidden;
background: transparent;
font-family: 'Inter', -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif;
color: var(--text-color);
cursor: default;
- user-select: none;
}Then add user-select: none; to specific UI elements like buttons and headers where it's needed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| user-select: none; | |
| body, | |
| #root { | |
| width: 100%; | |
| height: 100%; | |
| overflow: hidden; | |
| background: transparent; | |
| font-family: 'Inter', -apple-system, BlinkMacSystemFont, 'Segoe UI', sans-serif; | |
| color: var(--text-color); | |
| cursor: default; | |
| } |
🤖 Prompt for AI Agents
In src/renderer/styles/global.css around line 89, the global rule setting
"user-select: none" on the body prevents all text selection; remove that global
declaration and instead add "user-select: none" to specific UI chrome selectors
(e.g., buttons, .header, .toolbar, .nav) so interactive non-text elements remain
unselectable while page content, error messages and responses stay selectable;
update the CSS by deleting or overriding the body-level user-select and adding
targeted rules for the UI elements that should not be selectable.
1 participant
Summary by CodeRabbit
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.