Skip to content

theme(fix[spa-nav]): fragment scroll + copy-button recreation#20

Merged
tony merged 5 commits into
mainfrom
spa-nav-fragment-scroll-fix
Apr 19, 2026
Merged

theme(fix[spa-nav]): fragment scroll + copy-button recreation#20
tony merged 5 commits into
mainfrom
spa-nav-fragment-scroll-fix

Conversation

@tony
Copy link
Copy Markdown
Member

@tony tony commented Apr 17, 2026

Fixes #19 (fragment scroll). Also folds in two related SPA-nav defects surfaced while testing the scroll fix end-to-end.

Bugs fixed

1. Cross-page fragment links don't scroll to anchor

document.querySelector(hash) mis-parses dotted IDs. Python autodoc anchors like #libtmux_mcp.models.SearchPanesResult read as id libtmux_mcp AND class models AND class SearchPanesResult — never match. Every py:class / py:meth / py:attr cross-ref silently failed to scroll.

Scroll gated behind !isPop. The popstate listener calls navigate(..., true), so browser back/forward to a fragment URL never scrolled. gp-sphinx's own fetch+swap bypasses browser-native scroll restoration.

2. Copy buttons disappear after SPA swap from a page with no .copybtn

addCopyButtons cloned an existing .copybtn as its template. Landing pages and prose-only indexes have no code blocks, so copyBtnTemplate stayed null forever. SPA-navigating to any code-block page from there rendered without copy buttons.

3. Copy buttons only re-applied to div.highlight pre

addCopyButtons hard-coded that selector, ignoring whatever the project configured via copybutton_selector (e.g. prompt admonitions). Projects using custom selectors lost those copy affordances on every SPA swap.

Fix

packages/sphinx-gp-theme/src/sphinx_gp_theme/theme/static/js/spa-nav.js:

  • Scroll: querySelector(hash)getElementById(decodeURIComponent(hash.slice(1))); hash branch moved out of !isPop guard so popstate with fragment scrolls.
  • Copy buttons: inline HTML template matching sphinx-copybutton's output verbatim (same classes, same SVG). Initial page no longer needs any .copybtn for SPA to work.
  • Copy buttons: iterate window.GP_SPHINX_COPYBUTTON_SELECTOR || "div.highlight pre" so project-configured selectors are respected.

packages/gp-sphinx/src/gp_sphinx/config.py:

  • New _inject_copybutton_bridge on html-page-context. Emits a small inline <script>window.GP_SPHINX_COPYBUTTON_SELECTOR=…;</script> from the project's copybutton_selector config. Gated on sphinx_copybutton being loaded.

Verification

Tested end-to-end against libtmux-mcp docs at localhost:8024 via Playwright MCP. libtmux-mcp configures copybutton_selector = "div.highlight pre, div.admonition.prompt > p:last-child" — it's a good test bed since it exercises both the fragment-scroll path and the extended-selector path.

Scenario Result
SPA click → URL with #libtmux_mcp.models.SearchPanesResult scrollY=9268, at anchor
Browser forward (popstate) → same fragment URL scrollY=9268, at anchor
SPA click → fragment-less URL scrollY=0, scrolled to top
Landing / (no code blocks) → SPA-nav to /installation/ 11/11 <pre> get copy buttons
Landing / → SPA-nav to /recipes/ (prompts only) 8/8 prompt admonitions get copy buttons
Inline-generated button SVG class icon-tabler-copy (matches sphinx-copybutton)

Obsoletes libtmux-mcp's docs/_static/js/spa-copybutton-reinit.js shim, which was a project-local workaround for Bugs 2 and 3. That shim is being removed in a follow-up commit in libtmux-mcp.

Test plan

  • uv run ruff format --check .
  • uv run ruff check .
  • uv run mypy
  • uv run pytest — all tests pass
  • Manual: Playwright verification across all six scroll + copybutton scenarios above
  • Reviewer sanity: open any gp-sphinx Python docs site whose landing page has no code blocks; SPA-nav to a code-block page and a prompt-admonition page; confirm copy buttons appear

Out of scope

  • copybutton_image_svg config override — we hard-code sphinx-copybutton's default SVG. Projects that override the icon get the default SVG on SPA-swapped buttons until the initial sphinx-copybutton run repaints (which it won't, since it only runs on DOMContentLoaded). Acceptable for now.
  • i18n tooltip — we hard-code "Copy" instead of sphinx-copybutton's {{ messages[locale]['copy'] }}. Non-English docs get English tooltips on SPA-swapped buttons. Acceptable for now; can be bridged later.
  • No JS test harness in this repo; fix lacks a regression test. Adding one is a worthwhile follow-up but intentionally deferred.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 17, 2026

Codecov Report

