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

Allow web favicon to be built from a different source image file #519

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

galenwarren
Copy link

This regards this issue.

It would be useful to be able to use a different source image to generate the favicon vs. the image icons for Flutter web. Two reasons:

  • Favicons with transparent backgrounds work well, in general, but transparent backgrounds don't work well for icons for PWA apps (a background gets assigned which might be white, might be black, etc.)

  • PWA icons have the same issue that Android icons do, i.e. adaptive icons need to limit content to the "safe zone" so that the icons can be cropped as needed. Favicons don't need this extra padding and really can't afford to have it, given how small they are to begin with.

This PR adds an image_path_favicon option to the config to allow one to override the source icon used to generate the web favicon. This would allow one to specify a non-padded icon with a transparent background to use for favicon generation and a padded icon with a solid-color background for other image generation.

@MarkOSullivan94
Copy link
Collaborator

Good idea @galenwarren

Copy link
Collaborator

@MarkOSullivan94 MarkOSullivan94 left a comment

Choose a reason for hiding this comment

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

LGTM thanks for your contribution @galenwarren

@MarkOSullivan94
Copy link
Collaborator

@galenwarren there's a conflict, can you address them and I'll get this merged?

@galenwarren
Copy link
Author

@MarkOSullivan94 Thanks for taking a look at this!

I hope this isn't a dumb question, but what conflict are you referring to? I'm happy to take a look but it's not jumping out at me. I do see that people have added commits to coachclientconnect:support-favicon-image-file so I pulled down the latest from that branch but it seems ok, tests run.

@galenwarren
Copy link
Author

@MarkOSullivan94 Just wanted to circle back on this. I'm happy to help but not quite sure what you're asking me to do. Would love to get this one merged, though, so I can retire my fork. Thanks!

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.

4 participants