Add a handler for favicon.ico #423
Conversation
Closes dart-lang/build#892 If the users has not specified a favicon chrome will attempt to load `/favicon.ico` by default. If this would otherwise be a 404 we can provide a default and avoid an error in the console. The default icon is the same as the `dart.dev` site.
|
Hmm, I am not sure we should really do this. It is on the application developer to provide a favicon, and hiding the error at development time just means it will show up when they deploy which would be unexpected. I sort of view this as a similar issue to how flutter enables internet by default but only at debug mode causing all kinds of confusion. |
|
LGTM. I don't have strong feelings on this one. We have similar logic in DDR and package:test. This would be consistent with those tools at least. |
That argument makes sense. From another perspective the favicon is something that it's nice to not have to think about, and not make noise in the console, very early in a project lifecycle until you're getting closer to going to production. |
|
DBC – wonder if this would be a useful thing to just have in pkg:shelf – pondering... |
| return (request) async { | ||
| var response = await handler(request); | ||
| if (response.statusCode == 404 && | ||
| request.url.path.endsWith('favicon.ico')) { |
There was a problem hiding this comment.
I'd change this to pathSegments.length >= 1 && pathSegments.last == 'favicon.ico' – maybe?
We can revert at the first sign that someone accidentally ships to production because they thought that this favicon would work for free 😄 |
|
This does sound like a net positive because I like to have the binary signal of "error in the console or not" when I start building a web project. Seems reasonable to me that since we are using the dart logo as the icon it could almost serve as a reminder that you need to eventually replace it with your own. |
|
Which one is using the correct icon colors though? Right now it looks like dart.dev and pub.dev use slightly different icons but the colors on the pub.dev icon match the colors on the page better.
|
|
I'm not sure which is the current colorscheme, I pulled the one from |
|
I just filed: dart-lang/site-www#1673 for consistency. |
Closes dart-lang/build#892
If the users has not specified a favicon chrome will attempt to load
/favicon.icoby default. If this would otherwise be a 404 we canprovide a default and avoid an error in the console.
The default icon is the same as the
dart.devsite.