Continue white-label branding support for isolated builds#3259
Continue white-label branding support for isolated builds#3259
Conversation
- Gate <navigation-search> on branding (requires Elastic backend) - Auto-discover favicon/apple-touch-icon from docs source dir when branding is set; explicit overrides via branding.favicon / branding.apple-touch-icon - Suppress Elastic boilerplate in llms.txt for branded builds - Header: fix logo size (32px), prevent HTMX on home link, default to gradient bg when no header-bg is configured - Fix GitHubRepository returning hardcoded elastic/docs-builder on fallback; now returns null so edit/GitHub links are hidden rather than wrong; GITHUB_REPOSITORY env var fallback still works on CI Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe pull request extends branding configuration to auto-discover favicon and Apple Touch icon files when not explicitly provided, and propagates branding information through the rendering pipeline. Changes include: adding Favicon and AppleTouchIcon properties to BrandingConfiguration with auto-discovery via DiscoverBrandingFile; updating Header and favicon view templates to conditionally render branding assets; passing branding context through ViewModels (NavigationViewModel, GlobalLayoutViewModel); and updating markdown exporters to accept a branded flag controlling content generation. GitHubRepository is changed to nullable, and null-checks are added where applicable. Sequence DiagramsequenceDiagram
participant Builder as Configuration Builder
participant Context as Build Context
participant NavVM as Navigation ViewModel
participant LayoutVM as Layout ViewModel
participant Header as Header Template
participant Favicon as Favicon Template
Builder->>Builder: Validate branding config
Builder->>Builder: Auto-discover favicon/apple-touch-icon<br/>(DiscoverBrandingFile)
Builder->>Context: Set Configuration.Branding
Context->>NavVM: Create with Branding property
Context->>LayoutVM: Create with Branding properties<br/>(Favicon/AppleTouchIcon static paths)
NavVM->>Header: Provide Branding context
Header->>Header: Conditionally compute<br/>header CSS based on branding
Header->>Header: Render branded or default logo
LayoutVM->>Favicon: Provide branding favicon paths
Favicon->>Favicon: Check if Branding exists
alt Branding present
Favicon->>Favicon: Compute MIME type from extension
Favicon->>Favicon: Render branded favicon/apple-touch-icon
else No branding
Favicon->>Favicon: Render default favicon
end
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@src/Elastic.Documentation.Configuration/Builder/ConfigurationFile.cs`:
- Around line 306-309: ValidateBrandingImage currently returns null on
validation failure and the code uses the null-coalescing operator to call
DiscoverBrandingFile, causing a broken explicit path to be overwritten; change
the logic for branding.Favicon and branding.AppleTouchIcon so
DiscoverBrandingFile is only invoked when no explicit override was provided
(original value null/empty) — e.g., capture the original value, if
string.IsNullOrEmpty(original) then set from DiscoverBrandingFile, otherwise set
to the result of ValidateBrandingImage and do not fall back on discovery when
validation fails; mirror this behavior with the other fields (Icon, OgImage) for
consistency.
In `@src/Elastic.Documentation.Site/Layout/_Favicon.cshtml`:
- Around line 13-18: The current switch that sets mimeType (based on
System.IO.Path.GetExtension(fav)) uses a catch-all that maps ".jpg" to the
non-standard "image/jpg"; update the switch on mimeType to enumerate common
image extensions explicitly—keep ".ico" => null and ".svg" => "image/svg+xml",
add ".jpg" => "image/jpeg", ".jpeg" => "image/jpeg", ".png" => "image/png" (and
any other expected extensions you need), and only use a generic fallback if you
explicitly want one; modify the switch expression around the mimeType variable
so these explicit cases replace the var e => ... arm.
In `@src/Elastic.Documentation/GitCheckoutInformation.cs`:
- Line 49: GitHubRepository currently calls ExtractGitHubOrgRepo(Remote) but
Remote can be the placeholder string "elastic/docs-builder-unknown" which parses
as a valid org/repo; update the logic so that GitHubRepository returns null when
Remote equals the placeholder (or modify ExtractGitHubOrgRepo to detect and
return null for that sentinel) so downstream consumers don't get a fake repo;
reference the GitHubRepository property and the ExtractGitHubOrgRepo helper and
ensure any fallback value is treated as "no repo" (null) rather than a parsable
repo.
In `@src/tooling/docs-builder/Http/ReloadableGeneratorState.cs`:
- Line 102: The call to AddLlmMarkdownExport passes a positional boolean; update
the call to use a named boolean argument (e.g., branded:
_context.Configuration.Branding is not null) so the intent is explicit — locate
the call to markdownExporters.AddLlmMarkdownExport(...) in
ReloadableGeneratorState (the AddLlmMarkdownExport method call) and change the
argument to a named parameter referencing _context.Configuration.Branding is not
null.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5fe717f7-7b0a-41fa-8c40-3202dd740d69
📒 Files selected for processing (16)
src/Elastic.Documentation.Configuration/Builder/ConfigurationFile.cssrc/Elastic.Documentation.Configuration/Toc/DocumentationSetFile.cssrc/Elastic.Documentation.Site/Assets/web-components/Header/Header.tsxsrc/Elastic.Documentation.Site/Layout/_Favicon.cshtmlsrc/Elastic.Documentation.Site/Layout/_Head.cshtmlsrc/Elastic.Documentation.Site/Navigation/IsolatedBuildNavigationHtmlWriter.cssrc/Elastic.Documentation.Site/Navigation/NavigationViewModel.cssrc/Elastic.Documentation.Site/Navigation/_TocTree.cshtmlsrc/Elastic.Documentation.Site/_ViewModels.cssrc/Elastic.Documentation/GitCheckoutInformation.cssrc/Elastic.Markdown/DocumentationGenerator.cssrc/Elastic.Markdown/Exporters/ExporterExtensions.cssrc/Elastic.Markdown/Exporters/LlmMarkdownExporter.cssrc/Elastic.Markdown/HtmlWriter.cssrc/services/Elastic.Documentation.Isolated/IsolatedBuildService.cssrc/tooling/docs-builder/Http/ReloadableGeneratorState.cs
Mpdreamz
added
fix
and removed
whitelabel continuation
- ConfigurationFile: only auto-discover favicon/apple-touch-icon when no explicit override was given; a failing explicit path no longer silently falls back to a discovered file - _Favicon.cshtml: enumerate MIME types explicitly so .jpg maps to image/jpeg (was non-standard image/jpg); .gif and .webp added too - GitCheckoutInformation: treat sentinel remote "elastic/docs-builder-unknown" as null in GitHubRepository so it doesn't parse as a valid org/repo path - ReloadableGeneratorState: use named argument branded: for AddLlmMarkdownExport call Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
2 participants
Summary
Continuation of #3159. Fixes remaining Elastic-specific content leaking into branded builds.
Navigation search gated on branding
<navigation-search>(requires Elastic AI backend) now hidden for branded builds alongside Codex; addedBrandingproperty toNavigationViewModelFavicon auto-discovery
branding:is set, docs-builder looks forfavicon.ico/png/svgandapple-touch-icon.pngin the docs source directory automatically — no YAML config neededbranding.favicon/branding.apple-touch-icon_Favicon.cshtmlconverted to typedRazorSlice<GlobalLayoutViewModel>to render custom or Elastic defaults conditionallyllms.txt branding
LlmMarkdownExportertakes abrandedflag, passed throughExporterExtensionsandReloadableGeneratorStateHeader fixes (observed on cli-schema.pages.dev)
<img>size increased from 24 px → 32 pxcontainerRefdetached from branded logo spans)header-bgis configured (was black)Edit this page / GitHub link fix
GitHubRepositorynow returnsnullinstead of the hardcoded"elastic/docs-builder"when the remote URL can't be resolvededitUrlandgitHubDocsUrlguards updated tois not nullchecks — links are hidden rather than pointing to the wrong repoGITHUB_REPOSITORYenv var fallback still resolvescli-schema/cli-schemacorrectlyTest plan
<navigation-search>, custom favicon in tab, gradient header background, no Elastic boilerplate inllms.txtGITHUB_REPOSITORYenv var) and correct on CI./build.sh unit-testpasses🤖 Generated with Claude Code