❌ Patch coverage is 37.50000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.31%. Comparing base (f702425) to head (8361afa).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
packages/gp-sphinx/src/gp_sphinx/config.py 37.50% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #20      +/-   ##
==========================================
- Coverage   90.34%   90.31%   -0.04%     
==========================================
  Files         147      147              
  Lines       12597    12605       +8     
==========================================
+ Hits        11381    11384       +3     
- Misses       1216     1221       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tony tony changed the title theme(fix[spa-nav]): scroll to fragment on dotted ids and popstate theme(fix[spa-nav]): fragment scroll + copy-button recreation Apr 17, 2026
tony added 2 commits April 19, 2026 03:58
…tate fires

Two bugs in the cross-page fragment scroll path:

1. document.querySelector(hash) parses the hash as a CSS selector, so
   anchors like #libtmux_mcp.models.SearchPanesResult are read as "id
   libtmux_mcp, class models, class SearchPanesResult" and never match.
   Python autodoc ids routinely contain dots, so every :py:class:/:meth:
   cross-ref was failing. Switch to getElementById with the literal
   hash, decoded for percent-encoding.

2. The entire scroll block was gated behind !isPop, so popstate-driven
   navigation (browser back/forward) to a fragment URL never scrolled.
   Move the hash branch outside the guard and keep only the
   scrollTo(0, 0) inside it — browsers should still own scroll
   restoration on fragment-less popstate, but fragment targets should
   resolve on every navigation path.

Verified against libtmux-mcp docs: forward SPA click, browser
forward/back, and fragment-less nav all land correctly.
@tony tony force-pushed the spa-nav-fragment-scroll-fix branch from 9841caa to 8fbb3f1 Compare April 19, 2026 08:58
tony added 2 commits April 19, 2026 03:59
…ge copybutton_selector

sphinx-copybutton only runs on DOMContentLoaded. After SPA swap the new
DOM lands without buttons, so gp-sphinx's spa-nav.js re-creates them
inside ``reinit()``. Two defects in that path:

1. ``addCopyButtons`` captured a live ``.copybtn`` from the initial
   page to use as a clone template. A landing page with no code blocks
   (common: index pages, prose overviews) leaves ``copyBtnTemplate``
   null forever, so every subsequent SPA-swapped page that DOES have
   code blocks renders buttonless.

2. ``addCopyButtons`` only iterated ``div.highlight pre``, ignoring
   any additional selectors the project configured via
   ``copybutton_selector`` (e.g. custom prompt admonitions). After
   SPA swap such targets were left without the copy affordance that
   sphinx-copybutton had injected on initial load.

Fixes:

- Drop the clone approach. Emit an inline HTML template that matches
  sphinx-copybutton's button verbatim (same classes, same SVG). The
  initial page no longer needs any ``.copybtn`` for SPA swaps to
  work.
- Add ``gp_sphinx._inject_copybutton_bridge`` on the
  ``html-page-context`` event. It emits a tiny inline
  ``<script>window.GP_SPHINX_COPYBUTTON_SELECTOR=…;</script>`` from
  the project's ``copybutton_selector`` config (gated on
  sphinx-copybutton being loaded). ``spa-nav.js`` reads that global
  and iterates the same selector list sphinx-copybutton used at
  build time, with ``div.highlight pre`` as the fallback.

Verified end-to-end against the libtmux-mcp docs: landing page (no
code blocks) → SPA-nav to an 11-code-block page → every ``<pre>`` gets
a button; SPA-nav to a prompt-only page → every prompt admonition gets
a button.
sphinx-copybutton's ``copybutton.js_t`` emits ``<title>{{ locale copy_to_clipboard }}</title>``
inside its SVG (line 72). The inline template in ``spa-nav.js`` was
missing this element, so SPA-recreated copy buttons had no accessible
name — screen readers announced them as "button"; DevTools / Playwright
accessibility trees rendered them as unlabeled. Clicks still worked
functionally, but the UI was discoverably broken.

Add the ``<title>`` so SPA-recreated buttons are byte-equivalent to
sphinx-copybutton's originals at the accessibility-tree level.

Keeps "Copy to clipboard" hard-coded in English; sphinx-copybutton
localizes this via ``messages[locale]``, which the theme can't see
without another bridge. Non-English docs will get English titles on
SPA-recreated buttons until we wire that through; acceptable for now.
@tony tony force-pushed the spa-nav-fragment-scroll-fix branch from 8fbb3f1 to 6395284 Compare April 19, 2026 08:59
@tony tony merged commit 977b8ca into main Apr 19, 2026
10 checks passed
@tony tony deleted the spa-nav-fragment-scroll-fix branch April 19, 2026 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spa-nav: cross-page fragment links don't scroll to anchor (Python autodoc IDs, popstate)

2 participants