fix: app icon in macos dock and system tray#15
fix: app icon in macos dock and system tray#15mylinkedai wants to merge 1 commit intogaoyangz77:mainfrom
Conversation
gaoyangz77
left a comment
There was a problem hiding this comment.
感谢提交!图标优化的方向是对的,不过有几个问题需要修复:
必须修复
1. pnpm-lock.yaml 包含大量无关删除
lockfile 中删除了整个 client/mobile 的依赖声明(~8000 行),与图标修复无关。很可能是本地缺少 client/mobile 目录导致 pnpm install 自动清理了 lockfile。请撤回这部分改动,否则会破坏 mobile 客户端的构建。
2. DEV 和 PACKAGED 路径完全相同
const DEV_TRAY_ICON_PATH = resolve(__dirname, "../build/[email protected]");
const PACKAGED_TRAY_ICON_PATH = resolve(__dirname, "../build/[email protected]");两个常量值一样,app.isPackaged 的分支判断没有实际意义。注释说 "In dev, load the panel logo directly",但实际并没有区分。请修正路径或去掉无意义的区分。
建议改进
3. Dock 图标分辨率不足
setDockIcon() 从 [email protected](64×64)加载图标。macOS dock 图标推荐 512×512 或至少 128×128,64×64 在 dock 中会明显模糊。建议使用 icon.png 或专门的高分辨率图标。
4. Tray 图标缩放尺寸偏小
TRAY_ICON_SCALE = 0.6 将 64×64 缩小到 ~38×38,scaleFactor: 2.0 后显示为 ~19×19 逻辑像素。macOS 系统托盘标准是 22×22(44×44 @2x),当前偏小,请确认实际显示效果。
5. 旧 favicon 未清理
index.html 改用 /logo.png,但 apps/panel/public/favicon.ico 仍存在,可以顺手删除。
gaoyangz77
left a comment
There was a problem hiding this comment.
Thanks for the contribution! The icon improvement direction looks good, but there are a few issues to address:
Must Fix
1. pnpm-lock.yaml contains massive unrelated deletions
The lockfile removes the entire client/mobile dependency block (~8000 lines), which is unrelated to the icon fix. This likely happened because client/mobile (a separate git repo) was missing locally, causing pnpm install to prune the lockfile. Please revert this change — merging it would break the mobile client build.
2. DEV and PACKAGED paths are identical
const DEV_TRAY_ICON_PATH = resolve(__dirname, "../build/[email protected]");
const PACKAGED_TRAY_ICON_PATH = resolve(__dirname, "../build/[email protected]");Both constants resolve to the same path, making the app.isPackaged branch meaningless. The comment says "In dev, load the panel logo directly" but it doesn't actually do that. Please either differentiate the paths or remove the unnecessary branching.
Suggestions
3. Dock icon resolution is too low
setDockIcon() loads from [email protected] (64×64). macOS dock icons should ideally be 512×512 or at least 128×128 — a 64×64 source will look blurry in the dock. Consider using icon.png or a dedicated high-resolution icon file instead.
4. Tray icon scaling may be too small
TRAY_ICON_SCALE = 0.6 shrinks the 64×64 source to ~38×38, and with scaleFactor: 2.0 it displays at ~19×19 logical pixels. The macOS menu bar standard is 22×22 logical (44×44 @2x). The current size is slightly undersized — please verify the actual appearance.
5. Old favicon not cleaned up
index.html now uses /logo.png, but apps/panel/public/favicon.ico still exists. Consider removing it.
The client/mobile app is a separate private repo with no @easyclaw/* dependencies. Listing it in the workspace added ~8000 lines of unrelated entries to pnpm-lock.yaml and confused open-source contributors (cf. PR #15). Co-Authored-By: Claude Opus 4.6 <[email protected]>
gaoyangz77
left a comment
There was a problem hiding this comment.
Thanks for the fix here. I dug into the packaging side before merging, and I don't think the main macOS dock-icon change will work in release builds yet.
The new setDockIcon() code in apps/desktop/src/main.ts loads ../build/icon.png at runtime, but our packaged app does not currently ship that file. In apps/desktop/electron-builder.yml, the files: section includes build/trayIcon.png and build/[email protected], but not build/icon.png. I also checked the existing packaged app.asar contents locally and only found /build/trayIcon.png and /build/[email protected], not /build/icon.png. That means nativeImage.createFromPath(...) will be empty in the release app, so this fix likely won't take effect where we need it most.
The rest of the PR looks fine to me: the panel favicon switch builds cleanly, and the tray icon resize change seems reasonable. But I think we should revise the dock-icon piece before merging.
Could you update this to either:
- package
build/icon.pngexplicitly, or - point
setDockIcon()at an asset path we already know exists in packaged builds
Once that's adjusted, I'd be happy to take another look.
|
Closing — we've already fixed this on our end. Thanks for the contribution! |
No description provided.