fix: footer overlap in sidebar - #2260
Conversation
|
Welcome to AICR, @Pranav-IIITM! Thanks for your first pull request. Before review, please ensure:
A maintainer will review this soon. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review. 📝 WalkthroughWalkthroughAdds a theme-aware background, relative positioning, and stacking order to Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: 🔵 Low · up to The change may not fix the sidebar overlap if the documentation build omits the new stylesheet under the active theme. The PR is otherwise mergeable, but the owner should confirm that custom.css is present in the generated docs. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-fix-docs-footer-overlap.docs.buildwithfern.com/aicr |
|
We are investigating if our fern theme needs a fix. |
njhensley
left a comment
There was a problem hiding this comment.
🔍 Multi-persona review — footer overlap fix
Method: 3 independent persona reviewers (CSS/Fern-correctness · CI-DX/verification · Docs-UX/robustness) → adversarial senior meta-reviewer re-derived every finding from the resolved code, grounded against the official Fern docs (the css key + theming model). All 7 findings confirmed; none refuted.
Tier legend: 🔴 Blocker · 🟠 Major · 🟡 Minor · 🔵 Nitpick
Overall assessment
The core fix is correct and well-scoped. Painting an opaque, theme-adaptive var(--background) onto a footer that sidebar text was bleeding through is the right remedy, and a CSS override is the correct layer — Fern's colors: config can't add a background/z-index to a specific element. It will pass CI (no CSS linter; fern check validates the css: key; the publish-time yq surgery is scoped to .products[0].versions and doesn't touch it).
The one substantive note is that ~13 of the 20 lines are dead or duplicate: because Fern's --background variable already adapts to light/dark and Fern toggles dark mode via the .dark class (not prefers-color-scheme, not data-theme), the entire dark-mode section is inert or redundant and the file collapses to a single rule. Nothing here is a visual bug — the unreachable hex fallbacks can't misfire — but the redundancy misleads future maintainers. See inline comments for the specifics.
Confirmed non-issues (examined, not problems)
css: custom.cssis a valid Ferndocs.ymlkey and the file exists —fern checkvalidates it.publish-fern-docs.ymlyq surgery won't clobber the top-levelcss:key.- The
.dark #fern-footerdescendant combinator is correct — lines 13-20 are redundant, not broken. - No doc update required — pure cosmetic CSS, no enumerated user-visible behavior.
Process notes (not code defects)
- Branch is behind
main; the repo enforces up-to-date branches, so it needsgit fetch origin main && git rebase origin/mainbefore merge. fern/**non-.mdedits setcode=truein the merge gate, so the full qualification suite runs on this cosmetic PR (expected to pass).
Summary
| 🔴 Blocker | 🟠 Major | 🟡 Minor | 🔵 Nitpick | Recommendation |
|---|---|---|---|---|
| 0 | 0 | 3 | 4 | Approve with comments |
Highest-leverage single action: collapse the file to the one rule above and confirm it on the Fern preview build — that resolves the top three findings at once.
Automated multi-persona review; every finding was independently re-derived by a senior meta-reviewer before inclusion.
yuanchen8911
left a comment
There was a problem hiding this comment.
The deployed preview still reproduces the reported footer overlap because the new stylesheet is discarded during global-theme stitching. Please address the inline finding before merge.
|
Thanks for the detailed reviews, @njhensley and @yuanchen8911! I've pushed a new commit to address the initial feedback:
Before I make any further changes, what is the recommended way to inject this CSS so it composes with (or bypasses) the Let me know the preferred approach and I'll get it updated! |
926bf19 to
0eb5aea
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
…rlap Signed-off-by: Pranav-IIITM <jogdandpranav2007@gmail.com>
4314ef0 to
5898099
Compare
Summary
Added a custom CSS file to the Fern documentation configuration to fix a visual bug where the transparent footer overlapped with the left navigation sidebar.
Motivation / Context
When scrolling down the documentation pages, the left navigation sidebar text was bleeding through the transparent footer, making both unreadable. Applying a solid background with a proper z-index fixes this visual overlap in both light and dark modes.
Fixes: #2258
Related: NVIDIA/nvcf#818
Type of Change
Component(s) Affected
docs/,examples/)fern/Implementation Notes
fern/custom.csswith CSS overrides ensuring a solid background forfooter.bd-footerand#fern-footerin both light and dark mode.custom.cssglobally withinfern/docs.yml.Testing