fix: add favicon.ico support#477
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a favicon to the site by updating the Eleventy configuration to copy the favicon file and adding the corresponding link tag to the index template. Feedback was provided to use the url filter for the favicon path to ensure correct resolution when the site is hosted in a sub-directory.
|
/gemini review |
There was a problem hiding this comment.
Pull request overview
Adds favicon support to the Eleventy site so /favicon.ico is available and referenced from the HTML head, addressing the reported 404.
Changes:
- Add a
<link rel="icon">reference insrc/index.njk. - Configure Eleventy passthrough copy for
src/favicon.icoin.eleventy.js.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/index.njk |
Adds a favicon <link> tag in the page <head>. |
.eleventy.js |
Copies src/favicon.ico to the output via passthrough copy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <meta charset="UTF-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <title>Developer Directory</title> | ||
| <link rel="icon" type="image/x-icon" href="{{ '/favicon.ico' | url }}"> |
| @@ -4,6 +4,7 @@ module.exports = (eleventyConfig) => { | |||
| eleventyConfig.addShortcode("currentYear", () => new Date().getFullYear()); | |||
| // Add this line to copy your external assets | |||
| eleventyConfig.addShortcode("currentYear", () => new Date().getFullYear()); | ||
| // Add this line to copy your external assets | ||
| eleventyConfig.addPassthroughCopy("src/assets"); | ||
| eleventyConfig.addPassthroughCopy("src/favicon.ico"); |
There was a problem hiding this comment.
Code Review
This pull request adds a favicon to the project by configuring Eleventy to copy the asset and adding the corresponding link tag to the main template. The review feedback points out an inconsistency where the new favicon link correctly uses the url filter for subpath deployment portability, but existing stylesheet links do not, which could lead to broken styles in those environments.
| <meta charset="UTF-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <title>Developer Directory</title> | ||
| <link rel="icon" type="image/x-icon" href="{{ '/favicon.ico' | url }}"> |
There was a problem hiding this comment.
The addition of the url filter for the favicon is a good practice for supporting subpath deployments (e.g., GitHub Pages). However, I noticed that the existing stylesheet links on the following lines (11 and 12) do not use this filter. This creates an inconsistency where the favicon might load correctly on a subpath while the CSS fails. For full portability, you should consider applying the url filter to all internal assets in the <head>.
Fixes #134
Added favicon.ico support by:
favicon.icofileThis resolves the 404 issue for
/favicon.ico.