Skip to content
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

Updater: Add new Dolphin Updater icon #10506

Closed
wants to merge 5 commits into from

Conversation

Jeffrey-Lim
Copy link

Adds a new icon to the WinUpdater and MacUpdater. Fixes issue #11074.
Dolphin Updater

Copy link
Member

@BhaaLseN BhaaLseN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ping @MayImilae to review the icon change

Source/Core/WinUpdater/WinUpdater.rc Outdated Show resolved Hide resolved
Copy link
Member

@BhaaLseN BhaaLseN left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, no preference on the icon tho (so my approval probably doesn't mean much)

@MayImilae
Copy link
Contributor

Wasn't the plan to merge the updater into the main application?

@Jeffrey-Lim
Copy link
Author

Wasn't the plan to merge the updater into the main application?

I didn't know about that. Is anyone working on that currently? If not, we can merge this PR so the updater looks a little nicer in the meantime.

@OatmealDome
Copy link
Member

OatmealDome commented Mar 11, 2022

@MayImilae Just in case you're talking about my PR (#10428): it doesn't really "merge" them, it instead just puts the updater app literally inside the Dolphin app, and only on macOS. They're still technically two separate entities.

@mbc07
Copy link
Contributor

mbc07 commented Mar 11, 2022

This might sound a bit rude but I would rather (re)use the main Dolphin icon than the proposed design, which looks rather odd IMHO (two different icons, with different design styles, slapped together). It would also be consistent with DolphinTool and the macOS updater too, which currently just reuses the main app icon as well...

@delroth
Copy link
Member

delroth commented Mar 15, 2022

I personally think a different icon makes sense given the fact that our updater is not a user-facing application and should not be confused for one. But it might be even better to move it somewhere less visible or something along those lines. And I also don't think the proposed design really conveys that information ("don't touch this, it's only the updater backend").

@MayImilae
Copy link
Contributor

I think that moving it to a less visible location is a much better solution than changing the icon. I mean, we still can still change the icon if we want to, but it shouldn't be right next to the EXE.

As for this icon change in particular, it's... an updater icon I guess. I never got around to making an updater icon because I couldn't figure out a way to make a great updater icon that I'd want to ship. This PR doesn't achieve that either, but it is very basic and inoffensive at least, so if there is demand for changing the updater icon, it would do the job. Though honestly I think that changing the icon is not the best solution to any user confusion about the updater, moving it away from it's prominent location is.

@Wowfunhappy
Copy link

Worth noting that many of Apple's non-user-visible apps still have icons. Examples include Dock.app and Ticket Viewer.app in /System/Library/CoreServices (at least on my old OS version, 10.9).

As such, IMO it makes sense to move the updater to a different location and adopt this icon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
8 participants