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

feat(generators): Added generation ic_launcher_round.png in assets icon #216

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vdelacou
Copy link

Hi, thanks for submitting a Pull Request! 🎈

Before submitting, please be sure to check a few things:

Commit names formatting

This repository is automatically published to npm with
semantic-release.

In order to manage that, your commit should follow the AngularJS commit
conventions
as explained here

You can use yarn commit to write a commit respecting this convention.

This was referenced Mar 18, 2019
@vdelacou vdelacou changed the title add generateResizedRoundAssets method feat(generators): Added generation ic_launcher_round.png in assets icon Mar 18, 2019
@schumannd
Copy link

Just tested it, and it worked 👍

@ghost
Copy link

ghost commented Mar 28, 2019

Thanks for the PR. I've tried it with one of my projects but it seems like some of the rounded icons have a white strip at some of their edges, despite the background being navy in the original square icon. Sometimes they have this at top, bottom, left, and right all together, and sometimes a combination of these.

@vdelacou
Copy link
Author

Thank you for your answer. I guess it's all about the margin of 90% I added to get the icon center in a smaller square. I use a mask and rewrite the icon on it with the compose "copyopacity". I didn't experience the bug you describe, I will try with different icon size, transparency and format. For the moment I still don't know how to prevent that except remove the margin.

@dldinza-web
Copy link

Hello dear.

When do you plan to release this feature?

Best regards

console.log(`Wrote ${destinationPath + '.mask.png'}`);
});

gm(path.normalize(psdSafeSourcePath))
Copy link

Choose a reason for hiding this comment

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

Composing of images depends on creating *.mask.png. So i think this gm chain for compose original png with *.mask.png should be places inside of .writeAsync then(() => { when writeAsync is asynchronous.

@lpikora
Copy link

lpikora commented Apr 7, 2019

Fantastic job. I've tested it in my project and it works well. 👍

I've also added some comments to pull review commit's code - there is one potential problem with async writing of mask file and using it for composing opacity.

I think this PR should be merged and ic_launcher_round should be supported because starting from react native 0.58.0 android:roundIcon="@mipmap/ic_launcher_round" is used in React Native init project template (react-native-community/rn-diff-purge@version/0.57.0...version/0.58.0).

@rodrigos0ares
Copy link

Works great on react native 0.59.4
Thanks!

@airled
Copy link

airled commented May 7, 2019

Maybe one more approving review?

@skhameneh
Copy link

Works great, although it looks like Android 10 is pushing squircle icons.

@vdelacou
Copy link
Author

vdelacou commented Jun 4, 2019

Works great, although it looks like Android 10 is pushing squircle icons.

Oh I was not aware of that. I think you are right. Need to be done as squircle icons.

@skhameneh
Copy link

skhameneh commented Jun 4, 2019

I'm looking at this for the splash as well -> https://developer.android.com/preview/features/darktheme

The default generated splash background color on a transparent PNG is #00000000 which renders as black on both light and dark mode.

Update: Manually setting splashBackground to #FFFFFF00 renders yellow. I need to investigate how colors work in colors.xml.

@blackinitial
Copy link

now must be merging :(

@muhsin-k
Copy link

muhsin-k commented Sep 5, 2020

Could you please merge this PR

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.

9 participants