feat: custom title bar on Linux with native toggle#68
Conversation
Summary of ChangesHello @Psypeal, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's cross-platform user experience by introducing a custom title bar for Linux, mirroring the existing Windows implementation. It also provides users with greater control over their window aesthetics through a new setting that allows toggling between the custom and native title bars, with an integrated relaunch mechanism to apply changes seamlessly. Additionally, a long-standing issue with Electron's default reload shortcut interfering with application functionality has been addressed. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a custom title bar for Linux and adds a setting to toggle between the custom and native title bars, involving renaming WindowsTitleBar to CustomTitleBar and using an app:relaunch IPC channel. While no specific security vulnerabilities were found, a potential race condition was identified in how settings are saved before relaunching, which might prevent the setting change from being applied correctly. Additionally, a minor accessibility suggestion has been made for the new custom title bar.
| onGeneralToggle('useNativeTitleBar', v); | ||
| // Small delay to let config persist before relaunch | ||
| setTimeout(() => { | ||
| void window.electronAPI?.windowControls?.relaunch(); | ||
| }, 200); |
There was a problem hiding this comment.
Using setTimeout to wait for the configuration to save creates a race condition. The config update is an asynchronous operation, and the 200ms delay does not guarantee that it will complete before the application relaunches. This could result in the setting change not being applied until the next restart.
To fix this, you should await the config update before relaunching. This will require a few related changes:
- Update the
onGeneralToggleprop type in this file to return aPromise<void>:readonly onGeneralToggle: (key: 'launchAtLogin' | 'showDockIcon' | 'useNativeTitleBar', value: boolean) => Promise<void>;
- In
useSettingsHandlers.ts, makehandleGeneralToggleasyncandawaittheupdateConfigcall.
With those changes, you can safely await the operation here.
| onGeneralToggle('useNativeTitleBar', v); | |
| // Small delay to let config persist before relaunch | |
| setTimeout(() => { | |
| void window.electronAPI?.windowControls?.relaunch(); | |
| }, 200); | |
| await onGeneralToggle('useNativeTitleBar', v); | |
| void window.electronAPI?.windowControls?.relaunch(); |
| </span> | ||
| {/* Draggable area — app icon */} | ||
| <div className="flex flex-1 items-center pl-3" style={{ minWidth: 0 }}> | ||
| <img src={faviconUrl} alt="" className="size-5 shrink-0 rounded-sm" draggable={false} /> |
There was a problem hiding this comment.
For accessibility, the alt attribute on this <img> tag should be more descriptive. While an empty alt attribute is acceptable for purely decorative images, this icon represents the application and providing descriptive text like "Application icon" would be better for users relying on screen readers.
| <img src={faviconUrl} alt="" className="size-5 shrink-0 rounded-sm" draggable={false} /> | |
| <img src={faviconUrl} alt="Application icon" className="size-5 shrink-0 rounded-sm" draggable={false} /> |
ce7070d to
bab478c
Compare
📝 WalkthroughWalkthroughAdds a config-driven Changes
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/components/layout/TabbedLayout.tsx (1)
9-21:⚠️ Potential issue | 🟠 MajorFix import order to satisfy simple-import-sort (CI failure).
The pipeline is failing on import sorting. Please reorder imports to match the required grouping.
🧹 Suggested reorder
import { isElectronMode } from '@renderer/api'; import { getTrafficLightPaddingForZoom } from '@renderer/constants/layout'; import { useKeyboardShortcuts } from '@renderer/hooks/useKeyboardShortcuts'; import { useZoomFactor } from '@renderer/hooks/useZoomFactor'; -import { UpdateBanner } from '../common/UpdateBanner'; -import { UpdateDialog } from '../common/UpdateDialog'; -import { WorkspaceIndicator } from '../common/WorkspaceIndicator'; -import { CommandPalette } from '../search/CommandPalette'; - +import { CommandPalette } from '../search/CommandPalette'; +import { UpdateBanner } from '../common/UpdateBanner'; +import { UpdateDialog } from '../common/UpdateDialog'; +import { WorkspaceIndicator } from '../common/WorkspaceIndicator'; import { PaneContainer } from './PaneContainer'; import { Sidebar } from './Sidebar'; import { CustomTitleBar } from './CustomTitleBar';As per coding guidelines: Organize imports in order: external packages, path aliases (
@main,@renderer,@shared,@preload), then relative imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/layout/TabbedLayout.tsx` around lines 9 - 21, Reorder the imports in TabbedLayout.tsx to satisfy simple-import-sort: group and sort external packages first (none here), then path-alias imports from `@renderer` (e.g., isElectronMode, getTrafficLightPaddingForZoom, useKeyboardShortcuts, useZoomFactor), then other path aliases if present, and finally local/relative imports (components like UpdateBanner, UpdateDialog, WorkspaceIndicator, CommandPalette, PaneContainer, Sidebar, CustomTitleBar). Ensure each group is contiguous and alphabetically ordered within the group so simple-import-sort CI passes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/index.ts`:
- Around line 484-494: The current guard `if (!input.meta) return;` prevents
zoom shortcuts on Windows/Linux because they use `input.control` (Ctrl) instead
of `input.meta` (Cmd); update the guard in the keyboard handler (around the
earlier Ctrl/Cmd+R handling and zoom processing) to allow either modifier by
changing the check to require `input.meta || input.control` (i.e., replace the
`!input.meta` gate with a negation of `(input.meta || input.control)`) so zoom
keys (Ctrl+=, Ctrl+-, Ctrl+0 and Cmd equivalents) are processed on all
platforms.
In `@src/main/ipc/window.ts`:
- Line 8: The relaunch IPC handler in this module needs to be wrapped in a
try/catch and return a graceful value on failure: locate the handler for the
relaunch action (e.g., the ipcMain.handle for "relaunch" or the function that
calls app.relaunch()/app.exit()) and change it so the relaunch call is inside a
try block, return a success token (true or similar) on success, and on error
catch the exception, call console.error(...) with the caught error and return
null; also validate any incoming parameters before using them and avoid throwing
from the handler so callers get null on failure.
In `@src/renderer/components/settings/sections/GeneralSection.tsx`:
- Around line 26-31: The toggle handlers in GeneralSectionProps currently assume
a fixed delay and may relaunch the app before config is persisted; change the
onGeneralToggle signature to return a Promise<boolean|void> (update
GeneralSectionProps: onGeneralToggle(...) => Promise) and in the component where
toggles trigger a relaunch await onGeneralToggle(...) to resolve before calling
any restart/relaunch logic (search for usages of onGeneralToggle in this file
and the toggle handler around the restart flow and replace setTimeout-based
waits with an await on the returned promise).
---
Outside diff comments:
In `@src/renderer/components/layout/TabbedLayout.tsx`:
- Around line 9-21: Reorder the imports in TabbedLayout.tsx to satisfy
simple-import-sort: group and sort external packages first (none here), then
path-alias imports from `@renderer` (e.g., isElectronMode,
getTrafficLightPaddingForZoom, useKeyboardShortcuts, useZoomFactor), then other
path aliases if present, and finally local/relative imports (components like
UpdateBanner, UpdateDialog, WorkspaceIndicator, CommandPalette, PaneContainer,
Sidebar, CustomTitleBar). Ensure each group is contiguous and alphabetically
ordered within the group so simple-import-sort CI passes.
| if (input.type !== 'keyDown') return; | ||
|
|
||
| // Prevent Electron's default Ctrl+R / Cmd+R page reload so the renderer | ||
| // keyboard handler can use it as "Refresh Session" (fixes #58). | ||
| // Also prevent Ctrl+Shift+R / Cmd+Shift+R (hard reload). | ||
| if ((input.control || input.meta) && input.key.toLowerCase() === 'r') { | ||
| event.preventDefault(); | ||
| return; | ||
| } | ||
|
|
||
| if (!input.meta) return; |
There was a problem hiding this comment.
Zoom handling may be broken on Windows/Linux.
The guard if (!input.meta) return; at Line 494 prevents zoom key processing when the meta key (Cmd) is not pressed. On Windows/Linux, zoom shortcuts use Ctrl (sets input.control), not Cmd (sets input.meta).
This means zoom via Ctrl+=/Ctrl+-/Ctrl+0 will not work on Windows/Linux after this change.
🔧 Proposed fix
if ((input.control || input.meta) && input.key.toLowerCase() === 'r') {
event.preventDefault();
return;
}
- if (!input.meta) return;
+ // Zoom shortcuts: Cmd on macOS, Ctrl on Windows/Linux
+ const hasModifier = process.platform === 'darwin' ? input.meta : input.control;
+ if (!hasModifier) return;
const currentLevel = mainWindow.webContents.getZoomLevel();📝 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.
| if (input.type !== 'keyDown') return; | |
| // Prevent Electron's default Ctrl+R / Cmd+R page reload so the renderer | |
| // keyboard handler can use it as "Refresh Session" (fixes #58). | |
| // Also prevent Ctrl+Shift+R / Cmd+Shift+R (hard reload). | |
| if ((input.control || input.meta) && input.key.toLowerCase() === 'r') { | |
| event.preventDefault(); | |
| return; | |
| } | |
| if (!input.meta) return; | |
| if (input.type !== 'keyDown') return; | |
| // Prevent Electron's default Ctrl+R / Cmd+R page reload so the renderer | |
| // keyboard handler can use it as "Refresh Session" (fixes `#58`). | |
| // Also prevent Ctrl+Shift+R / Cmd+Shift+R (hard reload). | |
| if ((input.control || input.meta) && input.key.toLowerCase() === 'r') { | |
| event.preventDefault(); | |
| return; | |
| } | |
| // Zoom shortcuts: Cmd on macOS, Ctrl on Windows/Linux | |
| const hasModifier = process.platform === 'darwin' ? input.meta : input.control; | |
| if (!hasModifier) return; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/index.ts` around lines 484 - 494, The current guard `if
(!input.meta) return;` prevents zoom shortcuts on Windows/Linux because they use
`input.control` (Ctrl) instead of `input.meta` (Cmd); update the guard in the
keyboard handler (around the earlier Ctrl/Cmd+R handling and zoom processing) to
allow either modifier by changing the check to require `input.meta ||
input.control` (i.e., replace the `!input.meta` gate with a negation of
`(input.meta || input.control)`) so zoom keys (Ctrl+=, Ctrl+-, Ctrl+0 and Cmd
equivalents) are processed on all platforms.
|
|
||
| import { createLogger } from '@shared/utils/logger'; | ||
| import { BrowserWindow, type IpcMain } from 'electron'; | ||
| import { app, BrowserWindow, type IpcMain } from 'electron'; |
There was a problem hiding this comment.
Add try/catch + graceful fallback for relaunch handler.
The new handler exits the app without any error handling or return value, which violates the IPC guidance for graceful degradation. Consider wrapping the relaunch in try/catch, logging the error, and returning null on failure.
🔧 Suggested fix
ipcMain.handle('app:relaunch', () => {
- app.relaunch();
- app.exit(0);
+ try {
+ app.relaunch();
+ app.exit(0);
+ } catch (error) {
+ console.error('[IPC:window] relaunch failed', error);
+ return null;
+ }
});As per coding guidelines: Implement error handling with try/catch in main process and console.error for debugging; Implement IPC message handlers with parameter validation and graceful degradation; src/main/ipc/!(config).ts: Other IPC handlers (non-config) should return data directly or null on error.
Also applies to: 43-56
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/ipc/window.ts` at line 8, The relaunch IPC handler in this module
needs to be wrapped in a try/catch and return a graceful value on failure:
locate the handler for the relaunch action (e.g., the ipcMain.handle for
"relaunch" or the function that calls app.relaunch()/app.exit()) and change it
so the relaunch call is inside a try block, return a success token (true or
similar) on success, and on error catch the exception, call console.error(...)
with the caught error and return null; also validate any incoming parameters
before using them and avoid throwing from the handler so callers get null on
failure.
| interface GeneralSectionProps { | ||
| readonly safeConfig: SafeConfig; | ||
| readonly saving: boolean; | ||
| readonly onGeneralToggle: (key: 'launchAtLogin' | 'showDockIcon', value: boolean) => void; | ||
| readonly onGeneralToggle: (key: 'launchAtLogin' | 'showDockIcon' | 'useNativeTitleBar', value: boolean) => void; | ||
| readonly onThemeChange: (value: 'dark' | 'light' | 'system') => void; | ||
| } |
There was a problem hiding this comment.
Ensure config persists before relaunch.
The fixed 200ms delay can race the config write; the app may restart before the setting is saved, causing the toggle to revert on relaunch. Consider making onGeneralToggle return a Promise and awaiting completion before restarting.
✅ Safer flow
interface GeneralSectionProps {
readonly safeConfig: SafeConfig;
readonly saving: boolean;
- readonly onGeneralToggle: (key: 'launchAtLogin' | 'showDockIcon' | 'useNativeTitleBar', value: boolean) => void;
+ readonly onGeneralToggle: (
+ key: 'launchAtLogin' | 'showDockIcon' | 'useNativeTitleBar',
+ value: boolean
+ ) => Promise<void>;
readonly onThemeChange: (value: 'dark' | 'light' | 'system') => void;
}
@@
onChange={async (v) => {
const shouldRelaunch = await confirm({
title: 'Restart required',
message: 'The app needs to restart to apply the title bar change. Restart now?',
confirmLabel: 'Restart',
});
if (shouldRelaunch) {
- onGeneralToggle('useNativeTitleBar', v);
- // Small delay to let config persist before relaunch
- setTimeout(() => {
- void window.electronAPI?.windowControls?.relaunch();
- }, 200);
+ await onGeneralToggle('useNativeTitleBar', v);
+ await window.electronAPI?.windowControls?.relaunch();
}
}}Also applies to: 289-308
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/settings/sections/GeneralSection.tsx` around lines 26
- 31, The toggle handlers in GeneralSectionProps currently assume a fixed delay
and may relaunch the app before config is persisted; change the onGeneralToggle
signature to return a Promise<boolean|void> (update GeneralSectionProps:
onGeneralToggle(...) => Promise) and in the component where toggles trigger a
relaunch await onGeneralToggle(...) to resolve before calling any
restart/relaunch logic (search for usages of onGeneralToggle in this file and
the toggle handler around the restart flow and replace setTimeout-based waits
with an await on the returned promise).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/layout/CustomTitleBar.tsx`:
- Around line 8-13: Reorder the import statements in CustomTitleBar.tsx so
external packages come first: move the lucide-react import (Minus, Square, X)
above the aliased imports (isElectronMode, faviconUrl, useStore) to follow the
project's import ordering rule; ensure the final order is external packages,
then `@renderer` alias imports, then any relative imports, keeping the existing
identifiers (isElectronMode, faviconUrl, useStore, Minus, Square, X) unchanged.
---
Duplicate comments:
In `@src/main/index.ts`:
- Around line 484-495: The final guard currently blocks non-macOS Ctrl-key
shortcuts because it returns unless input.meta is true; change that guard to
allow either meta or control so Ctrl+/-/0 work on Windows/Linux by replacing the
`if (!input.meta) return;` check with a check that returns only if neither
`input.meta` nor `input.control` is set (i.e., `if (!(input.meta ||
input.control)) return;`), leaving the earlier Ctrl/Cmd+R preventDefault logic
with `input.control || input.meta` and the `input.key` checks intact.
In `@src/renderer/components/settings/sections/GeneralSection.tsx`:
- Around line 26-31: The hard-coded 200ms delay can race with config persistence
and cause toggles to revert; update the flow so toggles wait for the config
write to complete instead of using setTimeout. In practice modify the
toggle/restart sequence around onGeneralToggle and the code that triggers
relaunch (the block referenced at lines ~289-308) to await the async config save
promise (or use its completion callback) before calling any relaunch/restart
logic, and remove the fixed 200ms delay so the change is guaranteed flushed to
disk prior to app restart.
| import { useEffect, useState } from 'react'; | ||
|
|
||
| import { isElectronMode } from '@renderer/api'; | ||
| import faviconUrl from '@renderer/favicon.png'; | ||
| import { useStore } from '@renderer/store'; | ||
| import { Minus, Square, X } from 'lucide-react'; |
There was a problem hiding this comment.
Fix import ordering to match guidelines.
lucide-react (external) should be listed before alias imports.
🔧 Suggested reorder
-import { isElectronMode } from '@renderer/api';
-import faviconUrl from '@renderer/favicon.png';
-import { useStore } from '@renderer/store';
-import { Minus, Square, X } from 'lucide-react';
+import { Minus, Square, X } from 'lucide-react';
+
+import { isElectronMode } from '@renderer/api';
+import faviconUrl from '@renderer/favicon.png';
+import { useStore } from '@renderer/store';As per coding guidelines, "Organize imports in order: external packages, path aliases (@main, @renderer, @shared, @preload), then relative imports".
📝 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.
| import { useEffect, useState } from 'react'; | |
| import { isElectronMode } from '@renderer/api'; | |
| import faviconUrl from '@renderer/favicon.png'; | |
| import { useStore } from '@renderer/store'; | |
| import { Minus, Square, X } from 'lucide-react'; | |
| import { useEffect, useState } from 'react'; | |
| import { Minus, Square, X } from 'lucide-react'; | |
| import { isElectronMode } from '@renderer/api'; | |
| import faviconUrl from '@renderer/favicon.png'; | |
| import { useStore } from '@renderer/store'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/layout/CustomTitleBar.tsx` around lines 8 - 13,
Reorder the import statements in CustomTitleBar.tsx so external packages come
first: move the lucide-react import (Minus, Square, X) above the aliased imports
(isElectronMode, faviconUrl, useStore) to follow the project's import ordering
rule; ensure the final order is external packages, then `@renderer` alias imports,
then any relative imports, keeping the existing identifiers (isElectronMode,
faviconUrl, useStore, Minus, Square, X) unchanged.
|
Thanks for the PR, @Psypeal! This is a great addition for Linux users. However, since several other PRs have been merged recently, this branch now has merge conflicts in 7 files. Could you please Rebase or merge main into your branch and resolve the conflicts? Thanks. |
|
Thanks for noticing, matt. I will do that later.
Get Outlook for Android<https://aka.ms/AAb9ysg>
…________________________________
From: matt ***@***.***>
Sent: Monday, February 23, 2026 10:39:59 PM
To: matt1398/claude-devtools ***@***.***>
Cc: Psypeal ***@***.***>; Mention ***@***.***>
Subject: Re: [matt1398/claude-devtools] feat: custom title bar on Linux with native toggle (PR #68)
[https://avatars.githubusercontent.com/u/113107082?s=20&v=4]matt1398 left a comment (matt1398/claude-devtools#68)<#68 (comment)>
Thanks for the PR, @Psypeal<https://github.com/Psypeal>! This is a great addition for Linux users.
However, since several other PRs have been merged recently, this branch now has merge conflicts in 7 files. Could you please Rebase or merge main into your branch and resolve the conflicts?
Thanks.
—
Reply to this email directly, view it on GitHub<#68 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AOE55BLSNV7NANF63RHP35L4NMGL7AVCNFSM6AAAAACV33ZHJ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTSNBVGE2TCMZZG4>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
…leBar with CustomTitleBar that renders on both Windows and Linux. Add useNativeTitleBar settings toggle with app relaunch to switch between custom and native window frame.
bab478c to
6cc4299
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/components/layout/CustomTitleBar.tsx (1)
73-82:⚠️ Potential issue | 🟡 MinorMaximize button icon doesn't reflect current window state
When the window is maximized the button correctly announces "Restore" via
aria-label/title, but still renders<Square />(the maximize icon). The visual feedback is misleading.Minimize2from lucide-react "indicates fullscreen exit and window restoration functionality" and is the conventional choice for the restore state.🛠️ Proposed fix
-import { Minus, Square, X } from 'lucide-react'; +import { Maximize2, Minimize2, Minus, X } from 'lucide-react';<button type="button" className={`${buttonBase} ${buttonHover}`} style={{ color: 'var(--color-text-secondary)' }} onClick={() => void handleMaximize()} title={isMaximized ? 'Restore' : 'Maximize'} aria-label={isMaximized ? 'Restore' : 'Maximize'} > - <Square className="size-3.5" strokeWidth={2.5} /> + {isMaximized + ? <Minimize2 className="size-3.5" strokeWidth={2.5} /> + : <Maximize2 className="size-3.5" strokeWidth={2.5} />} </button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/layout/CustomTitleBar.tsx` around lines 73 - 82, The maximize button always renders the Square icon even when isMaximized is true, which misleads users; update the button rendering in CustomTitleBar so it conditionally shows <Square /> when isMaximized is false and <Minimize2 /> when isMaximized is true, keeping the existing aria-label/title and onClick handler (handleMaximize) intact; import Minimize2 from 'lucide-react' if not already imported and replace the static <Square /> usage with a ternary based on isMaximized to switch icons.
♻️ Duplicate comments (2)
src/renderer/components/settings/sections/GeneralSection.tsx (1)
300-318:⚠️ Potential issue | 🟠 MajorAwait config save before relaunch (avoid 200ms race). The fixed delay can still relaunch before the setting persists, causing the toggle to revert on restart.
🛠️ Suggested fix
interface GeneralSectionProps { readonly safeConfig: SafeConfig; readonly saving: boolean; - readonly onGeneralToggle: (key: keyof AppConfig['general'], value: boolean) => void; + readonly onGeneralToggle: (key: keyof AppConfig['general'], value: boolean) => Promise<void>; readonly onThemeChange: (value: 'dark' | 'light' | 'system') => void; }onChange={async (v) => { const shouldRelaunch = await confirm({ title: 'Restart required', message: 'The app needs to restart to apply the title bar change. Restart now?', confirmLabel: 'Restart', }); if (shouldRelaunch) { - onGeneralToggle('useNativeTitleBar', v); - // Small delay to let config persist before relaunch - setTimeout(() => { - void window.electronAPI?.windowControls?.relaunch(); - }, 200); + await onGeneralToggle('useNativeTitleBar', v); + await window.electronAPI?.windowControls?.relaunch(); } }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/settings/sections/GeneralSection.tsx` around lines 300 - 318, The restart may race ahead of the config persisting because of the fixed 200ms timeout; change the SettingsToggle onChange handler to await the config save before relaunching: call and await an async onGeneralToggle('useNativeTitleBar', v) (or await the underlying saveConfig promise used by onGeneralToggle), then call window.electronAPI?.windowControls?.relaunch() immediately (no setTimeout). Ensure onGeneralToggle (or the save function it uses) returns a Promise so the handler can await it.src/renderer/components/layout/CustomTitleBar.tsx (1)
10-13: Import ordering still unresolved from prior review.
lucide-react(external package) remains after the@rendereralias imports. This was already flagged in a previous review cycle.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/layout/CustomTitleBar.tsx` around lines 10 - 13, The import ordering is incorrect: external package imports (lucide-react: symbols Minus, Square, X) must appear before internal alias imports (`@renderer`: isElectronMode, faviconUrl, useStore); reorder the imports in CustomTitleBar.tsx so that the lucide-react import line comes before the `@renderer` imports and keep related imports grouped (external packages first, then internal aliases).
🧹 Nitpick comments (1)
src/renderer/api/httpClient.ts (1)
515-515: Consider adding aconsole.warnto match the pattern used by other unavailable browser-mode operations.
relaunchis user-triggered (settings toggle → confirmation dialog → calls this), so a silent no-op means the user sees the confirmation but nothing happens, with no diagnostic signal. Other "not-available-in-browser-mode" stubs in this class (e.g.openPath,selectFolders,findWslClaudeRoots,openInEditor) all emit aconsole.warn. The window-control stubs (minimize,maximize,close) are genuinely transparent in a browser, butrelaunchis not in the same category.♻️ Proposed refinement
- relaunch: async (): Promise<void> => {}, + relaunch: async (): Promise<void> => { + console.warn('[HttpAPIClient] relaunch is not available in browser mode'); + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/api/httpClient.ts` at line 515, The relaunch stub currently silently no-ops; update the relaunch async method to emit a console.warn like the other browser-mode stubs (e.g. openPath, selectFolders, findWslClaudeRoots, openInEditor) so user-triggered relaunch attempts produce a diagnostic message. Specifically, inside the relaunch: async (): Promise<void> => { ... } body, call console.warn with a clear message such as "relaunch is not available in browser mode" (include any relevant context if desired) and then return, preserving the Promise<void> signature.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/layout/CustomTitleBar.tsx`:
- Around line 29-31: The component currently only calls api.isMaximized() once
and never updates when the window is externally maximized/unmaximized; add
window 'maximize' and 'unmaximize' listeners in src/main/ipc/window.ts that emit
IPC events, expose preload API methods onMaximize(callback) and
onUnmaximize(callback), then update CustomTitleBar's useEffect (which currently
calls api.isMaximized().then(setIsMaximized)) to subscribe to api.onMaximize and
api.onUnmaximize and call setIsMaximized(true/false) accordingly (and cleanup
subscriptions on unmount); also change the icon rendering around isMaximized
(replace the unconditional <Square /> at the render that shows restore/chevron
when isMaximized is true and Square when false) so the button label/icon and
aria-label stay synchronized.
---
Outside diff comments:
In `@src/renderer/components/layout/CustomTitleBar.tsx`:
- Around line 73-82: The maximize button always renders the Square icon even
when isMaximized is true, which misleads users; update the button rendering in
CustomTitleBar so it conditionally shows <Square /> when isMaximized is false
and <Minimize2 /> when isMaximized is true, keeping the existing
aria-label/title and onClick handler (handleMaximize) intact; import Minimize2
from 'lucide-react' if not already imported and replace the static <Square />
usage with a ternary based on isMaximized to switch icons.
---
Duplicate comments:
In `@src/renderer/components/layout/CustomTitleBar.tsx`:
- Around line 10-13: The import ordering is incorrect: external package imports
(lucide-react: symbols Minus, Square, X) must appear before internal alias
imports (`@renderer`: isElectronMode, faviconUrl, useStore); reorder the imports
in CustomTitleBar.tsx so that the lucide-react import line comes before the
`@renderer` imports and keep related imports grouped (external packages first,
then internal aliases).
In `@src/renderer/components/settings/sections/GeneralSection.tsx`:
- Around line 300-318: The restart may race ahead of the config persisting
because of the fixed 200ms timeout; change the SettingsToggle onChange handler
to await the config save before relaunching: call and await an async
onGeneralToggle('useNativeTitleBar', v) (or await the underlying saveConfig
promise used by onGeneralToggle), then call
window.electronAPI?.windowControls?.relaunch() immediately (no setTimeout).
Ensure onGeneralToggle (or the save function it uses) returns a Promise so the
handler can await it.
---
Nitpick comments:
In `@src/renderer/api/httpClient.ts`:
- Line 515: The relaunch stub currently silently no-ops; update the relaunch
async method to emit a console.warn like the other browser-mode stubs (e.g.
openPath, selectFolders, findWslClaudeRoots, openInEditor) so user-triggered
relaunch attempts produce a diagnostic message. Specifically, inside the
relaunch: async (): Promise<void> => { ... } body, call console.warn with a
clear message such as "relaunch is not available in browser mode" (include any
relevant context if desired) and then return, preserving the Promise<void>
signature.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/main/index.tssrc/main/ipc/configValidation.tssrc/main/ipc/window.tssrc/main/services/infrastructure/ConfigManager.tssrc/preload/constants/ipcChannels.tssrc/preload/index.tssrc/renderer/api/httpClient.tssrc/renderer/components/layout/CustomTitleBar.tsxsrc/renderer/components/layout/TabbedLayout.tsxsrc/renderer/components/settings/hooks/useSettingsConfig.tssrc/renderer/components/settings/hooks/useSettingsHandlers.tssrc/renderer/components/settings/sections/GeneralSection.tsxsrc/shared/types/api.tssrc/shared/types/notifications.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- src/shared/types/api.ts
- src/main/ipc/window.ts
- src/renderer/components/layout/TabbedLayout.tsx
- src/preload/index.ts
- src/preload/constants/ipcChannels.ts
- src/main/ipc/configValidation.ts
- src/main/index.ts
- src/main/services/infrastructure/ConfigManager.ts
- src/renderer/components/settings/hooks/useSettingsConfig.ts
- src/shared/types/notifications.ts
| useEffect(() => { | ||
| if (api) void api.isMaximized().then(setIsMaximized); | ||
| }, [api]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/renderer/components/layout/CustomTitleBar.tsx | head -100Repository: matt1398/claude-devtools
Length of output: 4215
🏁 Script executed:
# Find preload/IPC related files to check for maximize event handlers
fd -t f -e ts -e tsx | xargs grep -l "onMaximize\|onUnmaximize" 2>/dev/null | head -20Repository: matt1398/claude-devtools
Length of output: 50
🏁 Script executed:
# Search for where the preload API is defined
fd -t f -e ts -e tsx | xargs grep -l "isMaximized" | head -20Repository: matt1398/claude-devtools
Length of output: 253
🏁 Script executed:
cat -n src/preload/index.ts | head -200Repository: matt1398/claude-devtools
Length of output: 8555
🏁 Script executed:
cat -n src/main/ipc/window.tsRepository: matt1398/claude-devtools
Length of output: 2202
🏁 Script executed:
cat -n src/shared/types/api.ts | head -150Repository: matt1398/claude-devtools
Length of output: 6706
🏁 Script executed:
cat -n src/preload/index.ts | grep -A 30 "windowControls"Repository: matt1398/claude-devtools
Length of output: 1551
🏁 Script executed:
# Check if there are window event listeners in the main process
grep -r "on\('maximize\|'unmaximize" src/main/ 2>/dev/null || echo "No window maximize/unmaximize event listeners found"Repository: matt1398/claude-devtools
Length of output: 119
🏁 Script executed:
# Also check for BrowserWindow event listeners
grep -r "win\.on(" src/main/ | grep -i "max\|restore" || echo "No event subscriptions found"Repository: matt1398/claude-devtools
Length of output: 96
🏁 Script executed:
# Look at the complete preload window controls section
grep -B 5 -A 20 "windowControls\s*:" src/preload/index.tsRepository: matt1398/claude-devtools
Length of output: 1155
Implement window maximize/unmaximize event listeners to keep button state synchronized
The component only samples isMaximized() on mount. On Linux, external maximize events (keyboard shortcuts, window manager snaps) don't update the React state, leaving the button title/aria-label and icon out of sync.
The main process currently has no event emitters for maximize/unmaximize. To fix this:
- Add window event listeners in
src/main/ipc/window.tsand emit IPC events - Expose
onMaximize()andonUnmaximize()callbacks in the preload API - Subscribe to these events in the component's useEffect (as outlined below)
Additionally, line 81 renders <Square /> unconditionally—it should toggle to a different icon (e.g., restore/chevron) when isMaximized is true.
🛠️ Proposed component fix (after preload changes)
useEffect(() => {
- if (api) void api.isMaximized().then(setIsMaximized);
- }, [api]);
+ if (!api) return;
+ void api.isMaximized().then(setIsMaximized);
+ const unsubMaximize = api.onMaximize?.(() => setIsMaximized(true));
+ const unsubUnmaximize = api.onUnmaximize?.(() => setIsMaximized(false));
+ return () => {
+ unsubMaximize?.();
+ unsubUnmaximize?.();
+ };
+ }, [api]);Also update line 81 to render the appropriate icon based on maximize state:
- <Square className="size-3.5" strokeWidth={2.5} />
+ {isMaximized ? <ChevronDown className="size-3.5" strokeWidth={2.5} /> : <Square className="size-3.5" strokeWidth={2.5} />}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/layout/CustomTitleBar.tsx` around lines 29 - 31, The
component currently only calls api.isMaximized() once and never updates when the
window is externally maximized/unmaximized; add window 'maximize' and
'unmaximize' listeners in src/main/ipc/window.ts that emit IPC events, expose
preload API methods onMaximize(callback) and onUnmaximize(callback), then update
CustomTitleBar's useEffect (which currently calls
api.isMaximized().then(setIsMaximized)) to subscribe to api.onMaximize and
api.onUnmaximize and call setIsMaximized(true/false) accordingly (and cleanup
subscriptions on unmount); also change the icon rendering around isMaximized
(replace the unconditional <Square /> at the render that shows restore/chevron
when isMaximized is true and Square when false) so the button label/icon and
aria-label stay synchronized.
Summary
Changes
Test plan
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Style