Repository navigation
fix(brand): ship favicon.zip from #1235 (correct shield-with-cross mark) - #1331
Merged
Merged
Conversation
#1251 closed #1235 with a hand-authored "S" rounded-square SVG that matched .sidebar__brand-mark, but rumblefrog had attached the actual intended favicon set as a comment on the issue (favicon.zip). The shipped icon was therefore wrong on the dimension that mattered: it was a stylistic riff on the sidebar brand, not the canonical SourceBans++ shield-with-cross mark. Replace the icon artwork byte-faithful from the favicon.zip: - favicon.svg → orange shield (#ea580c) with a black "+" centred and a small white "++" tucked in the bottom-right (the literal "++" in SourceBans++). - favicon.ico → 3-icon (48x48 + 32x32 + 16x16) — was 2-icon prior. - apple-touch-icon-180.png → repainted from the zip's apple-touch-icon (kept the existing filename so the header.tpl <link> doesn't move). - favicon-96x96.png (new) → higher-DPI tab strip source. - web-app-manifest-{192,512}x{192,512}.png (new) → maskable PWA install sources referenced by the new manifest. - site.webmanifest (new) → wires up the install-as-PWA path. Faithful to the zip with one fix: icon paths drop the leading slash so they resolve relative to the manifest URL (subdir installs would 404 on the root-relative shape the zip ships). Replaces both copies (themes/default/images/ + install/images/). Wire-up changes in web/themes/default/core/header.tpl: - Add <link rel="icon" type="image/png" sizes="96x96"> for the new 96 source. - Add <link rel="manifest"> for the PWA install path. - Refresh the explanatory comment to describe the new mark and the manifest plumbing (the prior comment described the "S" SVG that's no longer there). Theme-color metas + the SVG / ICO / apple-touch-icon <link> tags from #1251 are unchanged — that scaffolding was correct; only the asset behind it was wrong. The legacy installer (web/install/template/header.php) needs no markup change — it already references favicon.svg from the theme and favicon.ico from install/images/, both of which now ship the new mark. Closes #1235
The `template.logo` admin setting (Admin → Settings → General → Logo
path, default `logos/sb-large.png`) was a vestigial v1.x setting in
v2.0 — the legacy panel rendered it as a top-banner `<img src="$logo">`,
and when the v2.0 chrome rewrote the layout to use `.sidebar__brand-mark`
(a CSS-rendered "S" rounded-square), no template was updated to consume
`$logo` anymore. The setting still appeared in the admin UI and saved
back to a row nobody read; the default value (`logos/sb-large.png`)
pointed at a path that didn't even exist in `web/themes/default/`.
This commit resurrects the setting and wires it into the three places
the chrome renders the brand mark:
- `core/navbar.tpl` — the sidebar's top-left mark (every authenticated
route). Renders `<img src="{$theme_url}/{$logo}" …>`. No View binds
to this template (it's rendered procedurally by `core/navbar.php`),
so SmartyTemplateRule doesn't fire — and `$logo` is in scope via the
`$theme->assign('logo', …)` call in `core/header.php`, which runs
earlier in the page-builder lifecycle (header → navbar → title →
page → footer).
- `page_login.tpl` — the sign-in card's brand row. Same swap, but the
template binds to `Sbpp\View\LoginView` so SmartyTemplateRule cross-
checks property↔reference parity. Pre-resolves the joined URL in
`page.login.php` and passes a single `$brand_logo_url` property
(cleaner than threading both `$theme_url` + `$logo` through the View
surface for what's just a precomputed `themes/<theme>/<path>`).
- `updater.tpl` — the DB updater's header. The updater runs in its own
bootstrap context (`web/updater/index.php`) which doesn't load
`core/header.php`, so `$logo` / `$theme_url` aren't in scope. Hardcoded
to `../themes/default/images/favicon.svg` here — the updater is an
internal one-off page (admin-only, runs once per upgrade), and
threading the setting through `UpdaterView` would be plumbing for
zero customisation value.
Default value flips from the broken-in-v2.0 `logos/sb-large.png` to
`images/favicon.svg` (the SourceBans++ shield from #1235's favicon set):
- `web/install/includes/sql/data.sql` — fresh installs.
- `web/updater/data/809.php` (registered in `store.json`) — paired
migration converts existing installs whose value is still the v1.x
default forward to the new default. Conditional WHERE means admins
with custom values are untouched. Idempotent (the WHERE matches
zero rows on re-run).
CSS: `.sidebar__brand-mark` rule drops the orange-rounded-square +
centered-letter styling (`background`, `color`, `display: grid;
place-items: center`, `font-*`, `border-radius`) that was designed for
the pre-#1235 `<div>S</div>` shape. The new mark's visual identity
(orange shield + cross) lives in the SVG itself; the CSS keeps just
sizing + flex-shrink so the brand row's layout is unchanged.
Settings UI: adds a `<p class="settings-fieldset__help">` below the
"Logo path" input + `aria-describedby` link — text explains it's
theme-relative, what the default is, and what surfaces it controls.
Same shape as the sibling token-lifetime help paragraphs.
Refs #1235
rumblefrog
force-pushed
the
fix/issue-1235-favicon-correct-asset
branch
from
May 10, 2026 18:49
7ee287f to
54b0392
Compare
3 of 5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#1251 closed #1235 with a hand-authored "S" rounded-square SVG that mimicked
.sidebar__brand-mark— but rumblefrog had attached the actual intended favicon set as a comment on the issue (favicon.zip). The shipped icon was therefore wrong on the dimension that mattered: stylistic riff on the sidebar brand, not the canonical SourceBans++ shield-with-cross mark (orange shield, black+centred, small white++tucked in the bottom-right — the literal "++" in SourceBans++).This PR is two paired commits:
Commit 1 —
fix(brand): ship favicon.zip from #1235 (correct shield-with-cross mark)Replaces the icon artwork byte-faithful from the favicon.zip and adds the manifest plumbing the zip ships:
favicon.svgSrounded-square w/prefers-color-scheme: darkrulefavicon.ico(theme + install)apple-touch-icon-180.pngapple-touch-icon.png(also 180×180); kept the existing filename soheader.tpl's<link>doesn't movefavicon-96x96.pngweb-app-manifest-{192,512}x{192,512}.pngsite.webmanifestweb/themes/default/core/header.tplpicks up two new<link>tags (<link rel="icon" sizes="96x96">,<link rel="manifest">) and the explanatory comment is refreshed to describe the new mark + manifest plumbing. Thetheme-colormetas + the SVG / ICO / apple-touch-icon<link>tags from #1251 are unchanged — that scaffolding was correct; only the asset behind it was wrong.The legacy installer (
web/install/template/header.php) needs no markup change — it already referencesfavicon.svgfrom the theme andfavicon.icofrominstall/images/, both of which now ship the new mark.One deviation from byte-faithful
The zip's
site.webmanifestships icon paths with a leading slash (/web-app-manifest-192x192.png), which only resolves correctly when the panel is hosted at the document root. Every typical SourceBans deploy lives in a subdirectory, so the leading slash is dropped — paths now resolve relative to the manifest URL itself. Same effect on a root-mounted install, working fix on a subdir install. The manifest'snamefield is also normalised from "Sourcebans++" (the zip's casing — looks like a Real Favicon Generator default that lowercased the input) to "SourceBans++" (the canonical project capitalisation).theme_color/background_colorin the manifest are left at the zip's#ffffff— those only affect installed-PWA chrome, which is a niche use case for an admin panel; the in-browser<meta name="theme-color">tags continue to provide the brand-orange (#ea580c) light + zinc-950 (#09090b) dark treatment unchanged.Commit 2 —
fix(brand): wire template.logo setting back into the chrome brand markThe
template.logoadmin setting (Admin → Settings → General → Logo path) was a vestigial v1.x setting in v2.0 — the legacy panel rendered it as a top-banner<img src="$logo">, and when the v2.0 chrome rewrote the layout to use.sidebar__brand-mark(a CSS-rendered "S" rounded-square), no template was updated to consume$logoanymore. The setting still appeared in the admin UI and saved back to a row nobody read; the default value (logos/sb-large.png) pointed at a path that didn't even exist inweb/themes/default/.Without this commit, the favicon swap above creates visual inconsistency: tab shows the new shield, sidebar / sign-in card / updater header show the old "S" / "SB". This commit resurrects the setting and wires it into the three brand-mark surfaces:
core/navbar.tpl— the sidebar's top-left mark on every authenticated route. Renders<img src="{$theme_url}/{$logo}" …>. No View binds to this template (it's rendered procedurally bycore/navbar.php), so SmartyTemplateRule doesn't fire — and$logois in scope viacore/header.php's assign (header runs before navbar in the page-builder lifecycle).page_login.tpl— the sign-in card's brand row. Same swap, but the template binds toSbpp\View\LoginViewso SmartyTemplateRule cross-checks property↔reference parity. Pre-resolves the joined URL inpage.login.phpand passes a single$brand_logo_urlproperty (cleaner than threading both$theme_url+$logothrough the View surface for what's just a precomputedthemes/<theme>/<path>).updater.tpl— the DB updater's header. The updater bootstraps in its own context (web/updater/index.php) which doesn't loadcore/header.php, so$logo/$theme_urlaren't in scope. Hardcoded to../themes/default/images/favicon.svghere — the updater is an internal one-off page (admin-only, runs once per upgrade), and threading the setting throughUpdaterViewwould be plumbing for zero customisation value.Default value flips from the broken-in-v2.0
logos/sb-large.pngtoimages/favicon.svg:web/install/includes/sql/data.sql— fresh installs.web/updater/data/808.php(registered instore.json) — paired migration converts existing installs whose value is still the v1.x default forward to the new default. Conditional WHERE means admins with custom values are untouched. Idempotent (the WHERE matches zero rows on re-run).CSS:
.sidebar__brand-markrule drops the orange-rounded-square + centered-letter styling (background,color,display: grid; place-items: center,font-*,border-radius) that was designed for the pre-#1235<div>S</div>shape. The new mark's visual identity (orange shield + cross) lives in the SVG itself; the CSS keeps just sizing + flex-shrink so the brand row's layout is unchanged.Settings UI: adds a
<p class="settings-fieldset__help">below the "Logo path" input +aria-describedbylink. Same shape as the sibling token-lifetime help paragraphs.Closes #1235
Test plan
Favicon set (commit 1)
web-app-manifest-{192,512}x{192,512}.png.favicon-96x96.pngis requested by Chrome on a high-DPI display.#ea580c. Dark OS:#09090b. (HTML metas; manifest's#ffffffdoesn't apply in browser mode.)web/install/→ the wizard's tab favicon shows the new shield-with-cross mark too (same.ico+ the SVG<link>to the theme)./sourcebans/): manifest icons resolve relative to the manifest URL, not 404 against the document root.Brand mark wire-up (commit 2)
?p=login→ sign-in card's brand row shows the orange shield SVG, not the "S" letterform.web/updater/→ the updater header shows the orange shield SVG, not the "SB" letterform.aria-describedbyis set on the input.template.logoto a custom path (e.g.images/favicon-96x96.png) → the sidebar / sign-in card immediately render the new asset on next page load. Restore default → reverts.template.logois seeded asimages/favicon.svg.template.logowas the v1.x defaultlogos/sb-large.png: the808.phpmigration converts it forward toimages/favicon.svg. Admins who customised the value see no change.808.phpno-ops (zero rows match the WHERE).