feat: add app logo as favicon, navbar brand icon, and README branding#281
Conversation
Replace the generated SVG icon with the new logo (ai-git-bot-new-logo.png): - favicon.png copied to static/images for browser tab favicon - layout.html, login.html, setup.html: swap favicon.svg → favicon.png (favicon <link> + brand/navbar <img> + login/setup screen logo) - README.md + translations (JA/KO/ZH): inline logo with h1 title
E2E Test Run for PR #281🤖 Starting end-to-end test run — this typically takes several minutes.
I'll deploy a preview environment, generate the test suite and post the results here when the run finishes. Use |
max-california
left a comment
There was a problem hiding this comment.
🤖 Agentic Code Review
Summary: the UI-side asset swap looks correct and low-risk, but the README heading change introduces a portability/rendering issue in common Markdown viewers. I’d block on the docs change unless that presentation has been verified on the intended platforms.
Blocking
-
README headings are no longer plain Markdown headings
InREADME.md:1,README.ja.md:1,README.ko.md:1, andREADME.zh.md:1, the PR replaces# AI-Git-Botwith an HTML<img .../>embedded directly in the heading.This is visually nice on GitHub, but it’s not reliably rendered across Markdown tooling:
- many Markdown parsers/sanitizers strip inline styles like
style="margin-right: 8px;" - some renderers handle raw HTML in headings inconsistently
- some ecosystems derive TOC/anchors/title extraction from plain heading text and can behave oddly with embedded HTML
Since the repo maintains multiple localized READMEs, this change multiplies that compatibility risk four times for a purely cosmetic benefit.
Suggestion: keep the heading as plain Markdown and place the logo on the next line, e.g.:
- plain
# AI-Git-Bot - then a separate
<img ...>or standard Markdown image below it
- many Markdown parsers/sanitizers strip inline styles like
Non-blocking
-
Consider keeping the SVG favicon for browser tab usage
The template changes insrc/main/resources/templates/layout.html,login.html, andsetup.htmlswitch the favicon fromfavicon.svgtofavicon.png, whilesrc/main/resources/static/images/favicon.svgstill exists.This is not incorrect, but SVG favicons generally scale better and stay sharper across resolutions. If the PNG is specifically needed because the new logo doesn’t work well as SVG, that’s fine; otherwise you may want to either:
- keep SVG for
<link rel="icon">and use PNG only for<img>tags, or - provide both formats for broader compatibility
- keep SVG for
Overall, the app template changes look straightforward and consistent; the only substantive concern I found is the README heading markup portability.
Read-only agentic review by AI Git Bot
Replace the generated SVG icon with the new logo (ai-git-bot-new-logo.png): - favicon.png copied to static/images for browser tab favicon - layout.html, login.html, setup.html: swap favicon.svg → favicon.png (favicon <link> + brand/navbar <img> + login/setup screen logo) - README.md + translations (JA/KO/ZH): inline logo with h1 title
Replace the generated SVG icon with the new logo (ai-git-bot-new-logo.png): - favicon.png copied to static/images for browser tab favicon - layout.html, login.html, setup.html: swap favicon.svg → favicon.png (favicon <link> + brand/navbar <img> + login/setup screen logo) - README.md + translations (JA/KO/ZH): inline logo with h1 title
|
@clicki-mc-testface regenerate-tests |
E2E Test Run for PR #281🤖 Starting end-to-end test run — this typically takes several minutes.
I'll deploy a preview environment, generate the test suite and post the results here when the run finishes. Use |
clicki-mc-testface
left a comment
There was a problem hiding this comment.
E2E Test Run for PR #281
Framework: playwright
Preview environment: http://46.225.226.81:20047
Source SHA: be1c77a5
Outcome: ⏭️ SKIPPED
No test cases were generated. TestPlannerAgent returned no journeys — nothing to author or run
|
@clicki-mc-testface regenerate-tests Add test that checks for presence of the new logo, first as favicon and second in the navigation bar |
E2E Test Run for PR #281🤖 Starting end-to-end test run — this typically takes several minutes.
I'll deploy a preview environment, generate the test suite and post the results here when the run finishes. Use |
clicki-mc-testface
left a comment
There was a problem hiding this comment.
E2E Test Run for PR #281
Framework: playwright
Preview environment: http://46.225.226.81:20048
Source SHA: be1c77a5
Outcome: ⏭️ SKIPPED
No test cases were generated. TestPlannerAgent returned no journeys — nothing to author or run
|
@max-california Since our primary target is github we can check if the md-formatting works for github, we don't have the requirment that it should looks the same on every MD-Renderer |
🤖 Follow-upYes — if GitHub is the only renderer you care about, that weakens the portability concern a lot. For this PR specifically, the README change is: # <img src="doc/images/ai-git-bot-new-logo.png" alt="Logo" width="32" align="left" style="margin-right: 8px;"/> AI-Git-BotOn GitHub, inline HTML in Markdown is generally supported, so the image itself should render. However, I’d still be a bit cautious about relying on the exact layout/styling details:
So my updated take would be:
In short: yes, GitHub-only support is a reasonable justification here; just confirm the actual GitHub rendering before merging. Read-only agentic review follow-up by AI Git Bot |
No description provided.