[codex] Fix testenv site page header rendering#186
Conversation
|
Warning Review limit reached
More reviews will be available in 53 minutes and 40 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR extracts header-hiding logic into a reusable helper function, refactors file-level header hiding to always persist changes, and expands integration test coverage for SEO directives and header hiding. The script also includes minor formatting improvements to error messages, parameter defaults, and build configuration. ChangesSEO Directives and Header Hiding
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 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🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
This PR adjusts the test environment build pipeline so that standalone root pages (/app.html, /generator.html, etc.) remain headerless for focused previews, while the copied full-site pages under /site/*.html retain the normal navbar. It also adds regression coverage around the two rendering paths.
Changes:
- Stop injecting the “hide header” style into the copied
/site/*.htmlpages while keeping it for the standalone root pages. - Extract a reusable
hideTopHeaderInBuiltHtmlhelper (used by the file-based mutator and tests). - Extend the testenv SEO integration tests to cover both “SEO rewrite” and “header hiding” behaviors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/integration/create-testenv-seo.test.js | Adds regression tests for header hiding vs. SEO rewriting behavior. |
| scripts/create-testenv.mjs | Refactors header-hiding into an HTML helper and stops hiding headers on /site/*.html pages. |
Greptile SummaryThis PR fixes the test-environment builder so that header-hiding is applied only to the root standalone pages (
Confidence Score: 5/5Safe to merge — targeted removal of three misplaced header-hiding calls with a straightforward pure-function extraction and matching test coverage. Both changed files are build/test tooling only, with no effect on production runtime behaviour. The bug fix is simple and correct: the three hideTopHeaderInBuiltPage calls incorrectly targeting fullSiteDir are gone, while the equivalent calls targeting outputDir remain. New tests exercise both the SEO-rewrite path and the header-hiding path. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[main] --> B[copyWebBuildIntoDirectory tempWebDir → outputDir]
B --> C[hideTopHeaderInBuiltPage outputDir/app.html\noutputDir/generator.html\noutputDir/combinatorial.html]
C --> D[docusaurus build → fullSiteDir]
D --> E[copyWebBuildIntoDirectory tempWebDir → fullSiteDir]
E --> F[rm tempWebDir]
subgraph hideTopHeaderInBuiltHtml
G{html includes data-testenv-hide-header?}
G -- yes --> H[return html unchanged]
G -- no --> I[inject TESTENV_HIDE_HEADER_STYLE before end of head]
end
C --> G
Reviews (2): Last reviewed commit: "Potential fix for pull request finding" | Re-trigger Greptile |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
What changed
/site/*.htmlpages in the full-site test environmentWhy this changed
The test environment builder was hiding the top header on both the standalone root pages and the copied
/site/pages. That removed the expected navbar from/site/app.html,/site/generator.html, and related pages even though those pages should behave like the published site and keep the test-environment badge.Impact
Reviewers can now use the
/site/*.htmlpages with normal navigation intact, while the standalone experiment pages still stay minimal.Validation
pnpm test -- --runTestsByPath tests/integration/create-testenv-seo.test.jspnpm run verify:uipnpm run verify:localSummary by CodeRabbit
Tests
Refactor