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

Renew app icon #1647

Merged
merged 1 commit into from May 2, 2022
Merged

Renew app icon #1647

merged 1 commit into from May 2, 2022

Conversation

validblock
Copy link
Contributor

Updating app icon according to the Human Interface Guidelines.

@thisconnect
Copy link
Collaborator

Thanks @validblock , there is also trayicon.png (the one shown in the macos menu) and icon.icns (afaik used for the actual app icon) that should be updated at the same time.

Would you mind share the source file?

@validblock
Copy link
Contributor Author

trayicon.png (the one shown in the macos menu)

Do we really need the menu? The menu does one little thing: Quit the app. Just saying ;)

icon.icns

I saw, the icns file will be generated by using convert_icon.sh from the updated app_icon_source.png file.

Attached all resources used by me.
Archive.zip

@benma
Copy link
Contributor

benma commented Feb 23, 2022

Do we really need the menu? The menu does one little thing: Quit the app. Just saying ;)

The menu is actually needed to enable desktop notifications, as strange as that sounds. It's a Qt restriction.

@thisconnect
Copy link
Collaborator

@validblock thanks for the resources.

ACK from my side to the new macOS desktop icon, but I guess the Android icon should stay as is.

@validblock
Copy link
Contributor Author

ACK from my side to the new macOS desktop icon, but I guess the Android icon should stay as is.

I would create a single icon for each OS. No convert scripts from a master icon anymore.

@validblock
Copy link
Contributor Author

I try to generate an Android app icon by using the Android Asset Studio. The result is the same icon set as already exists. No difference. Stay this way.

ic_launcher

@validblock
Copy link
Contributor Author

Regarding to the article Create a macOS Menu Bar Application I would suggest the following tray icon:

trayicon
White Shift Crypto logo on a transparent background with spacing/padding around

Ok? Then I will update the pull request by replacing the trayicon.png.

@thisconnect
Copy link
Collaborator

thisconnect commented Feb 24, 2022

Ok? Then I will update the pull request by replacing the trayicon.png.

I think it makes sense to update the trayicon.

I saw, the icns file will be generated by using convert_icon.sh from the updated app_icon_source.png file.

Could you also run convert_icon.sh and update https://github.com/digitalbitbox/bitbox-wallet-app/blob/master/frontends/qt/resources/MacOS/Testnet.app/Contents/Resources/icon.icns ?

also I wanted ping @jadzeidan but forgot, PTAL

@jadzeidan
Copy link
Contributor

Ack. Change makes sense. LGTM :)

@thisconnect
Copy link
Collaborator

@validblock do you have time to commit the generated icons too so we could just merge this?

@thisconnect
Copy link
Collaborator

LGTM

Screen Shot 2022-03-03 at 8 52 21 AM

Screen Shot 2022-03-03 at 8 52 57 AM

Screen Shot 2022-03-03 at 8 53 19 AM

Screen Shot 2022-03-03 at 8 54 18 AM

@thisconnect
Copy link
Collaborator

@benma could you test Android?

@benma
Copy link
Contributor

benma commented Mar 3, 2022

@thisconnect is the icon supposed to change on all platforms? If so, the icons on Android, Windows and Linux have to be compiled as well.

  • frontends/qt/resources/win/convert.sh
  • frontends/android/mkicon.sh
  • frontends/qt/resources - make

I ran these and this is the output for me: benma@0bac53f

After this, on Android it looks like this (DEBUG is new, BitBoxApp is old)

image

On linux:

Before:

image

After:

image

Is this good?

@thisconnect
Copy link
Collaborator

macOS and linux LGTM.

I don't like that the logo is so big on Android.

CC @jadzeidan WDYT?

@jadzeidan
Copy link
Contributor

macOS and linux LGTM.

I don't like that the logo is so big on Android.

CC @jadzeidan WDYT?

LGTM with nit on Android that the logo is too large compared to the background. Ideally would look more like this:

image

@jadzeidan
Copy link
Contributor

jadzeidan commented Mar 3, 2022

I'm not sure how the Android icon is generated. Is the background autogenerated and you just need to provide the correct size logo in the middle?

@benma
Copy link
Contributor

benma commented Mar 3, 2022

@jadzeidan see my comment above

https://github.com/digitalbitbox/bitbox-wallet-app/blob/master/frontends/android/mkicon.sh

so it looks like the original but just resized to various sizes.

@jadzeidan
Copy link
Contributor

@jadzeidan see my comment above

https://github.com/digitalbitbox/bitbox-wallet-app/blob/master/frontends/android/mkicon.sh

So what's the best fix to make the logo on Android smaller. Provide a smaller logo specifically for Android?

@benma
Copy link
Contributor

benma commented Mar 3, 2022

In the info screen it looks good however:

image

Seems my launcher somehow cuts off the icon a bit.

@benma
Copy link
Contributor

benma commented Mar 3, 2022

@thisconnect
Copy link
Collaborator

So what's the best fix to make the logo on Android smaller. Provide a smaller logo specifically for Android?

I'd assume it looks better if there is a bit more whitespace for the android?

Alternative: keep as is or change the old one to have a white BG ?

@jadzeidan
Copy link
Contributor

jadzeidan commented Mar 3, 2022

I'd assume it looks better if there is a bit more whitespace for the android?

Yes I think so too.

Alternative: keep as is or change the old one to have a white BG ?

I don't like the old one because it has a background in a background (below).

image

So I think we should change it so it looks like this:

image

So the question what is the best way to do that?

@thisconnect
Copy link
Collaborator

@jadzeidan could you make a different icon for android that has 20% white space on each side? so the actual logo takes up about 60% of the image?

@jadzeidan
Copy link
Contributor

@thisconnect

Does this work?

shiftcrypto-logo-2

@jadzeidan
Copy link
Contributor

@thisconnect here is PNG version:

shiftcrypto-logo-2

@validblock
Copy link
Contributor Author

Any blocker there?

@benma
Copy link
Contributor

benma commented Apr 11, 2022

@validblock sorry the blocker here was me, I was absent for two weeks and need to test the above Android logo. Will do so in the coming days. Sorry that this PR is taking so long 🤯

@benma
Copy link
Contributor

benma commented Apr 28, 2022

Sorry for the very long delay!

@jadzeidan the icon you provided looks good to me:

image

(BitBoxApp DEBUG is new, BitBoxApp is old).

Too bad it is not documented anywhere how the round icons were generated :/

@validblock i took the liberty of adding a commit to this PR with the change for Android.

@thisconnect anything else missing or is this good to squash and merge?

@thisconnect
Copy link
Collaborator

LGTM @benma squash and merge!

@benma benma merged commit 6b03f0d into digitalbitbox:master May 2, 2022
@benma
Copy link
Contributor

benma commented May 2, 2022

Thanks very much @validblock and sorry again that this took so long!

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

Successfully merging this pull request may close these issues.

None yet

4 participants