fix(admin): wire admin.logo/siteName branding into login, signup, and invite-accept pages#1902
Conversation
… invite-accept pages The sidebar and setup wizard already honored the configured admin.logo/ siteName white-label overrides (emdash-cms#639/PR emdash-cms#705), but LoginPage, SignupPage, and InviteAcceptPage still rendered the stock hardcoded EmDash mark since they run before authentication. admin.astro already reads the branding config server-side per request; it's now passed down through AdminWrapper -> AdminApp -> a new AdminBrandingProvider context (mirroring how authProviders reaches the same pre-auth pages), so the configured logo shows on first render with no flash and no unauthenticated API call. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 068a5c0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
review/needs-review
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the right change for the right problem: the approved white-label scope (#639/#705) explicitly covered the pre-auth surfaces, and wiring admin.logo/siteName through a synchronous, server-side prop chain mirrors the existing authProviders pattern and avoids the logo flash that an async fetchManifest() would introduce. The implementation is backward compatible and localized correctly (no new user-facing strings).
I checked the changed Astro route, the new AdminBrandingProvider context, the prop threading through AdminWrapper/AdminApp, and the three page components that now consume it. The runtime logic is clean. My one material reservation is test coverage: only LoginPage.test.tsx received regression tests; SignupPage.test.tsx was not updated and there is no InviteAcceptPage.test.tsx at all, so the same bug could silently regress on the other two surfaces. Per the repo’s TDD-for-bugs convention, a bug without a reproducing test is not fixed.
Headline: merge after adding analogous branding tests for SignupPage and InviteAcceptPage.
| {/* Header */} | ||
| <div className="text-center mb-8"> | ||
| <LogoLockup className="h-10 mx-auto mb-2" /> | ||
| <BrandLogo logoUrl={brandLogo} siteName={brandSiteName} className="h-10 mx-auto mb-2" /> |
There was a problem hiding this comment.
[needs fixing] SignupPage now consumes the new useAdminBranding() context and renders a configurable logo/site name, but packages/admin/tests/components/SignupPage.test.tsx was not updated. Existing tests render SignupPage without a provider and do not assert either the custom-branding or fallback behavior, so the fix here is unverified. Add tests analogous to the new LoginPage regression tests:
it("renders the configured admin logo/site name instead of the stock mark", async () => {
const screen = await render(
<AdminBrandingProvider
adminBranding={{ logo: "https://example.com/logo.png", siteName: "Acme CMS" }}
>
<SignupPage />
</AdminBrandingProvider>,
);
const logoImg = screen.getByRole("img", { name: "Acme CMS" });
await expect.element(logoImg).toBeInTheDocument();
expect(logoImg.element().getAttribute("src")).toBe("https://example.com/logo.png");
expect(screen.getByRole("img", { name: "EmDash" }).query()).toBeNull();
});
it("falls back to the stock EmDash mark when no admin branding is configured", async () => {
const screen = await render(<SignupPage />);
await expect.element(screen.getByRole("img", { name: "EmDash" })).toBeInTheDocument();
});There was a problem hiding this comment.
Added — new renders the configured admin logo/site name instead of the stock mark + falls back to the stock EmDash mark tests in SignupPage.test.tsx, matching the LoginPage pattern. 22/22 tests pass.
| <div className="w-full max-w-md"> | ||
| <div className="text-center mb-8"> | ||
| <LogoLockup className="h-10 mx-auto mb-2" /> | ||
| <BrandLogo logoUrl={brandLogo} siteName={brandSiteName} className="h-10 mx-auto mb-2" /> |
There was a problem hiding this comment.
[needs fixing] InviteAcceptPage now consumes the new branding context and renders BrandLogo, but there is no test file for this component (packages/admin/tests/components/InviteAcceptPage.test.tsx does not exist). The invite-accept surface is part of the stated bug fix, so it also needs a regression test that verifies both the configured-logo path and the fallback to the EmDash lockup. Create InviteAcceptPage.test.tsx with at least those two assertions, using the same AdminBrandingProvider pattern as the new LoginPage tests.
There was a problem hiding this comment.
Added InviteAcceptPage.test.tsx with both requested assertions (configured-logo path via the register step, and fallback to the stock EmDash mark).
| interface AdminWrapperProps { | ||
| locale: string; | ||
| messages: Messages; | ||
| /** Configured admin white-label branding (logo, site name), read server-side from `admin.astro`. */ |
There was a problem hiding this comment.
[suggestion] The adminBranding prop type is duplicated inline here ({ logo?: string; siteName?: string }). Since AdminBranding is now defined in @emdash-cms/admin, consider exporting that type from the admin package and importing it here to avoid drift if more branding fields are added later.
There was a problem hiding this comment.
Done — exported AdminBranding from @emdash-cms/admin and imported it here instead of the inline duplicate.
github-actions
Bot
added
review/awaiting-author
…minBranding type Adds the SignupPage and InviteAcceptPage branding regression tests requested by review on PR emdash-cms#1902 (mirrors the existing LoginPage tests: configured logo/siteName renders, falls back to stock EmDash mark when unconfigured). Also exports `AdminBranding` from @emdash-cms/admin and uses it in PluginRegistry's AdminWrapperProps instead of a duplicated inline type, per the review suggestion.
github-actions
Bot
added
review/needs-rereview
There was a problem hiding this comment.
This is the right change for the right problem: the approved white-label scope (#639/#705) explicitly covered the pre-auth login/onboarding surfaces, and wiring admin.logo/siteName through a synchronous, server-side prop chain mirrors the existing authProviders pattern and avoids a logo flash.
I re-checked the full diff and my previous reservation has been addressed:
SignupPage.test.tsxnow has branding regression tests (configured logo + fallback to stock mark).- A new
InviteAcceptPage.test.tsxwas added with the same two regression tests. LoginPage.test.tsxretains its previously added branding regression tests.
Implementation remains clean:
AdminBrandingProvideris a thin context with a sensible{}default, so unconfigured installs fall back to the stock EmDash mark exactly as before.admin.astroreadsAstro.locals.emdash.config.adminserver-side and passes{ logo, siteName }down throughAdminWrapper→AdminApp, matching theauthProvidersprop chain.LoginPage,SignupPage, andInviteAcceptPagenow consumeuseAdminBranding()and pass the values to the existingBrandLogocomponent, which already had the right fallback logic.- No new user-facing strings are added, so Lingui wrapping stays complete.
- Changeset correctly describes the user-facing fix and applies
patchto both affected packages (emdashand@emdash-cms/admin).
No blocking issues found. LGTM.
github-actions
Bot
added
review/approved
|
Tested #1902's fix shape against our production config (the site from my comment on #1493) — it holds: the login page renders our configured branding. Method, for transparency: I didn't build the fork directly. I verified the branch is exactly upstream Results:
Signup and invite-accept couldn't be exercised live on our instance (public signup disabled; invite requires a valid token), but they consume the identical shared context. From an integrator's perspective this fixes exactly what we hit in production. 👍 (Verified with AI assistance — Claude Code / Claude Fable 5; the backport and browser verification were run by the session and reviewed by @cyface.) |
3 participants
What does this PR do?
Discussion #639 ("Admin white-labeling") was approved and merged as #705, adding
admin: { logo, siteName, favicon }config. That approved scope explicitly listed "Login/onboarding page (large logo)" as a target surface, but it was never wired up —LoginPage,SignupPage, andInviteAcceptPagestill render the stock hardcoded EmDash mark instead of the configured branding, so a configured logo doesn't show until after sign-in.Root cause:
authProvidersreaches these pre-auth pages via a synchronous server-side prop chain (admin.astro→AdminWrapper→AdminApp→ context), so there's no logo flash. The Sidebar/SetupWizard instead get branding via an asyncfetchManifest()call, which isn't usable pre-auth without a flash. This PR mirrors theauthProviderspattern for branding: a newAdminBrandingProvidercontext fed from the sameadminConfigadmin.astroalready reads server-side, threaded throughAdminWrapper/AdminAppintoLoginPage,SignupPage, andInviteAcceptPage(which already hadBrandLogo/LogoLockupcomponents with the right fallback logic — Signup/Invite just weren't passing them a logo/siteName).Fully backward compatible: no branding configured → context defaults to
{}→ stock EmDash mark renders exactly as today.Closes #639 (completes its approved-but-undelivered scope)
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (targeted:packages/admin/tests/components/LoginPage.test.tsx, red→green confirmed against a temporary revert)pnpm formathas been runAI-generated code disclosure