feat: adopt the Eclipse Foundation logo on the website and README - #66
Conversation
Replaces the placeholder app icon with the official horizontal lockup and icon-only mark. The icon-only mark and the dark-background SVGs are derived from the supplied horizontal artwork: the mark drops the wordmark paths and re-frames the viewBox, and the dark variants flip the two fill colours, which matches the supplied dark-background PNGs. The vertical lockups are not included because nothing uses them yet. The logo was designed and contributed by the Eclipse Foundation. Thanks to Elspeth Tory and her team for creating it.
|
EclipseSourceAI
left a comment
EclipseSourceAI
left a comment
There was a problem hiding this comment.
Note
Autonomous AI review.
This review was done by an AI agent and therefore may contain mistakes. Feel free to ignore any comment you disagree with. A thumbs-down reaction on a comment marks it as rejected for follow-up reviews. Noting why in a reply helps, since replies are read too.
Resolving all AI comments does not lead to an automatic approval. A maintainer still needs to review and sign off on the overall architecture and design.
To get an updated review after pushing changes, a maintainer may re-request a review from this account.
Running in Eclipse Enclave, submitted via review-guard-mcp
Swaps the placeholder app icon for the Eclipse Foundation logo across the marketing site, the Docusaurus docs, and the README, and deletes the three superseded placeholders.
I verified the asset derivation mechanically: the -darkbg SVGs are byte-identical to the light ones modulo #231f20/#221f20 → #ffffff, the mark SVGs are identical between website/assets/ and website/docs/static/img/, and neither SVG carries orphaned cls-* rules or unreferenced clippath/gradient ids after the wordmark was stripped. Social card is 1200x630 (matching the summary_large_image switch), favicon 128x128, README PNGs 800x312. No references to the deleted appicon.png / img/logo.png remain anywhere in the tree, and the duplication between website/assets/ and website/docs/static/img/ is required by website/build.sh, which copies the two trees separately.
Points worth a maintainer's attention:
- The
.nav-brand imgcomment justifieswidth: autowith the mark being taller than wide, but the mark'sviewBoxis exactly square, so the rule is equivalent to the 28x28 it replaces. - The docs footer
width: 168runs into infima's.footer__logocap. docs/assets/is inside the sandbox image build context and its content hash, so README branding churn invalidates the image for every user.- The logo is Eclipse Foundation artwork now checked in under the repo's blanket MIT declaration, and
NOTICE.mdonly names the trademark.
The footer-logo-stays-unlinked reasoning holds up: with autoAddBaseUrl on, pathname:/// gets withBaseUrl applied and lands back under /docs/.
| /* The mark is slightly taller than wide, so pin the height and let width follow. */ | ||
| .nav-brand img { height: 30px; width: auto; } |
There was a problem hiding this comment.
enclave-mark-lightbg.svg has viewBox="19.258 7.57 71.021 71.021", so it is exactly square and width: auto resolves to 30px. The comment's premise does not hold, and the only real change here is 28px to 30px (old rule).
| logo: { | ||
| alt: 'Eclipse Enclave', | ||
| src: 'img/enclave-logo-horizontal-darkbg.svg', | ||
| width: 168, |
There was a problem hiding this comment.
Infima caps .footer__logo at max-width: 10rem (160px) and custom.css does not override it, so 168 renders clamped. Either drop to 160 or add the override next to the other .footer rules.
There was a problem hiding this comment.
Checked this and the premise does not hold: --ifm-footer-logo-max-width is min(30rem, 90vw) in the bundled Infima, not 10rem.
infima/dist/css/default/default.css:317 --ifm-footer-logo-max-width: min(30rem, 90vw);
infima/dist/css/default/default.css:2219 .footer__logo { max-width: var(--ifm-footer-logo-max-width); }
The built site carries the same value (--ifm-footer-logo-max-width:min(30rem,90vw)), so the cap is 480px and width: 168 renders unclamped. Leaving it at 168 to match the marketing footer, which uses the same width. No change made.
| <img src="docs/assets/appicon.png" alt="Enclave" width="128" height="128"> | ||
| <picture> | ||
| <source media="(prefers-color-scheme: dark)" srcset="docs/assets/enclave-logo-horizontal-darkbg.png"> | ||
| <img src="docs/assets/enclave-logo-horizontal-lightbg.png" alt="Eclipse Enclave" width="400"> |
There was a problem hiding this comment.
docs/ is copied into the sandbox image and feeds the image build hash (Dockerfile path, hash input), so these README-only PNGs force an image rebuild for every user whenever the branding is touched. .dockerignore already has a "repo-only content" block (here) that docs/assets would fit into.
| </picture> | ||
| </p> | ||
|
|
||
| <h1 align="center">Eclipse Enclave</h1> |
There was a problem hiding this comment.
Dropping the <h1> leaves the README with no top-level heading, so GitHub's outline and any renderer that ignores <picture> lose the project name. The PR description does not mention this one; a maintainer should confirm it is intended.
There was a problem hiding this comment.
Fixed, though not quite as suggested. Rather than adding back a text <h1> that repeats the wordmark already in the logo, the logo now is the <h1>:
<h1 align="center">
<picture>
...
<img src="..." alt="Eclipse Enclave" width="400">
</picture>
</h1>That restores the single top-level heading you flagged, and the accessible name comes from the alt. It also keeps the heading rule GitHub draws under an <h1>, which the README already had before this PR.
| The Eclipse Foundation logo comes in a horizontal lockup (mark plus wordmark) | ||
| and an icon-only mark, each with a variant for light and for dark backgrounds. | ||
| The `-darkbg` variants are the same artwork with the cube faces, inner disc, and | ||
| wordmark in white; do not mix them up, or the wordmark disappears. |
There was a problem hiding this comment.
The logo artwork is Eclipse Foundation branding, but NOTICE.md only covers the name (Trademarks section), so the checked-in files fall under the blanket MIT declaration by default. Adding a line there recording the origin and that trademark terms apply to the marks would be cheap insurance.
There was a problem hiding this comment.
Not doing this one, on maintainer instruction rather than on the merits. The decision for this PR was to keep the logo credit in the commit message and the PR description only, and to leave NOTICE.md and the site itself untouched.
Flagging that the underlying point is separate from attribution: whether the checked-in artwork needs its own NOTICE.md entry recording origin and trademark terms is an IP question for a maintainer, and it applies to the logo regardless of this PR. Worth its own issue if we want it settled.
| * electric beam-blue core, and a gateway amber. The page reads as a bounded, | ||
| * The palette predates the Eclipse Foundation brand logo and does not derive | ||
| * from it: a deep navy field, an electric beam-blue core, and a gateway amber. | ||
| * The logo's teal-to-violet gradient is left to the logo. The page reads as a bounded, |
There was a problem hiding this comment.
This line runs to 87 columns while the rest of the block wraps at ~79. Reflow it.
Keep docs/assets out of the runtime image build context. docs/ feeds the image build hash, so README-only branding would rebuild the image for every user whenever the logo changes. Restore a top-level heading in README.md by wrapping the logo in the <h1> rather than repeating the wordmark as text, drop a nav-brand comment whose premise was wrong (the mark's viewBox is square), and reflow an overlong comment line.
2 participants
Replaces the placeholder app icon with the official Eclipse Enclave logo across the marketing site, the Docusaurus docs, and the repository README.
Credits
The logo was designed and contributed by the Eclipse Foundation. Many thanks to Elspeth Tory and her team for creating it and making it available to the project.
What changed
Only the variants that something actually uses are checked in. Two of them are derived from the supplied horizontal artwork:
viewBoxre-framed to a square around the mark, using measurements taken from the reference PNG so the framing is exact.darkbg, so the SVG variants flip the two fill colours (#231f20/#221f20→#ffffff). The result was checked against the supplied dark-background PNGs.docs/assets/enclave-logo-horizontal-{lightbg,darkbg}.png<picture>for light/darkwebsite/assets/enclave-mark-lightbg.svg,enclave-logo-horizontal-lightbg.svg,favicon.png,social-card.pngwebsite/docs/static/img/enclave-mark-{lightbg,darkbg}.svg,enclave-logo-horizontal-darkbg.svg,favicon.png,social-card.pngsrc/srcDark), docs footer, favicon, OG cardAlso in scope:
<h1>, so the file keeps a single top-level heading without repeating the wordmark as text.docs/assets/is added to the.dockerignore"repo-only content" block.docs/feeds the runtime image build context and its hash, so README-only branding would otherwise rebuild the image for every user whenever the logo changes.twitter:cardis nowsummary_large_image; the old card pointed at a square app icon.docs/assets/appicon.png,website/assets/appicon.png,website/docs/static/img/logo.png).Notes for review
themeConfig.footer.logovalidates against a schema that rejectsautoAddBaseUrl, and Docusaurus'Linkwould rewrite apathname://href back under/docs/. Documented inwebsite/docs/README.mdnext to the existing linking-out gotcha.NOTICE.mdis intentionally untouched. Credit for this contribution lives in the commit message and here. Whether the checked-in artwork warrants its ownNOTICE.mdentry recording origin and trademark terms is a separate IP question that applies to the logo regardless of this PR.Verification
make build,make lint, andmake check-license-headerspass, and the Docusaurus site builds with the new assets resolving in the generated HTML. The.dockerignoreexclusion was checked against the samepatternmatcher/ignorefilepair the build hash uses:docs/assets/**is ignored whiledocs/README.mdanddocs/ARCHITECTURE.mdare not.make testhas one failure,TestDockerImportsConfinedToBackendAndCarveOuts. It is pre-existing and unrelated — it also fails on a clean tree, caused by a stale checkout under.claude/worktrees/that the arch test scans.