-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Make optional SVG favicon come after the fallback #27843
Conversation
Fixes gatsbyjs#26083 Chrome choosing the favicon which comes last.
@moonmeister I did end up needing to update the snapshots, and managed to get all tests passing locally. Seems like there are a couple of tests here that are still failing though—
My feeling is that these are unrelated to the changes that I made in this PR, but I'm wondering what your thoughts are. Does this look ok to merge as-is, or is there something I should do to make these tests pass? |
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.
lgtm, but @moonmeister if you could take a quick glance when you have the chance that'd be good
@trevorblades Yes, those are unrelated |
All good |
@moonmeister @ascorbic is there anything blocking this PR from being merged/released? 😇 |
This PR is an extension of #27566, with the requested changes from #27566 (review)