fix: show favicon on login/registration pages and update color to blue#1615
Conversation
Uncomment favicon links in index.html and index.saas.html so favicons display before authentication. Update favicon background from purple to blue (#2463EB). Simplify white-label favicon override in app component. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
… on internal pages if custom logo is set
There was a problem hiding this comment.
Pull request overview
Enables the application favicon to display on unauthenticated routes (login/registration), updates the favicon background color to blue, and refactors favicon handling for white-label configurations.
Changes:
- Updates favicon assets (SVG + adds ICO/PNG variants) and changes background color to
#2463EB. - Adjusts
index.htmlfavicon<link>block (intended to make favicons visible pre-auth). - Refactors
AppComponentto centralize default favicon insertion and simplify white-label favicon override.
Reviewed changes
Copilot reviewed 1 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/index.html | Modifies favicon <link> block in the document <head>. |
| frontend/src/assets/favicon.svg | Changes favicon background color from purple to blue. |
| frontend/src/assets/favicon.ico | Adds default ICO favicon asset. |
| frontend/src/assets/favicon-32x32.png | Adds 32x32 PNG favicon asset. |
| frontend/src/assets/favicon-16x16.png | Adds 16x16 PNG favicon asset. |
| frontend/src/app/app.component.ts | Adds setDefaultFavicon() and changes how default/white-label favicons are applied. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private setDefaultFavicon() { | ||
| const faviconIco = document.createElement('link'); | ||
| faviconIco.rel = 'icon'; | ||
| faviconIco.type = 'image/x-icon'; | ||
| faviconIco.href = 'assets/favicon.ico'; | ||
|
|
||
| const favicon16 = document.createElement('link'); | ||
| favicon16.rel = 'icon'; | ||
| favicon16.type = 'image/png'; | ||
| favicon16.setAttribute('sizes', '16x16'); | ||
| favicon16.href = 'assets/favicon-16x16.png'; | ||
|
|
||
| const favicon32 = document.createElement('link'); | ||
| favicon32.rel = 'icon'; | ||
| favicon32.type = 'image/png'; | ||
| favicon32.setAttribute('sizes', '32x32'); | ||
| favicon32.href = 'assets/favicon-32x32.png'; | ||
|
|
||
| document.head.appendChild(faviconIco); | ||
| document.head.appendChild(favicon16); | ||
| document.head.appendChild(favicon32); |
There was a problem hiding this comment.
setDefaultFavicon() unconditionally appends three new icon elements each time it’s called, which can create duplicates (it’s called both in ngOnInit when no token and again when whiteLabelSettings.favicon is missing). Consider de-duping by removing existing favicon links first or by reusing/storing references (e.g., with ids) and updating hrefs instead of appending repeatedly.
| let expirationToken = localStorage.getItem('token_expiration'); | ||
|
|
||
| if (!expirationToken) { | ||
| this.setUserLoggedIn(false); | ||
| this.setDefaultFavicon(); | ||
| } |
There was a problem hiding this comment.
Calling setDefaultFavicon() only in ngOnInit when there is no token covers initial unauthenticated loads, but it won’t run again when a user logs out (AppComponent isn’t recreated). This can leave the previous white-label favicon visible on /login or /registration after logout. Consider also resetting the favicon in logOut()/logoutAndRedirectToRegistration() or on route transitions to auth pages.
| <!--<link rel="icon" type="image/x-icon" href="assets/favicon.ico"> | ||
| <link rel="icon" type="image/png" sizes="16x16" href="assets/favicon-16x16.png"> | ||
| <link rel="icon" type="image/png" sizes="32x32" href="assets/favicon-32x32.png"> --> | ||
| <link rel="icon" type="image/png" sizes="32x32" href="assets/favicon-32x32.png">--> |
There was a problem hiding this comment.
The favicon tags are still inside an HTML comment (opened at line 9). As written, browsers will ignore all three favicon links, so this change won’t actually “uncomment” them. Remove the surrounding (or move the comment markers so the tags are not commented) to ensure favicons load on the login/registration pages.
| <!--<link rel="icon" type="image/x-icon" href="assets/favicon.ico"> | ||
| <link rel="icon" type="image/png" sizes="16x16" href="assets/favicon-16x16.png"> | ||
| <link rel="icon" type="image/png" sizes="32x32" href="assets/favicon-32x32.png"> --> | ||
| <link rel="icon" type="image/png" sizes="32x32" href="assets/favicon-32x32.png">--> |
There was a problem hiding this comment.
PR description mentions uncommenting favicon links in both index.html and index.saas.html, but index.saas.html still has the favicon tags commented out. If SaaS builds use index.saas.html, favicons still won’t appear there; please apply the same change to index.saas.html (or update the PR description if it’s not needed).
| const newLink = document.createElement('link'); | ||
| newLink.rel = 'icon'; | ||
| newLink.href = whiteLabelSettings.favicon; | ||
| document.head.appendChild(newLink); |
There was a problem hiding this comment.
When a white-label favicon is present, this always appends a new without removing/updating existing icon links. That can leave multiple favicons in (including the defaults) and the chosen favicon may become browser-dependent. Prefer updating an existing icon link or clearing prior "link[rel~='icon']" entries before inserting the white-label favicon (and consider setting an appropriate type, e.g. image/png).
| const newLink = document.createElement('link'); | |
| newLink.rel = 'icon'; | |
| newLink.href = whiteLabelSettings.favicon; | |
| document.head.appendChild(newLink); | |
| const head = document.head || document.getElementsByTagName('head')[0]; | |
| const existingIcons = head.querySelectorAll("link[rel~='icon']"); | |
| existingIcons.forEach((icon) => head.removeChild(icon)); | |
| const newLink = document.createElement('link'); | |
| newLink.rel = 'icon'; | |
| newLink.type = 'image/png'; | |
| newLink.href = whiteLabelSettings.favicon; | |
| head.appendChild(newLink); |
Uncomment favicon links in index.html and index.saas.html so favicons display before authentication. Update favicon background from purple to blue (#2463EB). Simplify white-label favicon override in app component.