fix(docs): restore the Home hero mark and guard absolute asset paths - #12539
Conversation
The Dynamo mark above the Home page heading rendered as an empty tile. LandingStyles.tsx sourced it from `/dynamo/assets/img/dynamo-logo.svg`, a path Fern does not serve: it rewrites asset references only in MDX and docs.yml, never inside a `<style>` string, so the `url()` reached the browser verbatim and 404'd. #12373 introduced it by swapping a working absolute URL for a repo-relative one. Render the mark as an `<img>` from the page MDX so Fern's asset pipeline rewrites the path, and pull it back over the heading with CSS; the header top padding reserves its place. Tile chrome is unchanged in both themes. Add check_asset_paths.py, wired as the `check-asset-paths` pre-commit hook, so a site-absolute asset path cannot reach the site again. It rejects `/.../assets/...` while leaving the two forms Fern does rewrite alone: `../../assets/...` in MDX and `./assets/...` in docs.yml. Also correct three stale claims in the LandingStyles.tsx header comment: the landing rules do not live in main.css (#12330 moved them out, so there is no fallback baseline), CustomFooter's SITE_CSS does reach these pages, and the placement filenames predate the docs restructure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Dan Gil <dagil@nvidia.com>
|
/ok to test 25f23f6 |
WalkthroughThe PR adds a Fern asset-path checker and pre-commit hook. It updates landing styles, MDX placement paths, the home-page logo, and the terminal demo asset reference. ChangesFern asset path validation
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.pre-commit-config.yaml:
- Line 93: Update the pre-commit hook’s `files` regex to match both `.md` and
`.mdx` files under `docs/fern/pages`, changing the page suffix pattern to
`\.mdx?` while preserving the existing component, asset, and script matches.
In `@docs/fern/components/LandingStyles.tsx`:
- Around line 109-138: Update the max-width: 640px responsive rules to target
.dynamo-welcome__mark instead of header::before, sizing and positioning the 92px
mark appropriately for mobile. Increase the mobile header padding to reserve the
mark’s height and prevent the heading from overlapping it, while preserving the
existing desktop behavior.
🪄 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: 1a6e56e3-a0be-4c18-8e83-7d8d29e7ac1c
📒 Files selected for processing (5)
.pre-commit-config.yamldocs/fern/components/LandingStyles.tsxdocs/fern/components/TerminalDemo.tsxdocs/fern/pages/home/index.mdxdocs/fern/scripts/check_asset_paths.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25f23f60f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The mobile breakpoint still reset the hero header padding to 2.75rem and sized the `header::before` this PR removed, so the 92px mark stayed put and the heading ran over it -- 89px of overlap measured at 375px. Point those rules at `.dynamo-welcome__mark` and reserve its height in the padding. Widen the guard: `translations/**` carries the zh-CN pages, which publish through the locale in docs.yml and 404 the same way, but matched neither the hook filter nor DEFAULT_GLOBS. Add the backtick as an opening delimiter too -- these files hold CSS in template literals, so a bare assignment was reachable and unmatched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Dan Gil <dagil@nvidia.com>
The floating notification rail linked to Slack and the events calendar, both of which the hero CTA buttons already cover, so it repeated two links a few hundred pixels apart. Remove CommunityRail and the now-dead SlackIcon, CalendarAppIcon and UPCOMING_EVENTS import with it, plus the .dynamo-welcome__community, __notification and __calendar-app rules across the base styles and three breakpoints. SLACK_URL and CALENDAR_URL stay -- the CTAs use them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Dan Gil <dagil@nvidia.com>
|
/ok to test 91b3f8b |
docs.yml carries the logo, favicon and font path entries. Fern rewrites the ./assets/... form there, but a site-absolute one ships verbatim and 404s, and the hook skipped the file entirely. Same gap as translations/. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Dan Gil <dagil@nvidia.com>
|
/ok to test a409877 |
docs.yml writes its logo, favicon and font paths as bare YAML scalars, so a pattern anchored on a quote, backtick or `(` read the file and found nothing. Adding docs.yml to the hook scope was therefore inert. Accept `key: /...` and `- /...` as openers too. The earlier verification passed only because it quoted the injected value, which is not the form the file uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Dan Gil <dagil@nvidia.com>
|
/ok to test 59b6142 |
The pattern's holes were found three times in review -- backtick, docs.yml scope, then unquoted YAML scalars -- and each fix was checked by hand and thrown away. Pin all twelve cases, positive and negative, and wire them as check-asset-paths-selftest, mirroring check_published_styles.py --test. Drop the script from the main hook's files: it was inert there, since the scanner skips its own docstring. The self-test hook covers that trigger now. Collapse the two overlapping opener comments left by the earlier patches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Dan Gil <dagil@nvidia.com>
|
/ok to test 6e71ea2 |
Scope lives in two files: DEFAULT_GLOBS for a bare run, the hook's files: for the commit path. Both had to be widened for translations/ and again for docs.yml, and each time only one was obvious. A files: narrower than the globs is the silent case -- changed files skip the hook and only a full manual run catches them. --test now derives a flat and a nested sample from every glob and asserts the hook pattern matches. Verified it fails closed: dropping translations from files: turns four cases red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Dan Gil <dagil@nvidia.com>
|
/ok to test 914a450 |
The hook's files: enumerated the same file types DEFAULT_GLOBS already listed, so every scope change meant editing both. The previous commit answered that by generating cases to prove the two agreed, which polished the duplication instead of removing it. Make files: a coarse ^docs/fern/ trigger with pass_filenames: false and let DEFAULT_GLOBS decide what to scan. Drift is now impossible rather than detected, and the drift machinery goes with it: 209 lines down to 163. Coverage widens as a side effect. index.yml sits under docs/fern but matched no branch of the old pattern, so editing it triggered nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Dan Gil <dagil@nvidia.com>
|
/ok to test ad0ed4a |
Restoring the hero mark also dropped CommunityRail and its 23 style rules, which took the Slack and calendar links off the right of the Home page. Two secondary buttons had been added beside Get started to stand in for them, so the same two destinations appeared twice in the middle of the hero and nowhere on the right. The rail comes back and the stand-ins come out. Get started keeps the actions row to itself. Signed-off-by: Dan Gil <dagil@nvidia.com>
|
/ok to test 234f63e |
Summary
The Dynamo mark above the Home page heading renders as an empty tile in production.
LandingStyles.tsxsourced it from/dynamo/assets/img/dynamo-logo.svg, a path Fern does not serve: Fern rewrites asset references only in MDX anddocs.yml, never inside a<style>string, so theurl()reached the browser verbatim and 404'd.Introduced in #12373, which swapped a working absolute URL for a repo-relative one.
Fix. Render the mark as an
<img>from the page MDX so Fern's asset pipeline rewrites the path, then pull it back over the heading with CSS; the header's top padding reserves its place. Tile chrome (92px box, 24px radius, green border, shadows, light/dark backgrounds) is unchanged.Guard.
check_asset_paths.py, wired as thecheck-asset-pathspre-commit hook, rejects site-absolute/.../assets/...references while leaving the two forms Fern does rewrite alone:../../assets/...in MDX and./assets/...indocs.yml.Comment corrections. Three stale claims in the
LandingStyles.tsxheader comment: the landing rules do not live inmain.css(#12330 moved them out, so there is no fallback baseline),CustomFooter'sSITE_CSSdoes reach these pages, and the placement filenames predate the docs restructure.Validation
#0c0d0btile, green mark, correct border and shadowcheck_asset_paths.py, default scopemainurl("/dynamo/assets/...")inLandingStyles.tsxTerminalDemo.tsxpathpre-commit runon the change set<img className=... src="../../../assets/img/...">resolves to hashedfdr-prod-docs-files-public.s3...URLsLocal
fern docs devwas not run (no Fern CLI in this environment). The rendering evidence above is from the published site with the final CSS injected, not a local build.Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores