-
Notifications
You must be signed in to change notification settings - Fork 0
Fix Windows system tray and add macOS ICNS icon support #1
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
Conversation
Co-authored-by: bryfur <7673964+bryfur@users.noreply.github.com>
Co-authored-by: bryfur <7673964+bryfur@users.noreply.github.com>
Co-authored-by: bryfur <7673964+bryfur@users.noreply.github.com>
|
@copilot can you also then add support for the mac specific icon format? |
Co-authored-by: bryfur <7673964+bryfur@users.noreply.github.com>
Co-authored-by: bryfur <7673964+bryfur@users.noreply.github.com>
Added macOS ICNS icon support in commit 8a5cc30. The changes include:
The ICNS file provides native macOS icon format with retina display support for crisp rendering at all sizes in the menu bar. |
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.
Pull request overview
This PR fixes the Windows system tray icon issue where the default Windows icon was displayed instead of the custom WoW Sync icon. The root cause was that the Windows Win32 API's LoadImage() function with IMAGE_ICON type requires an ICO format file, but the code was attempting to load a PNG file. The fix implements proper multi-resolution ICO file generation for Windows and ICNS file generation for macOS.
Key changes:
- Generated multi-resolution ICO file (16x16 through 256x256) for Windows system tray compatibility
- Generated multi-resolution ICNS file with retina support for macOS menu bar icons
- Updated platform-specific tray implementations to use the appropriate icon formats
Reviewed changes
Copilot reviewed 4 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| wow_sync/tray/tray_windows.py | Changed icon path from PNG to ICO format for Windows tray implementation |
| wow_sync/tray/tray_macos.py | Added ICNS file support with PNG fallback for macOS menu bar icons |
| icons/generate_icons.py | Enhanced icon generation script to create multi-resolution ICO and ICNS files using Pillow and icnsutil |
| build.py | Updated PyInstaller build script to use platform-specific icon formats (ICNS for macOS, ICO for Windows) |
| requirements.txt | Added icnsutil library as optional build dependency for ICNS generation |
| icons/icon.ico | Generated binary ICO file containing multiple resolutions (16x16, 32x32, 48x48, 64x64, 128x128, 256x256) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| for filename, size in sizes.items(): | ||
| # Process all icon sizes and save as PNG files | ||
| icon_sizes_for_ico = {} |
Copilot
AI
Dec 28, 2025
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.
The data structure change from dictionary to list is good for maintaining order, but the variable name icon_sizes_for_ico is misleading. This dictionary is used to collect resized images for both ICO and ICNS file generation, not just ICO. Consider renaming it to icon_sizes_cache or resized_icons for better clarity.
| resized.save(output_path, "PNG", optimize=True) | ||
| print(f" ✓ Saved {output_path}") | ||
|
|
||
| # Collect images for ICO file (avoid duplicates) |
Copilot
AI
Dec 28, 2025
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.
The comment states "avoid duplicates" but the logic actually ensures only one image per size is stored, using the last one processed. If the sizes list has duplicates (like two entries with size 256), the second will overwrite the first. Consider either removing duplicates from the sizes list earlier, or clarifying the comment to explain this overwrite behavior.
| # Collect images for ICO file (avoid duplicates) | |
| # Track unique icon sizes for the ICO file (skip duplicate sizes in the sizes list) |
|
|
||
| temp_png = icons_dir / f"icon_{normal_size}x{normal_size}.png" | ||
| img.save(temp_png, "PNG") | ||
| icns.add_media(file=str(temp_png)) | ||
| temp_png.unlink() | ||
|
|
||
| # Add retina resolution | ||
| if retina_size in icon_sizes_for_ico: | ||
| retina_img = icon_sizes_for_ico[retina_size] | ||
| elif retina_size <= 1024: | ||
| retina_img = transparent_img.resize((retina_size, retina_size), Image.Resampling.LANCZOS) | ||
| else: | ||
| # For 1024, just use the processed image | ||
| retina_img = transparent_img.resize((1024, 1024), Image.Resampling.LANCZOS) | ||
|
|
||
| temp_retina_png = icons_dir / f"icon_{normal_size}x{normal_size}@2x.png" | ||
| retina_img.save(temp_retina_png, "PNG") | ||
| icns.add_media(file=str(temp_retina_png)) |
Copilot
AI
Dec 28, 2025
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.
The temporary PNG files created for ICNS generation are immediately deleted after being added to the ICNS file. However, if an exception occurs between saving the temp file and unlinking it (e.g., in the icns.add_media() call), the temp file will not be cleaned up. Consider using a try-finally block or context manager to ensure cleanup, or use a temporary directory that's automatically cleaned up.
| async def setup(self): | ||
| """Setup the tray icon using Windows Shell NotifyIcon.""" | ||
| icon_path = ICON_DIR / "icon.png" | ||
| # Use ICO file for Windows (supports multiple resolutions) |
Copilot
AI
Dec 28, 2025
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.
The comment "Use ICO file for Windows (supports multiple resolutions)" is helpful, but it would be even more informative to mention that PNG files cannot be loaded as ICO files using the Win32 API's LoadImage function, which was the root cause of the bug being fixed.
| # Use ICO file for Windows (supports multiple resolutions) | |
| # Use ICO file for Windows: LoadImage cannot load PNG files as ICOs, | |
| # and this limitation previously caused a bug when using a PNG icon. |
| '--onedir' if sys.platform == 'darwin' else '--onefile', | ||
| '--noconsole', | ||
| '--icon=icons/icon.png', | ||
| '--icon=icons/icon.icns' if sys.platform == 'darwin' else ('--icon=icons/icon.ico' if sys.platform == 'win32' else '--icon=icons/icon.png'), |
Copilot
AI
Dec 28, 2025
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.
The build script has a complex ternary expression that's difficult to read and maintain. Consider breaking this into multiple lines or using if-elif-else logic for better readability. For example:
if sys.platform == 'darwin':
icon_arg = '--icon=icons/icon.icns'
elif sys.platform == 'win32':
icon_arg = '--icon=icons/icon.ico'
else:
icon_arg = '--icon=icons/icon.png'Then use icon_arg in the list.
Windows system tray was displaying the generic application icon instead of the WoW Sync icon. The Win32
LoadImage()API withIMAGE_ICONtype requires ICO format; attempting to loadicon.pngas an ICO fails silently and falls back toIDI_APPLICATION. Similarly, macOS benefits from ICNS format which supports multiple resolutions including retina (@2x) displays for crisp rendering at all sizes.Changes
Windows Icon Support (ICO):
icons/generate_icons.py: Generate multi-resolutionicon.ico(16x16 through 256x256) using Pillow'ssizesparameter for proper DPI scalingwow_sync/tray/tray_windows.py: Loadicon.icoinstead oficon.pngicons/icon.ico: Generated 68KB ICO file with 6 embedded resolutionsmacOS Icon Support (ICNS):
icons/generate_icons.py: Generate multi-resolutionicon.icnswith retina support (16x16@2x through 512x512@2x) using icnsutil librarywow_sync/tray/tray_macos.py: Loadicon.icnswith PNG fallback for backward compatibilityicons/icon.icns: Generated 1MB ICNS file with standard macOS resolutions and retina supportrequirements.txt: Addedicnsutilas build dependencyCross-Platform Build Support:
build.py: Use platform-specific icon formats (ICNS for macOS, ICO for Windows, PNG for Linux)Before:
After:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.