feat(seo): full OpenGraph/Twitter set, collapsible stderr, docs reference (Phase 6) - #197
Conversation
…ence Phase 6 of the book-theme parity plan. Meta tags: app/seo.ts adds, on top of upstream's article tags, what the Sphinx lecture sites emit and upstream leaves out -- og:type, og:site_name, og:url (from a new `site_url` option; myst's `site.domains` never reaches the theme's manifest), a site-level og:image / twitter:image when the page has no thumbnail (`og_logo_url` / `twitter_logo_url`, the book theme's names, so a lecture repo copies its values across), twitter:site (upstream puts it in the root route's meta, which the article route's replaces under Remix v2 semantics, so it never rendered) and og:locale from current_language. Same-key upstream tags are replaced, not duplicated. Collapsible stderr: upstream renders a stderr stream as a plain <pre>; verified, and ported as a native <details> around the stderr `output` node (STDERR_RENDERERS, layered after JUPYTER_RENDERERS) rather than the Sphinx build's post-load DOM surgery, so the fold holds in the server-rendered HTML with no script. Styled in the book theme's amber. Docs: a docs/ reference with one page per book-theme user page (configuration, layout, authors, launch, notebooks, git metadata, code highlighting, text colour schemes, dark mode, RTL and editions, announcements, the stderr fold), each naming its myst.yml keys and what has no counterpart. Tests: `social-meta` asserts the full set on the no-thebe fixture, which gains site_url, twitter and both logo URLs; `stderr-collapsed` asserts the fold is closed by default, stdout stays visible, and it opens. Six unit tests cover the helpers. No baseline moves. Closes #92 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
🎭 Visual regression results
Details
Skipped testsmobile-chrome › theme.spec.ts › QuantEcon theme — visual regression › without JavaScript › drawer-opens-without-javascript |
Matches the autoexpand behaviour #196 now ships. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
It changes SEO meta output on every page and the notebook output render path, and the new docs describe outline behavior that only lands in the separate, unmerged PR #196, so merge ordering and final sign-off warrant human review.
Pull request overview
This PR implements Phase 6 of the book-theme parity plan (#92): the full OpenGraph/Twitter meta-tag set, a collapsible stderr fold on notebook cells, and a docs/ feature reference. It builds on the declared-options path (#173/#194): a new app/seo.ts helper layers og:type, og:site_name, og:url (from a new site_url option), site-level og:image/twitter:image (og_logo_url/twitter_logo_url), twitter:site and og:locale on top of upstream's getMetaTagsForArticle, merged via mergeMeta so same-key upstream tags are replaced rather than duplicated. A STDERR_RENDERERS override wraps the upstream output renderer, folding stderr streams in a native <details> at render time. I verified the SEO merge logic and the OUTPUT_RENDERERS.output/jupyter_data shape against @myst-theme 1.3.0 upstream source, and confirmed the documented defaults (git-metadata max 6, back-to-top 80px, launcher defaults) match the code.
Changes:
- Add
app/seo.ts(origin/locale/tag-merge helpers) and wire it into both routemetafunctions on top of upstream article tags. - Add
STDERR_RENDERERSwrapping the Jupyteroutputrenderer to fold stderr in a<details>, with.qe-stderrstyling; newsite_url/og_logo_url/twitter_logo_urloptions intemplate.yml/types.ts. - Add a
docs/reference set, unit tests (seo.test.mjs), visual tests, and a fixture cell; update README/PLAN/CHANGELOG.
File summaries
| File | Description |
|---|---|
| app/seo.ts | New SEO helper: origin resolution, BCP-47→OG locale, tag set, merge |
| app/routes/$.tsx, _index.tsx | Wire socialMetaTags/mergeMeta into article + index meta |
| app/renderers.tsx | STDERR_RENDERERS wraps upstream output in a <details> fold |
| app/root.tsx | Register STDERR_RENDERERS after JUPYTER_RENDERERS |
| app/types.ts, template.yml | Declare site_url/og_logo_url/twitter_logo_url options |
| styles/quantecon.css | .qe-stderr amber fold styling (light + dark) |
| tests/unit/seo.test.mjs, tests/visual/theme.spec.ts | Unit + visual coverage for the tag set and the fold |
| tests/visual/fixture-no-thebe/* | Fixture cell (stderr+stdout) and site.options for the tests |
| docs/*, README.md, PLAN.md, CHANGELOG.md | Parity docs and record updates |
Review details
- Files reviewed: 28/28 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Copilot review on #197: each Unreleased entry links its PR. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
It changes server-rendered social meta on every page and overrides a core jupyter output renderer, and several docs intentionally describe outline behavior that only lands once the separate PR #196 merges first, so human verification of the merge ordering and site-wide rendering is warranted.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
README.md:242
- This table row states both
og_logo_urlandtwitter_logo_urlare used "when a page has no thumbnail", but that is only accurate forog_logo_url. Inapp/seo.ts,twitterImage = options?.twitter_logo_url || image, sotwitter_logo_urltakes precedence over a page thumbnail fortwitter:image(confirmed bytests/unit/seo.test.mjswhere a/thumb.pngpage image still yieldstwitter:image = https://x/tw.png). A maintainer reading this row may not expect the configured Twitter logo to override page thumbnails. Consider clarifying the two behaviours separately.
- Files reviewed: 28/28 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Brings in #196 (pinned outline). Two add/add conflicts, both kept in full: styles/quantecon.css (the `.qe-outline*` rules from main, then the `.qe-stderr` rules from this branch) and tests/visual/theme.spec.ts (the "On this page outline (#182)" describe from main, then the "Meta/SEO and notebook output polish (#92)" describe from this branch). The no-thebe fixture config, CHANGELOG and PLAN auto-merged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`live_compute` read as a setting that might take a mode value, and a name with "browser" or "pyodide" in it was considered and rejected because the gate wraps whichever backend `project.thebe` configures (lite, binder or server). `enable_live_compute` keeps the `enable_` prefix the option table already uses (`enable_rtl`), so it is visibly a boolean, and keeps "live compute", the term the README, changelog and header control already use. Also documents the option in the docs/ feature reference that landed in main via #197, and drops the "Phase 4" label from the changelog entry: this is not a parity-plan item (mmcky, PR #195 discussion), and Copilot noted that Phase 4 is internationalisation in any case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Closes #92, Phase 6 of the book-theme parity plan and the last build item on the #147 gate besides #182 (PR #196). Three parts, as PLAN.md scopes the phase.
Meta / SEO
Measured against
python-programming.quantecon.org, the Sphinx sites emitog:title,og:type,og:url,og:image,og:description,og:site_nameandtwitter:card,twitter:site,twitter:creator,twitter:title,twitter:description,twitter:image. Upstream'sgetMetaTagsForArticlecovers about half.app/seo.tsadds the rest on top and replaces same-key upstream tags rather than duplicating them:og:typewebsite, as on every Sphinx lecture pageog:site_nameog:urlsite_urloption plus the page path (base URL included)og:image/twitter:imageog_logo_url/twitter_logo_urloptions, the book theme's names so a lecture repo copies its_config.ymlvalues acrosstwitter:sitetwitteroption; upstream emits it only from the root route's meta, which the article route's replaces under Remix v2 semantics, so it had never renderedog:localecurrent_language(Phase 4)Why
site_urland notsite.domains.domainsis the natural myst key, and it is validated and stored, but the site manifest the CLI hands the theme does not carry it (verified on the built fixture'sconfig.json). The option is the Sphinx sites'html_baseurlequivalent;domainsremains a fallback in the helper should the manifest ever include it.Collapsible stderr
PLAN.md asked to verify before porting: upstream renders a stderr stream as a plain
<pre class="jupyter-error">. The Sphinx build'sstderr-warnings.jsfolds it after page load with DOM surgery; hereSTDERR_RENDERERSwraps the stderroutputnode in a native<details>at render, closed by default, with the book theme's amber styling, so the fold holds in the server-rendered HTML with no script and survives re-renders. Stdout and results in the same cell stay visible.Docs
A
docs/reference with one page per book-themedocs/user/*page (configuration, layout, authors, launch, notebooks, git metadata, code highlighting, text colour schemes, dark mode, RTL and editions, announcements, andfeatures/stderr-warnings), each naming itsmyst.ymlkeys and saying plainly what has no counterpart and why (the announcement banner, the alternative code palettes, the collapse of outline sub-entries).Tests
social-metaasserts the full set on the no-thebe fixture, which gainssite_url,twitterand both logo URLs, and checksog:imageandtwitter:cardappear once each.stderr-collapsedasserts the fold is closed by default, stdout stays visible, and it opens on click; the no-thebe notebook gains a cell with stderr and stdout streams. Six unit tests cover origin, locale, the tag set and the merge. Full local suite: 36 passed on the first run plus the three re-run after two test fixes; no baseline moves (all fixture changes are on the snapshot-free no-thebe project).Merge after #196. The two touch different files, but
docs/layout.mdanddocs/dark-mode.mddescribe the pinned, collapsing outline that #196 delivers, so this lands second.🤖 Generated with Claude Code