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

16 icons (+2 resubmissions) #824

Merged
merged 3 commits into from
Mar 20, 2024

Conversation

drygoatair
Copy link
Contributor

So I already committed the Notes and Tasks icons in a previous PR but as they were still in the icons folder and the new_icons.yaml file was empty, I added them to this commit with better filenames.

@flameshikari
Copy link
Member

flameshikari commented Mar 18, 2024

Thanks for a lot for your input, but please, be more attentive when making icons.

  • you're still trying to do transparencies where it can be easily avoided. for example, passandroid. this is how it can looks like (just quick examples, can be better aligned ofc):
    passandroid icon_icon

    also barcode/qr-code are more simplified and bigger. don't make icons one-to-one, make simpler but keep the idea (keep in mind that some small details may not be visible on phone screen, so you can get rid of them)

  • exist, hellrider3, passandroid: incorrect corners size

  • hellrider3, opp_app: the background is transparent

  • hellrider3: I understand that this icon repeats the original one, but I would suggest to draw half of the logo more straight forward and reflect it

  • not entirely critical, but try to solve the problem of your vectors icons, their size aren't 192x192px, they are bigger somehow. probably it's something with the artboard (this is how it's called in Illustrator, no clue about other editors)

  • try to use the clipping mask. a very useful thing, you don't have to cut shapes to fit them within another shape

  • also instead of making new icons for 'stock apps' (I mean, /e/OS apps), you could link them with existing icons like aosp_mail and sms_alt_3

I admit that some of what I said would be worth to note in the guidelines so that contributors would not have to fix issues later, so sorry for that. Don't hurry, make quality not quantity. I also have notes about your icons in previous PR, but I was busy and couldn't reply. I'll simplify them a little. You're doing great by the way!

@flameshikari flameshikari added this to the 1.9.0 milestone Mar 18, 2024
@flameshikari flameshikari self-assigned this Mar 18, 2024
@drygoatair
Copy link
Contributor Author

Alright, I can do that!

About the corner size though: the corners of passandroid icon are exactly the same as the template (I started using it as an underlay). Did you mean another icon or am I missing something?

@flameshikari
Copy link
Member

the corners of passandroid icon are exactly the same as the template

Not exactly. They should be 10px.

image

@drygoatair
Copy link
Contributor Author

drygoatair commented Mar 19, 2024

Ah, should've zoomed in a bit more 🤦

I updated the icons:

  • Fixed corners, setting the radius to 10px
  • Got rid of all transparencies,
  • Updated my export settings so SVG files are set to 192px x 192px
  • Simplified passandroid
  • Chopped hellrider3 in half and mirrored the rest

I tried giving the hellrider3 icon 10px radius corners but this just looked weird. So I went the other way and made it fit within the confines of the circular background. Let me know what you think.

I left the mail and sms icons as is, as they don't have a linked icon yet. This way they're set automatically after installing Delta (right now they don't change).

@drygoatair drygoatair force-pushed the banana_branch branch 4 times, most recently from 0d038fc to 61e64bb Compare March 19, 2024 20:04
@flameshikari
Copy link
Member

I've fixed other unnoticed issues and reworked some icons in 3762525

@flameshikari flameshikari merged commit 1bd583e into Delta-Icons:master Mar 20, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants