Add favicon to Volcano website#508
Conversation
volcano-sh-bot
added
kind/feature
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @kubeboiii! It looks like this is your first PR to volcano-sh/website 🎉 |
volcano-sh-bot
added
the
size/M
![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 configures favicons and metadata for the website by adding a favicon path and various head tags (apple-touch-icon, manifest, mask-icon) to docusaurus.config.js, and updating the name in site.webmanifest. The reviewer noted that while the PR description mentions copying favicon.ico to the site root, the physical static/favicon.ico file is missing from the changes and needs to be added.
| url: "https://volcano.sh", | ||
| baseUrl: "/", | ||
|
|
||
| favicon: "img/favicons/favicon.ico", |
There was a problem hiding this comment.
The PR description states that this change "copies favicon.ico to the site root for clients that request /favicon.ico". However, there is no static/favicon.ico file included in this pull request, nor are there any build scripts or plugins configured to perform this copy.
To make the favicon available at the root /favicon.ico (which many browsers and crawlers query by default), please physically copy the favicon.ico file to static/favicon.ico and commit it as part of this PR.
There was a problem hiding this comment.
static/favicon.ico is already in this PR (see Files changed, commit 7f89bb7). It is a direct copy of static/img/favicons/favicon.ico and is served at /favicon.ico via Docusaurus static assets. No build step needed.
The site already had favicon files under static/img/favicons but Docusaurus was not configured to emit them in the document head. Wire favicon, apple-touch, manifest, and Safari mask icon links. Signed-off-by: Himanshu <himanshuwankhadeh@gmail.com>
kubeboiii
force-pushed
the
fix/507-add-favicon
branch
from
4857921 to
7f89bb7
Compare
volcano-sh-bot
removed
the
do-not-merge/invalid-commit-message
|
/lgtm from my side .. @JesseStutler |
3 participants
/kind feature
The Volcano website did not show a favicon in the browser tab. Favicon
assets already existed under
static/img/favicons/but were notreferenced in
docusaurus.config.js. This change wires the favicon inDocusaurus config, adds head tags for apple-touch-icon and web manifest,
copies
favicon.icoto the site root for clients that request/favicon.ico, and sets the manifest display name to Volcano.Fixes #507
Test plan
npm run buildcompletes successfully<link rel="icon" href="/img/favicons/favicon.ico">build/favicon.icois present after build