-
Notifications
You must be signed in to change notification settings - Fork 6
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
macOS App Icon fix #4
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -105,7 +105,7 @@ int main(int argc, char *argv[]) | |
app.setOrganizationName("Theengs"); | ||
app.setOrganizationDomain("Theengs"); | ||
|
||
#if !defined(Q_OS_ANDROID) && !defined(Q_OS_IOS) | ||
#if !defined(Q_OS_ANDROID) && !defined(Q_OS_IOS) && !defined(Q_OS_MACOS) | ||
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. This doesn't set the dock or bundle icon anyway, this set the application icon, the one handled by the windows manager. On macOS I don't think it is shown at all nowadays. Might be on older versions, might be again in the future. 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. This produces the macOS dock icon shortly after the app has started, on macOS Catalina anyway ;) the one in the first screenshot in the issue Even after I put the previously missing proper .icns file into the app resources it was still replaced by this as the dock icon. 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. I changed the logo.png file on mine and it isn't shown anywhere. 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. And did you have a Finder icon and dock icon and which icon was it, when there was no .icns file in the Resources folder? I wrote this issue with the artifact download https://github.com/theengs/app/actions/runs/2511311797 which does not have any icon file included, only showing the generic app icon and which made me write up the issue in the first place. Does that download not show the same issues on your macOS? 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. I rebuilt the app to make sure anything wasn't cached and it looks like you are right. The bundled icon is used until the setWindowIcon() is called. 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. I also left the builds run with my included changes (minus the reverted NOT definition checks) here for testing the binaries https://github.com/theengs/app/actions/runs/2524476503 At least icon wise this is working all as expected on 10.15 for the Finder and Dock. |
||
// Application icon | ||
QIcon appIcon(":/assets/logos/logo.png"); | ||
app.setWindowIcon(appIcon); | ||
|
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 Q_OS_LINUX will also catch Android (and every linux derivative), that's why this check is done that way.
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.
Understood, I first had
#if !defined(Q_OS_ANDROID) && !defined(Q_OS_IOS) && !defined(Q_OS_MACOS)
but then thought why three NOTs when 2 defines can do it, not realising the predicament. So back to the initial version.
My first thought as well when I saw the smaller Dock icon, but wanted to get the code and preconditions sorted first to get the actual Finder and Dock icon working. I will have a look at the other icons and make the Theengs logo smaller within wider margins to reproduce the .icns file.
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.
Done and done. Please let me know what you think about the icon with wider margins now.
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.
Looks great!
By the way I don't think the problem was Image.xcassets (it's what xcode is doing by default now) but just the corresponding string in the plist that was still set to theengs.icns...
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.
But that should be the expected plist string
theengs.icns
for CFBundleIconFile, and working fine, now that theengs.icns is actually copied into the Resources folder withICON = $${PWD}/assets/macos/theengs.icns