Skip to content
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

Adding Favicon to the webpage. #296

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

nakul-py
Copy link
Contributor

@nakul-py nakul-py commented Feb 7, 2025

Description

Adding Favicon to the Cosmos Journeyer Webpage.
Fixes #293

@BarthPaleologue BarthPaleologue self-requested a review February 7, 2025 14:16
Copy link
Owner

@BarthPaleologue BarthPaleologue left a comment

Choose a reason for hiding this comment

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

Did you even test your PR before opening it? Because if you had you would have noticed the favicon is not being displayed 🤔

It's just a simple path issue, so we should be able to merge once that's fixed (see details in comments).

favicon.png Outdated Show resolved Hide resolved
src/html/index.html Outdated Show resolved Hide resolved
@nakul-py
Copy link
Contributor Author

nakul-py commented Feb 7, 2025

As i have said before cannot build with webpack. I test it locally on vite haha.

@BarthPaleologue
Copy link
Owner

Ah I see, it explains ^^ Just so you know you can just run pnpm install in this branch to get back webpack so you can test your changes 👍

Then when you check out your vite branch, you just run pnpm install again to get back vite. You are not stuck with vite thanks to package.json being in git ;)

@nakul-py
Copy link
Contributor Author

nakul-py commented Feb 7, 2025

I tried but webpack gives errors

@BarthPaleologue
Copy link
Owner

What kind of Webpack errors @nakul-py ? Because reinstalling the packages when changing branch does work. If you are using npm instead of pnpm, you may want to perform npm ci instead of npm i because npm is quite the shitty package manager. That's one of the reasons I use pnpm

Copy link
Owner

@BarthPaleologue BarthPaleologue left a comment

Choose a reason for hiding this comment

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

Build CI does not work. Probably the path to the favicon is incorrect relative to the html file.

@nakul-py
Copy link
Contributor Author

nakul-py commented Feb 8, 2025

Build CI does not work. Probably the pass to the favicon is incorrect relative to the html file.

Ok i will do this later when project runs with vite.

@BarthPaleologue
Copy link
Owner

No problem, we are not in a hurry 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Favicon
2 participants