Add support for additional app icon formats#216
Merged
SRWieZ merged 1 commit intoNativePHP:mainfrom May 3, 2025
Merged
Conversation
Extended the InstallsAppIcon trait to copy .ico and .icns icon formats alongside existing ones. This ensures compatibility with platforms requiring these formats.
gwleuverink
requested changes
May 3, 2025
Contributor
gwleuverink
left a comment
gwleuverink
left a comment
There was a problem hiding this comment.
Looks good 👍
Can you verify we don’t need to add these here too: https://github.com/NativePHP/electron/blob/main/src/Traits/InstallsAppIcon.php
If it isn’t coupled it might be a smell we can remove one of the two.
If it is, it wouldn’t be a bad idea to leave a comment that these two places must be kept in sync manually (or perhaps decouple it by using the internal config?)
Contributor
Author
|
@gwleuverink I don't know if I'm misunderstanding something here, but I made the change to the file you referred to |
Contributor
|
No you're right I was going over PR's and must have gotten some wires crossed. Approved! |
gwleuverink
approved these changes
May 3, 2025
SRWieZ
approved these changes
May 3, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extended the InstallsAppIcon trait to copy .ico and .icns icon formats alongside existing ones. This ensures compatibility with platforms requiring these formats.
For macOS: icon.icns
For Windows: icon.ico
Fallback: icon.png
Doc PR: NativePHP/nativephp.com#119