-
Notifications
You must be signed in to change notification settings - Fork 3.9k
feat: conditional app icon and macOS dev dock #13428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ import windowStateKeeper from 'electron-window-state' | |
| import { join } from 'path' | ||
|
|
||
| import iconPath from '../../../build/icon.png?asset' | ||
| import logoDevPath from '../../../build/logo-dev.png?asset' | ||
| import { titleBarOverlayDark, titleBarOverlayLight } from '../config' | ||
| import { configManager } from './ConfigManager' | ||
| import { contextMenu } from './ContextMenu' | ||
|
|
@@ -23,8 +24,16 @@ const DEFAULT_MINIWINDOW_HEIGHT = 400 | |
| // const logger = loggerService.withContext('WindowService') | ||
| const logger = loggerService.withContext('WindowService') | ||
|
|
||
| // 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. | ||
| // macOS ignores the BrowserWindow `icon` option; dock icon is managed separately via app.dock.setIcon(). | ||
| // 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) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note This comment was translated by Claude. Issue: Mixed use of The code mixes two different development mode checks:
Suggestion: Consistently use if (is.dev) return nativeImage.createFromPath(logoDevPath)Original Content问题: 代码中混用了两种不同的开发模式判断:
建议: 统一使用 if (is.dev) return nativeImage.createFromPath(logoDevPath) |
||
| if (isLinux) return nativeImage.createFromPath(iconPath) | ||
| return undefined | ||
| } | ||
| const appIcon = createAppIcon() | ||
|
|
||
| export class WindowService { | ||
| private static instance: WindowService | null = null | ||
|
|
@@ -83,7 +92,7 @@ export class WindowService { | |
| }), | ||
| backgroundColor: isMac ? undefined : nativeTheme.shouldUseDarkColors ? '#181818' : '#FFFFFF', | ||
| darkTheme: nativeTheme.shouldUseDarkColors, | ||
| ...(isLinux ? { icon: linuxIcon } : {}), | ||
| ...(appIcon ? { icon: appIcon } : {}), | ||
| webPreferences: { | ||
| preload: join(__dirname, '../preload/index.js'), | ||
| sandbox: false, | ||
|
|
@@ -97,6 +106,11 @@ export class WindowService { | |
|
|
||
| this.setupMainWindow(this.mainWindow, mainWindowState) | ||
|
|
||
| // Set macOS dock icon in dev mode (production uses the .icns from the app bundle). | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note This issue/comment/review was translated by Claude. Same issue: should use Similar to line 32 above, the Suggestion: if (is.dev && isMac) {
app.dock?.setIcon(logoDevPath)
}Original Content同样的问题:应使用 与上面第32行相同,这里的 建议: if (is.dev && isMac) {
app.dock?.setIcon(logoDevPath)
} |
||
| if (isDev && isMac) { | ||
| app.dock?.setIcon(logoDevPath) | ||
| } | ||
|
|
||
| //preload miniWindow to resolve series of issues about miniWindow in Mac | ||
| const enableQuickAssistant = configManager.getEnableQuickAssistant() | ||
| if (enableQuickAssistant && !this.miniWindow) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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做任何处理,注释有些误导性