From efe7fd8212460807f00730c754e64560a31034e8 Mon Sep 17 00:00:00 2001 From: DrDrij Date: Fri, 21 Aug 2026 14:14:07 +0800 Subject: [PATCH 1/8] refactor: rebuild the contents drawer on the Popover API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every bug this panel has produced — the flash on load, the animated correction, the platform-dependent guard assertion, the icon fade in #127, the width trap in #130 — came from one premise: the panel was always rendered and always painted, with visibility expressed as a transform that only exists once app.css has loaded. That guarantees an unstyled frame where it is visible and a correction afterwards that a transition will animate. Each fix so far has patched a symptom of it. A popover is hidden by the UA stylesheet while closed, so it is already invisible on the very first paint with no author CSS at all. There is no unstyled frame to correct, and so nothing for a transition to animate. The failure mode stops existing rather than being guarded against. What this removes: - `useNavOpen` from both components. There is no open/closed state in React at all now, so the drawer works before hydration and the button cannot desync from the panel. - The `useMounted` transition gate — nothing left to gate. - The `.qe-contents-sidebar` critical-CSS rule and its sync-list entry. - `useSidebarHeight` from both call sites. It bails unless one call site holds both its refs; ours each attached one, so both instances early-returned on every scroll event and did nothing. - The duplicate width utilities, so #130 cannot recur — the three widths are expressed once in CSS with no dependence on emission order. - The icon cross-fade, replaced by a `display` swap driven off `:popover-open` via `:has()`, so it cannot animate on first paint. Behaviour the browser now supplies for free: Escape, click-outside dismissal, the closed panel kept out of the tab order and the accessibility tree, and the invoker's expanded state (verified in the AX tree as false -> true). Both icons are aria-hidden and the button holds the single accessible name; previously each icon carried its own label, so a screen reader could announce both "Show" and "Hide" for one control. Verified against a real build in Chromium and WebKit: hidden on an unstyled first paint (0 boxes, display none), nothing animates when app.css arrives, slides 18-19 frames to x=0 on open, sized 250x750 in both, Escape and light dismiss both close it. The existing visual baselines pass untouched in both projects, so the rendered result is pixel-identical to what it replaces; test:fouc passes 2/2. Two details worth keeping in mind. The UA stylesheet sizes popovers `fit-content` in both axes, so height has to be reset to `auto` or the panel stops at its content instead of stretching to `inset-block`. And React 18's JSX types predate the API, so `popover`/`popovertarget` need the declarations in app/popover.d.ts (react-dom renders them correctly — both survive renderToString on 18.3.1). Non-supporting browsers (iOS <= 16, Firefox < 125) treat the attribute as inert and would render the panel permanently open, so it is hidden outright there via @supports; loading a popover polyfill would restore it if that tail matters. --- app/components/ContentsSidebar.tsx | 62 ++-------- .../NavigationAndArticleWrapper.tsx | 8 +- app/components/toolbar/SidebarToggle.tsx | 52 ++++---- app/popover.d.ts | 22 ++++ app/root.tsx | 12 +- styles/app.css | 112 ++++++++++++++++++ tests/visual/fouc.spec.ts | 58 +++------ tests/visual/theme.spec.ts | 7 +- 8 files changed, 204 insertions(+), 129 deletions(-) create mode 100644 app/popover.d.ts diff --git a/app/components/ContentsSidebar.tsx b/app/components/ContentsSidebar.tsx index a22a86815..81e614ac0 100644 --- a/app/components/ContentsSidebar.tsx +++ b/app/components/ContentsSidebar.tsx @@ -3,16 +3,12 @@ import { getProjectHeadings } from '@myst-theme/common'; import { useBaseurl, useLinkProvider, - useNavOpen, useProjectManifest, useSiteManifest, - useThemeTop, withBaseurl, } from '@myst-theme/providers'; -import { useSidebarHeight } from '@myst-theme/site'; -import classNames from 'classnames'; import { slugToUrl } from 'myst-common'; -import { useEffect, useState } from 'react'; +import { TOC_POPOVER_ID } from './toolbar/SidebarToggle'; type StrictHeading = Omit & { level: number }; type HeadingGroup = StrictHeading[]; @@ -44,26 +40,9 @@ function Section({ group }: { group: HeadingGroup }) { ); } -/** - * True only after the component has mounted on the client. - * - * Both the server render and the first (hydrating) client render return - * `false`, so the markup matches and React does not warn; the effect then - * flips it on the frame after hydration. - */ -function useMounted() { - const [mounted, setMounted] = useState(false); - useEffect(() => setMounted(true), []); - return mounted; -} - export function ContentsSidebar() { - const [open] = useNavOpen(); - const mounted = useMounted(); const config = useSiteManifest(); const project = useProjectManifest(); - const top = useThemeTop(); - const { toc } = useSidebarHeight(top); const baseurl = useBaseurl(); const Link = useLinkProvider(); @@ -96,37 +75,14 @@ export function ContentsSidebar() { ); return ( -
-
Contents
-