feat: conditional app icon and macOS dev dock#13428
feat: conditional app icon and macOS dev dock#13428Pleasurecruise wants to merge 1 commit intomainfrom
Conversation
Add a dev logo asset and use a platform-aware icon for BrowserWindow. Import build/logo-dev.png and replace the previous linux-only icon with a createAppIcon() helper that returns: - undefined on macOS (BrowserWindow icon ignored) - logo-dev in development - icon.png on Linux in production Apply the resulting appIcon to window options when present, and set the macOS dock icon to the dev logo in development. This keeps production behavior intact (Windows uses the exe icon and macOS uses the bundle .icns) while providing a visible icon during development.
There was a problem hiding this comment.
Pull request overview
Adds development-time app icon handling across platforms, including a macOS Dock icon override in dev mode, while preserving production icon behavior.
Changes:
- Introduces a
createAppIcon()helper to choose aBrowserWindowicon based on platform and dev/prod mode. - Sets the macOS Dock icon to
logo-dev.pngin development mode. - Adds a new dev icon asset (
build/logo-dev.png).
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/main/services/WindowService.ts | Centralizes icon selection for BrowserWindow and sets macOS Dock icon in dev mode. |
| build/logo-dev.png | Adds a dedicated development icon asset. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // Create nativeImage for Linux window icon (required for Wayland) | ||
| const linuxIcon = isLinux ? nativeImage.createFromPath(iconPath) : undefined | ||
| // Window icon for taskbar/title bar — only meaningful on Windows and Linux. |
There was a problem hiding this comment.
Note
This issue/comment/review was translated by Claude.
But there is no special handling for Windows here, the comment is somewhat misleading.
Original Content
但实际上这里没有对Windows做任何处理，注释有些误导性
There was a problem hiding this comment.
Note
This review was translated by Claude.
Code Review Summary
🔴 Changes Required
The code mixes two development mode detection methods: isDev and is.dev:
- Line 6 import
isDevfrom@main/constant - Line 4 import
isfrom@electron-toolkit/utils(includesis.dev) - Other locations (lines 351, 595) all use
is.dev
Problems:
isDevonly checksprocess.env.NODE_ENV === 'development'is.devcomes from@electron-toolkit/utilsand uses more reliable detection logic- In Electron main process,
NODE_ENVmay not be set correctly
This leads to:
- macOS Dock icon not displaying in development mode (confirmed by @EurFelux)
- Windows taskbar icon not displaying in development mode
Fix Suggestion
Unify usage to is.dev:
// Line 32
if (is.dev) return nativeImage.createFromPath(logoDevPath)
// Line 109
if (is.dev && isMac) {
app.dock?.setIcon(logoDevPath)
}Other
- Functional design is reasonable, backward compatible ✅
- Clear comments, good code structure ✅
- logo-dev.png size is appropriate (1024x1024) ✅
Please fix and request review again.
Original Content
代码审查总结
🔴 需要修改
代码中混用了 isDev 和 is.dev 两种开发模式判断方式：
- 第6行导入
isDevfrom@main/constant - 第4行导入
isfrom@electron-toolkit/utils（包含is.dev） - 文件中其他位置（第351、595行）都使用
is.dev
问题：
isDev只检查process.env.NODE_ENV === 'development'is.dev来自@electron-toolkit/utils，使用更可靠的判断逻辑- 在 Electron 主进程中，
NODE_ENV可能未被正确设置
这导致：
- macOS Dock 图标在开发模式下不显示（@EurFelux 已确认）
- Windows 开发模式下任务栏图标不显示
修复建议
统一使用 is.dev：
// 第32行
if (is.dev) return nativeImage.createFromPath(logoDevPath)
// 第109行
if (is.dev && isMac) {
app.dock?.setIcon(logoDevPath)
}其他
- 功能设计合理，向后兼容 ✅
- 注释清晰，代码结构良好 ✅
- logo-dev.png 尺寸合适（1024x1024）✅
请修复后重新请求审查。
| // Windows production: icon is embedded in the .exe by electron-builder, no override needed. | ||
| function createAppIcon() { | ||
| if (isMac) return undefined | ||
| if (isDev) return nativeImage.createFromPath(logoDevPath) |
There was a problem hiding this comment.
Note
This comment was translated by Claude.
Issue: Mixed use of isDev and is.dev causes feature to not work
The code mixes two different development mode checks:
- Line 6 imports
isDevfrom@main/constant - Line 4 imports
isfrom@electron-toolkit/utils(which includesis.dev)
isDev only checks NODE_ENV, while is.dev uses more reliable Electron tool detection. Other locations in the file (lines 351, 595) all use is.dev.
Suggestion: Consistently use is.dev
if (is.dev) return nativeImage.createFromPath(logoDevPath)Original Content
问题：isDev 与 is.dev 混用导致功能不生效
代码中混用了两种不同的开发模式判断：
- 第6行导入
isDevfrom@main/constant - 第4行导入
isfrom@electron-toolkit/utils（包含is.dev）
isDev 只检查 NODE_ENV，而 is.dev 使用更可靠的 Electron 工具判断。文件中其他位置（第351、595行）都使用 is.dev。
建议： 统一使用 is.dev
if (is.dev) return nativeImage.createFromPath(logoDevPath)|
|
||
| this.setupMainWindow(this.mainWindow, mainWindowState) | ||
|
|
||
| // Set macOS dock icon in dev mode (production uses the .icns from the app bundle). |
There was a problem hiding this comment.
Note
This issue/comment/review was translated by Claude.
Same issue: should use is.dev instead of isDev
Similar to line 32 above, the isDev check here may be incorrect, causing the macOS Dock icon to not display in development mode.
Suggestion:
if (is.dev && isMac) {
app.dock?.setIcon(logoDevPath)
}Original Content
同样的问题：应使用 is.dev 而非 isDev
与上面第32行相同，这里的 isDev 判断可能不正确，导致 macOS Dock 图标在开发模式下无法显示。
建议：
if (is.dev && isMac) {
app.dock?.setIcon(logoDevPath)
}
What this PR does
Before this PR:
dev mode had no custom Dock icon
After this PR:
Fixes #
Why we need it and why it was done in this way
The following tradeoffs were made:
Add a dev logo asset and use a platform-aware icon for BrowserWindow. Import build/logo-dev.png and replace the previous linux-only icon with a createAppIcon() helper that returns:
The following alternatives were considered:
Links to places where the discussion took place:
Breaking changes
If this PR introduces breaking changes, please describe the changes and the impact on users.
Special notes for your reviewer
Checklist
This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.
/gh-pr-review,gh pr diff, or GitHub UI) before requesting review from othersRelease note