Align docs site custom CSS with GitHub Docs color system while preserving landing page purples#39997
Align docs site custom CSS with GitHub Docs color system while preserving landing page purples#39997Copilot wants to merge 3 commits into
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot show screenshots |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added in Homepage (light) Homepage (dark) Quick Start (light) Quick Start (dark) |
|
@copilot keep purples for the landing page |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed in |
There was a problem hiding this comment.
Pull request overview
This PR updates the Starlight docs site styling to align the docs pages with GitHub Docs’ blue-based color system, while introducing landing-page-specific overrides intended to preserve the previous purple branding treatment on the home/hero page.
Changes:
- Replaced global accent tokens (light + dark) from purple to GitHub Docs-style blues and propagated those updates across key UI surfaces (backgrounds, header chrome, links, badges, pagination, focus rings).
- Updated Mermaid diagram styling and other content affordances to match the new blue accent palette.
- Added
body.landing-pageCSS overrides to restore the prior purple palette and background treatments on the landing page.
Show a summary per file
| File | Description |
|---|---|
| docs/src/styles/custom.css | Replaces site-wide accent palette with GitHub Docs blues and adds landing-page-specific purple overrides. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/5 changed files
- Comments generated: 2
| body.landing-page .sl-markdown-content a:hover, | ||
| body.landing-page nav a:hover { | ||
| color: #d896ff; | ||
| border-bottom-color: rgba(216, 150, 255, 0.3); | ||
| } |
| :root[data-theme='light'] body.landing-page a, | ||
| :root[data-theme='light'] body.landing-page nav a:hover, | ||
| :root[data-theme='light'] body.landing-page nav a[aria-current="page"], | ||
| :root[data-theme='light'] body.landing-page .sl-markdown-content a:hover { | ||
| color: #5b3da3; | ||
| } |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. The PR only changes docs/src/styles/custom.css and adds image files under docs/public/images/pr-review/. Test Quality Sentinel skipped. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #39997 does not have the 'implementation' label (has_implementation_label=false) and has 0 new lines of code in business logic directories (well below the 100-line threshold). The changes are docs/CSS theme updates, not core business logic. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /zoom-out and /improve-codebase-architecture — commenting on maintainability issues and one subtle correctness regression.
📋 Key Themes & Highlights
Key Themes
- Specificity regression: Light landing page
a:hoverno longer changes color (the broad:root[data-theme='light'] body.landing-page arule outranks the basea:hoverrule due to higher specificity) - DRY concern: Purple RGB value
139, 92, 246is hardcoded in 6+rgba()calls; an--landing-accent-rgbcustom property would centralise this - Cross-file coupling: The
body.landing-pageCSS block depends on a JS pathname-detection script inCustomHead.astrowith no in-CSS pointer to that dependency - Binary assets: Four JPEG screenshots appear to be PR-review evidence only and should not be committed to the repository
Positive Highlights
- ✅
var(--sl-color-purple)→var(--sl-color-accent)is a clean semantic improvement that makes the video caption border respond correctly to both themes automatically - ✅ The
body.landing-pageCSS scoping pattern is a sound approach to split-section theming without Starlight plugins - ✅ CSS custom property overrides on
body.landing-pagecorrectly cascade above:rootvalues becausebodyis a descendant ofhtml, so the purple accent tokens flow through cleanly - ✅ Light/dark theme switching handled correctly via
:root[data-theme='light'] body.landing-pageselectors
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer
Comments that could not be inline-anchored
docs/src/styles/custom.css:1619
[/improve-codebase-architecture] Light landing page a:hover has no color change — link color stays #5b3da3 on hover, silently dropping the #6b46c1 hover contrast from the original palette.
Because :root[data-theme='light'] body.landing-page a { color: #5b3da3 } (specificity 0,2,3) outranks the base :root[data-theme='light'] a:hover { color: #0550ae } (0,2,2), hover never fires a color shift on the light landing page.
<details>
<summary>💡 Suggested fix</summary>
Add an explicit…
docs/src/styles/custom.css:1625
[/improve-codebase-architecture] The purple RGB channel 139, 92, 246 is hardcoded in 6+ rgba() calls across this block, creating a single-point-of-failure for any future purple brand tweak.
<details>
<summary>💡 Suggested refactor</summary>
Define a CSS custom property on body.landing-page and reference it via color-mix or an --*-rgb pattern:
body.landing-page {
/* existing token overrides */
--landing-accent-rgb: 139, 92, 246;
}
/* then in button overrides */
body.…
</details>
<details><summary>docs/src/styles/custom.css:1564</summary>
**[/zoom-out]** The `body.landing-page` class is assigned by pathname-detection JS in `docs/src/components/CustomHead.astro` (checking for `/gh-aw/`). If the docs base URL ever changes, all these overrides silently become dead code with no CSS-level indication of the dependency.
Consider adding a brief comment here pointing to that coupling:
```css
/*
* Landing-page class is applied by the inline script in CustomHead.astro.
* Search for `classList.add(landing-page)` if the base path changes…
</details>
<details><summary>docs/src/styles/custom.css:1600</summary>
**[/improve-codebase-architecture]** `a:hover` and `nav a[aria-current="page"]` share a color today, but they are semantically unrelated states. Grouping them makes future independent color changes error-prone.
<details>
<summary>💡 Suggested split</summary>
```css
/* hover: slightly lighter than rest state */
body.landing-page a:hover {
color: #e5c7ff;
}
/* active nav item */
body.landing-page nav a[aria-current="page"] {
color: #e5c7ff;
}Verbose but safe — changing one no longer …
docs/public/images/pr-review/docs-home-dark.jpg:1
[/zoom-out] These four JPEG screenshots appear to be visual evidence for PR review, not referenced in any docs page content. Committing them permanently adds binary blob weight to git history and ships them as site assets to every visitor.
Consider uploading them directly as images in the PR description (drag-and-drop into the text box) and removing this commit of binary files. If they are intended as part of the docs site, the path should reflect that intent (e.g. `docs/public/images/scre…
docs/src/styles/custom.css:1580
[/improve-codebase-architecture] display: block here is already guaranteed by body:has(.hero)::before { display: block } on line 416 (the landing page always has a .hero element). The redundant declaration is harmless, but it slightly obscures the real control flow.
<details>
<summary>💡 Options</summary>
- Remove
display: blockfrom this rule; it only setsbackground, which is its real purpose. - Or keep it and add a comment: `/* also covered by body:has(.hero)::before,…
There was a problem hiding this comment.
The blue-to-purple split is sensible and the specificity math for the landing page overrides is correct. Two issues block merge.
### Blocking issues (2)
1. Missing focus-ring override (high) — :focus-visible was hard-coded to blue (#58a6ff / #0969da) but the new body.landing-page block has no :focus-visible override. Keyboard users on the landing page will see blue focus rings on purple buttons and links. See inline comment at line 1345.
2. Flash of incorrect styles from JS-dependent class (medium) — body.landing-page is applied after first paint via DOMContentLoaded, so the landing page briefly renders in blue. The CSS already uses body:has(.hero) for the glow effect — the same zero-JS pattern should be used for all the landing-page overrides. See inline comment at line 1564.
### Already-flagged in existing review
- Line 1608:
body.landing-page .sl-markdown-content a:hoverwins overbody.landing-page a:hoverdue to specificity, making the lighter#e5c7ffhover color unreachable for markdown links. - Line 1615: Light-theme landing page hover color equals the base link color — no visible hover feedback.
🔎 Code quality review by PR Code Quality Reviewer
| /* Focus Visible State */ | ||
| :focus-visible { | ||
| outline: 2px solid #d896ff; | ||
| outline: 2px solid #58a6ff; |
There was a problem hiding this comment.
Missing body.landing-page :focus-visible override: keyboard focus rings on the landing page will render blue despite the purple theme intent.
💡 Suggested fix
The :focus-visible outline was changed from purple to blue here, but the body.landing-page block (added further below) doesn't include a corresponding override. Any keyboard-navigable element on the landing page — the CTA button, nav links — will show a #58a6ff blue focus ring against a purple-themed page. This is especially jarring on the primary CTA whose background is the restored purple.
Both rules use hardcoded hex values (not var(--sl-color-accent)), so the CSS custom property overrides on body.landing-page do not automatically fix this — an explicit selector is required:
body.landing-page :focus-visible {
outline-color: #d896ff;
}
:root[data-theme='light'] body.landing-page :focus-visible {
outline-color: #6b46c1;
}| } | ||
|
|
||
| /* Keep the landing page on the previous purple palette while docs pages use the GitHub Docs blues. */ | ||
| body.landing-page { |
There was a problem hiding this comment.
JS-dependent class risks a flash of incorrect styles (FOCS): body.landing-page is added by an inline script that runs after first paint, so the landing page renders in blue before switching to purple.
💡 Why this matters and how to fix it
The CustomHead.astro inline script checks window.location.pathname and calls document.body.classList.add('landing-page'). Because document.body is null when <head> scripts execute, the code falls back to DOMContentLoaded — meaning the class is applied after the browser has already painted the first frame. Users will see the blue docs theme flash briefly before the purple overrides land.
The file already solves this problem at line 416:
body:has(.hero)::before {
display: block;
}That selector is CSS-native and zero-JS — the <body> carries .hero in the server-rendered HTML. Replacing body.landing-page with body:has(.hero) throughout this block eliminates the FOCS entirely:
body:has(.hero) {
--sl-color-accent-low: #6639ba;
--sl-color-accent: #d896ff;
/* ... */
background: linear-gradient(...) !important;
}
:root[data-theme='light'] body:has(.hero) { ... }
body:has(.hero)::before { background: ... }
/* etc. */If body.landing-page must stay for other reasons, inject the class synchronously via a <script> placed immediately before </body>, or add landing-page server-side in the Astro component when the route is the index page.
This updates the Starlight docs theme so the documentation pages read closer to GitHub Docs instead of the previous purple-heavy palette, while preserving the purple treatment on the landing page. The change is focused on shared color tokens and the major UI surfaces that define the docs experience.
Theme tokens
Core surfaces
Interactive states
Content affordances