-
Notifications
You must be signed in to change notification settings - Fork 259
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
Update launcher icon with flat long-shadow variant #638
Conversation
Finally looked into the link and my understanding of it is that the recommendation is regarding system icons. The padding is to protect the icon from getting cut off by elements such as sidebars, which is obviously not a problem for launcher icons. Am I missing something? |
Yeah, it is about system icons, I didn't pay enough attention. I still have the impression that some padding is implied for launcher icons, but I can't find any numbers. |
Have you tested the icon? It would be nice to see what it looks like in action and see if it is fine the way it is now. I can't build apps on my current setup so it would be really nice if someone could and share some screenshots. Seeing how the icon was made by someone who has made icons for plenty of others apps too, I think any requirements such as padding would already have been taken care of. |
Sorry about bumping but can someone please test the icons so that this can be merged? |
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.
With this PR, the update of the icon, we need a several more changes.
- Update of the Readme: icon needs to be changed to the new one and in the About section there shall be a paragraph crediting @themichaelcook
- we need an icon, which looks the same, but indicates the debug version installed on the phone. Currently we have this in https://github.com/wallabag/android-app/tree/master/app/src/debug/res/drawable-xxxhdpi Without that icon i cannot test it du to having the release version installed for production purpose and not for testing
- it would be nice if this PR has the credits for the about screen
This PR conflicts with #645 |
I've already updated the icon in #596. I'll add credits for the icon as well. I checked the debug icon and if you could provide me with a SVG of the same then I could move the bug to the new icon and update that as well. Making changes to the About screen is beyond my skill set presently. 🙈 As for the conflict with #645, that PR was opened to go with a version of this icon with added padding but I think I and @di72nn have arrived at an understanding regarding that. @di72nn, do you think it would be okay to close your PR? |
Sorry, i forgot about #596. Thanks for the reminder. Thanks for adding credits to that PR. The bug being used is the following unicode char: https://www.fileformat.info/info/unicode/char/1f41b/index.htm Currently, in the app we have navigation pane, which we can open from the left. There is also the official wallabag logo being used. But this logo we do not want to replace, don't we? If we have all those changes in here and #596, from my point of view, we can go ahead merging this. |
Added a debug icon and added credits for the icon in #596. |
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.
Sorry about bumping but can we please move ahead with this? |
I have figured out how to add support for adaptive icons and would like to open a new PR with adaptive icon support. I hope it is okay to close this one. |
Yes, no problem @lokesh-krishna . Thanks a lot for your ongoing commitment and contribution to the project |
As discussed at #317, here's the PR to update the app launcher icon. Round icon support hasn't been added and neither has support for adaptive icons. I don't know how to do either and I think we'll need Michael Cook's help for the latter with regards to a foreground and background layer unless someone here can do it.
All the
.png
s were generated using the provided.svg
.Also, as mentioned at #595, the icons need go into the
mipmap
directory only if we are building different apks for different display density (which I think we aren't).Also, the discussion at #595 mentioned the welcome icons but I don't think they need changing because they represent the service while these icons represent one specific product based off that service. Also, they look better in that context. That's just my understanding of it and I will make a PR for them as well if someone feels that is required.
Someone will need to credit Michael Cook for the icon in the About screen so that the requisite credits are present when this icon goes live. This is how he said he would like to be credited:
Would be nice if this was also mentioned in the changelog for the version in which this goes live for that's how I discovered him in the first place and I'd like to pay that forward.