chore: add brand assets + sync infrastructure#212
Merged
Conversation
Single source of truth at assets/branding/ (12 SVG: 6 sizes x solid/transparent + README). Propagated copies in ui/public/branding/, docs/public/branding/, root favicons. Sync via scripts/sync-branding.ps1 (never edit propagated copies directly). Code wiring: - ui/src/components/layout/AppSidebar.vue: replace placeholder div with favicon-32.svg - ui/index.html, docs/index.html: favicon links + apple-touch-icon + theme-color #0C0F1A - README.md/ru/zh: brand mark above header (icon-256, 128px) ui/DESIGN.md updated: - new colors.brand-ink token - brand: block in YAML frontmatter (paths + usage map) - ## Brand Mark body section (canonical source rule, sizing rules, gallery link) - 5 new Donts (recolor, shadows, sizing, transparent on light, propagated-copy edits)
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (4)
WalkthroughPR добавляет систему брендовых активов: документацию и правила, скрипт синхронизации SVG, публичную галерею, favicon метаданные, интеграцию иконки в Sidebar и маршруты/исключения для статических брендинговых файлов. ChangesBranding Assets & Integration System
🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Owner
Author
|
@coderabbitai review |
Owner
Author
|
@codex review |
✅ Actions performedReview triggered.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive branding system for Engram, including canonical SVG assets, a synchronization script, and updated design documentation. Key updates involve integrating the new icons into the project's READMEs, HTML headers, and UI sidebar, as well as creating a live brand gallery. Feedback from the review suggests improving the cross-platform compatibility of the sync script and removing hardcoded local development paths and domains from the documentation.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f05901e45c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
docs/index.html (1)
11-11:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winТа же проблема с
apple-touch-icon(SVG), что и вui/index.htmlстрока 9.SVG не поддерживается в качестве
apple-touch-iconна iOS. Исправление аналогично описанному в комментарии кui/index.html.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/index.html` at line 11, В данном документе есть ссылка <link rel="apple-touch-icon" href="..."> указывающая на SVG (engram-icon-256.svg), а iOS не поддерживает SVG для apple-touch-icon; замените ссылку на растровый PNG (например engram-icon-180.png или другой подходящий размер), добавьте корректный размер атрибута (sizes="180x180") и обеспечьте наличие файла PNG в ресурсах, аналогично исправлению в ui/index.html.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/sync-branding.ps1`:
- Around line 34-39: В текущем цикле синхронизации (переменные $targets,
$svgFiles, $dest, Copy-Item) нужно не только копировать/перезаписывать, но и
удалять устаревшие SVG из целевой папки: перед копированием получи список
существующих SVG в $dest (Get-ChildItem *.svg), вычисли имена файлов, которых
нет в $svgFiles (сравни по .Name), и удаляй их (Remove-Item -Force), затем
выполняй New-Item/Copy-Item как сейчас — это гарантирует, что ui/public/branding
и docs/public/branding будут зеркалить assets/branding.
In `@ui/DESIGN.md`:
- Around line 232-233: В разделе DESIGN.md исправить конфликт в описании
варианта логотипа для sidebar: согласовать значение поля sidebar-logo (текущее
значение favicon-32.svg — solid) с упоминаниями в секциях "usage" и "Sidebar"
или явно разделить на два разных поля (например sidebar-logo-solid и
sidebar-logo-transparent) и обновить ссылки/описания соответственно; также
проверьте и синхронизируйте связанные поля apple-touch-icon и любые упоминания
тех же ассетов в других местах (референты: ключи sidebar-logo и apple-touch-icon
и секции Usage/Sidebar).
In `@ui/index.html`:
- Line 9: The apple-touch-icon currently points to an SVG which iOS doesn't
support; add a 180x180 PNG named apple-touch-icon.png into assets/branding/,
update the link href in ui/index.html (replace the SVG href) to reference the
PNG, and modify scripts/sync-branding.ps1 to copy/sync PNG files alongside SVGs;
also apply the same href change in docs/index.html (the corresponding link on
line 11). Ensure the PNG filename and link are consistent across all files and
the sync script includes *.png in its source list.
In `@ui/public/branding/index.html`:
- Around line 502-503: В футере удалите утёкший абсолютный путь разработчика и
замените его на безопасный/динамический текст: уберите строку с literal "Source:
<code>D:\Dev\engram\ui\public\branding\</code>" в том HTML-блоке и либо оставьте
только "Engram dashboard" с датой, либо подставляйте публично безопасную
информацию (например версия из package.json или относительный URL) при сборке;
отредактируйте тот же фрагмент в index.html рядом с элементом <span id="ts">
(скрипт, который устанавливает дату) чтобы выводил только допустимый
текст/метаданные, а не локальный путь разработчика.
- Around line 450-452: Replace the incorrect import example that uses "import
logoUrl from '/branding/engram-icon.svg?url'" and the misleading comment
"cache-busted by Vite" with a direct absolute public URL reference and remove
any local developer filesystem paths; specifically, update the usage around the
logoUrl import and the <img :src="logoUrl"> example to use an absolute path like
"/branding/engram-icon.svg" in the markup (no JS import) and delete the Windows
path "D:\Dev\engram\ui\public\branding\" from the public file content/comments.
---
Duplicate comments:
In `@docs/index.html`:
- Line 11: В данном документе есть ссылка <link rel="apple-touch-icon"
href="..."> указывающая на SVG (engram-icon-256.svg), а iOS не поддерживает SVG
для apple-touch-icon; замените ссылку на растровый PNG (например
engram-icon-180.png или другой подходящий размер), добавьте корректный размер
атрибута (sizes="180x180") и обеспечьте наличие файла PNG в ресурсах, аналогично
исправлению в ui/index.html.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f4531194-bdb8-4ed2-a418-a7ba2bf74d93
⛔ Files ignored due to path filters (38)
assets/branding/engram-icon-256-transparent.svgis excluded by!**/*.svgassets/branding/engram-icon-256.svgis excluded by!**/*.svgassets/branding/engram-icon-512-transparent.svgis excluded by!**/*.svgassets/branding/engram-icon-512.svgis excluded by!**/*.svgassets/branding/engram-icon-transparent.svgis excluded by!**/*.svgassets/branding/engram-icon.svgis excluded by!**/*.svgassets/branding/favicon-16-transparent.svgis excluded by!**/*.svgassets/branding/favicon-16.svgis excluded by!**/*.svgassets/branding/favicon-32-transparent.svgis excluded by!**/*.svgassets/branding/favicon-32.svgis excluded by!**/*.svgassets/branding/favicon-transparent.svgis excluded by!**/*.svgassets/branding/favicon.svgis excluded by!**/*.svgdocs/public/branding/engram-icon-256-transparent.svgis excluded by!**/*.svgdocs/public/branding/engram-icon-256.svgis excluded by!**/*.svgdocs/public/branding/engram-icon-512-transparent.svgis excluded by!**/*.svgdocs/public/branding/engram-icon-512.svgis excluded by!**/*.svgdocs/public/branding/engram-icon-transparent.svgis excluded by!**/*.svgdocs/public/branding/engram-icon.svgis excluded by!**/*.svgdocs/public/branding/favicon-16-transparent.svgis excluded by!**/*.svgdocs/public/branding/favicon-16.svgis excluded by!**/*.svgdocs/public/branding/favicon-32-transparent.svgis excluded by!**/*.svgdocs/public/branding/favicon-32.svgis excluded by!**/*.svgdocs/public/branding/favicon-transparent.svgis excluded by!**/*.svgdocs/public/branding/favicon.svgis excluded by!**/*.svgdocs/public/favicon.svgis excluded by!**/*.svgui/public/branding/engram-icon-256-transparent.svgis excluded by!**/*.svgui/public/branding/engram-icon-256.svgis excluded by!**/*.svgui/public/branding/engram-icon-512-transparent.svgis excluded by!**/*.svgui/public/branding/engram-icon-512.svgis excluded by!**/*.svgui/public/branding/engram-icon-transparent.svgis excluded by!**/*.svgui/public/branding/engram-icon.svgis excluded by!**/*.svgui/public/branding/favicon-16-transparent.svgis excluded by!**/*.svgui/public/branding/favicon-16.svgis excluded by!**/*.svgui/public/branding/favicon-32-transparent.svgis excluded by!**/*.svgui/public/branding/favicon-32.svgis excluded by!**/*.svgui/public/branding/favicon-transparent.svgis excluded by!**/*.svgui/public/branding/favicon.svgis excluded by!**/*.svgui/public/favicon.svgis excluded by!**/*.svg
📒 Files selected for processing (10)
README.mdREADME.ru.mdREADME.zh.mdassets/branding/README.mddocs/index.htmlscripts/sync-branding.ps1ui/DESIGN.mdui/index.htmlui/public/branding/index.htmlui/src/components/layout/AppSidebar.vue
- scripts/sync-branding.ps1: use forward slashes in Join-Path string
literals for PowerShell Core cross-platform compatibility (Linux/macOS)
- ui/public/branding/index.html: replace hardcoded unleashed.lan:37777
with generic your-engram-domain.com placeholder in usage example
- ui/public/branding/index.html: replace absolute Windows path in footer
with repo-relative assets/branding/ reference
- internal/worker/service.go: add /branding/* and /favicon.svg routes so
brand assets are reachable when dashboard is served by the worker
- internal/worker/middleware.go: exempt /branding/* and /favicon.svg from
token auth (public static assets, same treatment as /assets/*)
Reviewer comment ids: PRRC_kwDORY-ek86-shrt, PRRC_kwDORY-ek86-shrz,
PRRC_kwDORY-ek86-shsJ, PRRC_kwDORY-ek86-skE1
- scripts/sync-branding.ps1: remove stale SVGs from target directories before copying (fixes sync divergence when a source file is renamed or deleted). Adds $sourceNames diff + Remove-Item pipeline per CR suggestion. - ui/DESIGN.md: remove conflicting "sidebar logo" example from the Transparent variant description; the Sidebar section already specifies solid favicon-32.svg as the canonical sidebar mark. Reviewer comment ids: PRRC_kwDORY-ek86-sm_6, PRRC_kwDORY-ek86-snAC
iOS Safari does not respect SVG apple-touch-icon. Adds 180x180 raster PNG generated from engram-icon-256.svg via cairosvg, propagated to both ui/public/branding/ and docs/public/branding/ via sync-branding.ps1. sync-branding.ps1: extend to copy SVG + PNG assets and prune stale assets of either type. Source set now declared via *.svg + *.png glob. ui/index.html, docs/index.html: update apple-touch-icon link to PNG with explicit sizes=180x180. Resolves the last MINOR finding from CodeRabbit on PR #212.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3dfffab421
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+745
to
+746
| s.router.Get("/branding/*", serveAssets) | ||
| s.router.Get("/favicon.svg", serveAssets) |
There was a problem hiding this comment.
When the dashboard is served by the worker, these new /branding/* and /favicon.svg routes still go through serveAssets, which only assigns content types for .js and .css; I checked internal/worker/static.go, and Go's fallback detection returns application/octet-stream for these SVGs while SecurityHeaders adds X-Content-Type-Options: nosniff. In that worker-served context the new favicon links and sidebar logo can be rejected or not rendered by browsers even though they work under Vite, so the route needs to serve .svg as image/svg+xml (and keep image MIME handling complete).
Useful? React with 👍 / 👎.
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
1 participant
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
Single source of truth for brand assets at
assets/branding/(12 SVG + README).Propagated copies in
ui/public/branding/,docs/public/branding/, root favicons.What
<div>E</div>-><img src="/branding/favicon-32.svg">#0C0F1Acolors.brand-inktoken,brand:YAML block,## Brand Markbody section, 5 new Don'tsWhy
Brand was a placeholder
<div>E</div>in sidebar and no favicons anywhere. SSOT pattern letsassets/branding/stay the only place to edit; sync script keeps consumers in sync.Touched
48 files, +827/-6. No code logic changes outside Sidebar img swap.
Summary by CodeRabbit
Примечания к выпуску
Chores
Documentation