Skip to content

fix: restore favicon and add frame-based animation - #240

Merged
khanirfan18 merged 4 commits into
khanirfan18:mainfrom
NiranjanRSoorej06:titleicon
Jul 21, 2026
Merged

fix: restore favicon and add frame-based animation#240
khanirfan18 merged 4 commits into
khanirfan18:mainfrom
NiranjanRSoorej06:titleicon

Conversation

@NiranjanRSoorej06

@NiranjanRSoorej06 NiranjanRSoorej06 commented Jul 9, 2026

Copy link
Copy Markdown

Description

This PR fixes the browser tab icon by implementing a frame-based animated favicon.

Changes Made

  • Added favicon animation frames under public/favicon/.
  • Updated index.html to correctly reference the favicon.
  • Implemented frame-based favicon animation in main.jsx by cycling through PNG frames.
  • Added cache-busting query parameters to minimize favicon caching issues during development.

Problem

The favicon was not displaying correctly in the browser. This implementation ensures the favicon loads properly and adds a lightweight animation for a more engaging browser tab experience.

Videos

Screen.Recording.2026-07-10.003738.mp4
Screen.Recording.2026-07-10.003657.mp4

Notes

  • The animation is implemented by cycling through PNG frames, as animated GIF favicons are not consistently supported across modern browsers.
  • Browser support for animated favicons varies, and some browsers may throttle updates.

Closes #231

@netlify

netlify Bot commented Jul 9, 2026

Copy link
Copy Markdown

👷 Deploy request for finnboard0 pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 9e13f25

@netlify

netlify Bot commented Jul 9, 2026

Copy link
Copy Markdown

Deploy Preview for finnboard ready!

Name Link
🔨 Latest commit 9e13f25
🔍 Latest deploy log https://app.netlify.com/projects/finnboard/deploys/6a59255b5f15000007ff86fe
😎 Deploy Preview https://deploy-preview-240--finnboard.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@khanirfan18

Copy link
Copy Markdown
Owner

thanks for the fix @NiranjanRSoorej06. moving the favicon into public/ and referencing it from index.html looks like the right direction for the production favicon issue.
i’d prefer not to merge the animated favicon part in this pr though. the issue is about the favicon not displaying in production, while this pr also adds a frame-based animation loop in main.jsx, which feels outside the scope of the bug fix.

a few things i’d like changed before merge:

  • please remove the js-based favicon animation logic from main.jsx and keep this pr focused on restoring the favicon in production
  • keep a single static favicon reference in index.html, for example /favicon/frame_00.png
  • remove the duplicate <title> tag so only one title remains
  • if you want to add animated favicon support, please open that as a separate enhancement pr or issue
    once it’s reduced to the favicon restoration itself, i’ll be happy to review it again.

@NiranjanRSoorej06

Copy link
Copy Markdown
Author

@khanirfan18 , thanks for the review and the detailed feedback! I've addressed the requested changes:

  • Removed the JavaScript-based favicon animation logic from main.jsx
  • Kept a single static favicon reference in index.html
  • Removed the duplicate <title> tag

The PR is now focused solely on restoring the favicon in production. Please let me know if there's anything else that needs to be updated. Thanks!

@khanirfan18

Copy link
Copy Markdown
Owner

thanks for the contribution! @NiranjanRSoorej06 could you remove the public/favicon/frame_* images from this PR? they don't play any role in the current changes, so it'd be better to keep the PR focused on only the necessary files.

@NiranjanRSoorej06

Copy link
Copy Markdown
Author

Hey @khanirfan18 ,thanks for the review! I've addressed the requested changes, including removing the unused public/favicon/frame_* images, and updated the PR. If everything looks good now, I'd appreciate it if you could review and merge it. Thanks again!

@Deniwn22 Deniwn22 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good improvement overall. One edge case: empty string vs null aren't treated the same downstream — might need a normalization step.

@kimberly-WY kimberly-WY left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The happy path is clean. What happens if the third-party API returns 429 here? Might be worth handling that case explicitly.

@khanirfan18
khanirfan18 merged commit 497d538 into khanirfan18:main Jul 21, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Browser Favicon Not Displayed in Production

4 participants