Skip to content

Add favicon #70

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

Merged
merged 1 commit into from
Jul 22, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/webpack.config.dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ module.exports = {
new HtmlWebpackPlugin({
inject: true,
template: indexHtmlPath,
favicon: path.join(srcPath, 'favicon.ico'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it work if it's in the root folder with HTML instead? This feels more like meta information than app module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think it would be better to have a folder that contains static assets (favicon.ico and logo.svg)? I think favicon.ico should be in that folder (let's call it assets?) but not sure about logo.svg.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And yeah, it works when it's not in the src folder.

Copy link
Contributor

@lacker lacker Jul 22, 2016

Choose a reason for hiding this comment

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

For logo.svg and similar images, since it gets imported from javascript, it's annoying to put it in a different root folder. You would end up with import '../../../../../../../../assets/foo/bar/baz/qux.png' in big projects rather than import './blah.png'. The favicon is different since webpack handles it without js doing anything.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup. I moved favicon.ico to top level since it’s a “magic” file like index.html.
The modules live in src.

}),
new webpack.DefinePlugin({ 'process.env.NODE_ENV': '"development"' }),
// Note: only CSS is currently hot reloaded
Expand Down
1 change: 1 addition & 0 deletions config/webpack.config.prod.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ module.exports = {
new HtmlWebpackPlugin({
inject: true,
template: indexHtmlPath,
favicon: path.join(srcPath, 'favicon.ico'),
minify: {
removeComments: true,
collapseWhitespace: true,
Expand Down
Binary file added template/src/favicon.ico
Binary file not shown.