All Sessions

E2E Tests for Sidenotes and Footnotes

February 5, 20268:58 AM12:55 PM(3h 57m)9 turns
Model: claude-opus-4-5-20251101
Read22Edit21Bash12Write4Task3Grep1AskUserQuestion1ExitPlanMode1
8:58 AM

Implement the following plan:

E2E Tests for Sidenotes and Footnotes

Overview

Update and expand the existing reading-footnotes.spec.ts E2E tests to cover the portal-based sidenote system and footnote interactions. Fix one broken test and add new tests for margin notes, popovers, hover expansion, keyboard accessibility, and viewport-dependent behavior.

File to modify

  • testing/e2e/tests/reading-footnotes.spec.ts — update and expand

No config changes needed (reading-footnotes project already exists in playwright.config.ts).

Existing tests — what changes

#TestStatus
1Footnotes render as superscript numbersKeep as-is
2Footnote tooltip shows on clickKeep as-is
3Sidenote anchors render with dotted underlineKeep as-is
4Sidenotes with id are present in the DOMFix: selector aside.sidenote[data-sidenote-id].sidenote-margin[data-sidenote-id] (connected sidenotes now return null; margin notes rendered via portal with class sidenote-margin)
5First sidenote (reading tips) has no id and is always visibleKeep as-is

New tests to add

Wide screen tests (viewport 1400x900)

  1. Margin notes are positioned — Each .sidenote-margin should have a numeric top style >= 0 (not the initial -9999 placeholder). Use toPass() retry since positioning is async (RAF-based).

  2. Margin notes don't overlap — Collect top and offsetHeight for all .sidenote-margin elements, sort by top. Verify each note's top >= previous note's (top + height).

  3. Margin note expands on hover — Hover a .sidenote-margin. Verify its computed max-height changes to none (CSS :hover rule). The z-index should also be 10.

Narrow screen tests (viewport 1000x900)

  1. Margin notes hidden on narrow screen.sidenote-margin elements should not be visible (display: none below XL breakpoint).

  2. Sidenote popover shows on click — Click a .sidenote-ref-trigger. A [role="tooltip"] should appear.

  3. Sidenote popover closes on Escape — Open popover, press Escape, verify tooltip disappears.

Interaction tests (default viewport)

  1. Footnote tooltip closes on Escape — Click a footnote button.align-super, verify tooltip visible, press Escape, verify tooltip gone.

  2. Keyboard accessibility for sidenote anchors — Focus a .sidenote-ref-trigger (it has tabIndex={0}, role="button"), press Enter, verify tooltip appears.

Implementation notes

  • Use page.setViewportSize() within individual tests for viewport-specific behavior
  • Use expect(async () => { ... }).toPass() for async positioning assertions (RAF-based)
  • All tests use the auth fixture with a student user navigating to the same reading URL
  • Selectors: .sidenote-margin, .sidenote-ref-trigger, button.align-super, [role="tooltip"], aside.sidenote

Verification

cd testing/e2e && CI=true bunx playwright test --project=reading-footnotes

If you need specific details from before exiting plan mode (like exact code snippets, error messages, or content you generated), read the full transcript at: ~/.claude/projects/-Users-challen-www-usingandunderstanding-ai/0c63a79b-92ff-4444-8c8d-6076f94f3533.jsonl

Let me start by reading the existing test file to understand the current structure.

Now I have a thorough understanding of the implementation. Let me also quickly check the sidenote-ref component for the keyboard handling details.

Now I have everything I need. Let me also check if the dev server is running before writing the tests.

Dev server is running. Now let me write the updated test file.

Now let me run the tests to see if they pass.

The margin notes aren't rendered at all on narrow screens because the portal-based rendering might not create them when they'd be hidden. Let me check more carefully — on narrow viewports, the margin notes get display: none via CSS, but the portal may not render them at all if the component detects narrow viewport. Let me look at how the sidenote provider handles this.

Looking at the sidenote-ref code again, I see the portal only renders when content && marginContainer?.current is truthy. The content comes from useSidenote(id) which depends on getSidenoteContent. On narrow screens, the margin notes ARE rendered into the DOM (via the portal) but hidden with display: none CSS. But the count is 0 — this means the portal container might not be in article. The margin container is a sibling div, not inside article. Let me check where the portal container is in the DOM.

Looking at the sidenote-ref code line 82-95: it creates a portal into marginContainer.current. The SidenoteProvider renders this container at line 117 as a sibling <div> within a relative wrapper. But the key issue is that on narrow screens at 1000px viewport, the content may not be registered yet or the portal isn't inside article.

The issue is simpler — on narrow viewports, the margin notes are rendered but with display: none. The .sidenote-margin[data-sidenote-id] selector should still find them. But the count is 0... Let me check if the sidenote content registration happens at all on narrow screens. Looking at sidenote-ref.tsx line 82: content && marginContainer?.current — the content comes from useSidenote(id) which is just a map lookup, viewport-independent. So the portal should render.

Wait — the issue might be that the Sidenote component (which registers content) might not have rendered yet. Let me check the sidenote.tsx component.

I see — the connected Sidenote component registers its content in a useEffect. Then SidenoteRef reads from the map via useSidenote(id). This is async — there's a render cycle where content might not be available yet. But the real issue is probably that at narrow viewports (1000px), we need to wait for the content registration + portal rendering. The margin notes still get rendered into the DOM even on narrow screens (the CSS hides them), but maybe there's a timing issue.

Let me check the actual screenshot to understand what's happening, and also check if this is just a race condition where we need to wait.

The page is rendering fine. The issue is that at 1000px width, the margin container and portals might not render because the SidenoteRef effect for measuring only runs when hasContent is true and marginContainer?.current exists. But the portal renders unconditionally based on content && marginContainer?.current. Let me think about this differently.

The portal itself is created in the render function of SidenoteRef (line 82-95). It should render into the DOM regardless of viewport. Let me check if marginContainer.current is null at narrow widths — it shouldn't be, as it's just a ref to a div.

Actually, let me re-read the flow: the Sidenote component registers its content via useEffect. The SidenoteRef calls useSidenote(id) to get the content. But useSidenote depends on contentVersion state in the provider to trigger re-renders? No — looking at the provider, contentVersion state is set but never consumed. The getSidenoteContent callback reads from the map but doesn't use state. So SidenoteRef won't re-render when content is registered.

Wait, contentVersion is set via setContentVersion in registerSidenote. But where is it consumed? Looking at the provider... it's not passed to context, it's not used anywhere. This seems like a bug, but the tests pass at 1400px width, so it must work somehow. Let me look more carefully.

Actually, the contentVersion state change triggers a re-render of SidenoteProvider, which re-renders all children including SidenoteRef. Since getSidenoteContent is a useCallback with no deps, it returns the same function reference, but the re-render still causes SidenoteRef to call useSidenote(id) again, getting fresh data from the map.

So the portal should be created on narrow screens too. The issue might be that on a 1000px viewport, the page loads and the content is registered, portals are created, but perhaps the locator .sidenote-margin[data-sidenote-id] is not finding them because the portal renders outside article? Let me check — the portal renders into marginContainer.current, which is a div inside SidenoteProvider's wrapper, which is inside the prose article.

Let me use toPass() to wait for the margin notes to appear:

That test passes now. Let me run the full suite.

The keyboard accessibility test fails because the tooltip has the class xl:hidden — at the default viewport (1280px), the sidenote popover is hidden via xl:hidden since margin notes are shown instead. The popover is only visible on narrow screens. I need to set the viewport to narrow for this test.

The "sidenotes with id are present" test has the same timing issue — at the default viewport (1280px, the XL breakpoint), the margin notes are rendered via portal but need time. This test doesn't use toPass() retry. Let me check what the default viewport is in the playwright config.

The default viewport for Desktop Chrome is 1280x720. At exactly 1280px, we're right at the xl breakpoint (min-width: 1280px). The margin notes might or might not be visible depending on Tailwind's exact breakpoint handling. The issue is the same timing problem — portal content registration is async. I need to use toPass() for test #4 as well.

All 18 tests pass (5 smoke + 13 reading-footnotes). Let me run it one more time to confirm stability.

All 18 tests pass consistently. Here's a summary of the changes:

Fixed (1 test):

  • Test #4 "sidenotes with id are present in the DOM" — Changed selector from aside.sidenote[data-sidenote-id] to .sidenote-margin[data-sidenote-id] (portal-rendered margin notes), and wrapped in toPass() retry since content registration is async.

New tests (8 tests):

Wide screen (1400x900):

  1. Margin notes are positioned — Verifies all .sidenote-margin elements have a top style >= 0 (not the -9999 placeholder)
  2. Margin notes don't overlap — Collects top/height of all margin notes, sorts by position, verifies no overlaps
  3. Margin note expands on hover — Verifies max-height changes to none and z-index becomes 10 on hover

Narrow screen (1000x900): 4. Margin notes hidden on narrow screen — Verifies margin notes exist in DOM but are not visible 5. Sidenote popover shows on click — Clicks a .sidenote-ref-trigger and verifies tooltip appears 6. Sidenote popover closes on Escape — Opens popover, presses Escape, verifies it disappears

Interaction tests: 7. Footnote tooltip closes on Escape — Clicks footnote button, verifies tooltip, presses Escape 8. Keyboard accessibility for sidenote anchors — Focuses trigger, presses Enter, verifies tooltip appears (narrow screen since popover uses xl:hidden)

Read7Bash7Edit3Task1Write1Grep1
10:52 AM

Much better! Now let's also open the sidenote on hover even on large displays. And can we add a bit of padding below each so it doesn't run right into the top of the next?

Let me look at the relevant component and CSS code to understand the current behavior.

Now let me check the sidenote provider's MARGIN_GAP and how it factors into layout, since adding padding below relates to the gap between stacked notes.

Now I understand the full picture. Two changes needed:

  1. Hover on wide screens: The inline trigger (onMouseEnter/onMouseLeave) currently shows the narrow-screen popover (which has xl:hidden). On wide screens, hovering the trigger text should highlight the corresponding margin note instead. I need to communicate which sidenote is "active" so the margin note can get a highlight/expanded style.

  2. Padding below margin notes: The MARGIN_GAP of 12px controls spacing between stacked notes, but visually adding padding-bottom on the .sidenote-margin CSS would give each note breathing room. The gap calculation uses offsetHeight which includes padding, so CSS padding will automatically be accounted for.

Let me think about approach #1. The simplest way: add a CSS class to the margin note when its corresponding trigger is hovered. I can use the sidenote provider context to share an "active sidenote id" state between the trigger and the margin note.

Actually, a simpler approach: since both the trigger and margin note share the same id, I can use a CSS data attribute approach. When the trigger is hovered, set a data attribute on the margin note directly via a ref or DOM query. But since the margin note is in a portal, I can't use CSS sibling selectors.

The cleanest approach: add an activeId state to the sidenote provider context. When a trigger is hovered, set the active id. The margin note reads the active id and applies an "active" class. Let me implement this.

Now I'll make all three changes:

  1. Add activeId state to the sidenote provider context
  2. Set the active id from the trigger's hover events, and apply an active class on the margin note
  3. Add padding-bottom to .sidenote-margin CSS and add the .sidenote-margin-active style matching the hover expansion

Now add the activeId state and pass it through context:

Now update sidenote-ref.tsx to use the active id — set it on hover from the trigger, and apply a class on the margin note:

Now update the trigger's hover handlers to set the active id:

Now update the margin note portal to apply the active class, and also add hover handlers on the margin note itself so hovering the margin note also expands it (keeping the existing CSS :hover working, plus the new active state from the trigger):

Now update the CSS to add padding-bottom and the .sidenote-margin-active class:

Edit8Read4
10:53 AM

How about this. Why don't we, when a sidenote is open, hide all other sidenotes on that side of the page? That might be simplest. And a bit of CSS transition here would be nice, a fast fade in fade out.

(no content)

Good idea — much cleaner. I've already added the activeId plumbing to the provider and ref component. Let me adjust the margin note to also mark inactive notes, then handle everything in CSS with transitions.

Now I need to add the "inactive" class when another sidenote is active, and update the CSS. Let me also re-read the current CSS state.

Good. Now let me make both changes. First, update the margin note className to also mark inactive notes:

Now update the CSS — add padding-bottom, opacity transitions, the active/inactive states, and fold the active expansion into the same rules as hover:

Now let me run the E2E tests to see what needs updating:

All 18 tests pass. Now let me update the E2E tests to also cover the new behavior — hovering a trigger hides other margin notes and expands the active one:

Now let me add two new tests: one for trigger hover expanding the active margin note while hiding others, and one for the trigger hover behavior specifically (hovering the inline text, not the margin note itself).

The opacity transition is 0.15s, so the test checks too soon. I need to use toPass() to wait for the transition to complete.

Edit4Read3Bash3
10:55 AM

OK, but now perhaps we should just let sidenotes overlap in the margin so that they can be correctly positioned horizontally? Meaning, not all the same height. Does that make sense? It would be nice if they were always aligned with their anchor.

(no content)

That makes sense. Since the inactive notes fade out when one is active, overlap at rest doesn't matter — you only ever read one at a time. I'll remove the overlap-avoidance layout engine and just anchor each note directly to its trigger's vertical position.

Now I'll strip out the overlap-avoidance machinery from the provider and simplify the ref's positioning to just use the anchor top directly.

Now simplify the ref — remove the registerPosition call and margin height measurement, just use anchor top directly.

Now update the E2E tests — the "margin notes do not overlap" test should be replaced with a test verifying anchor alignment, and the new trigger-hover test's assertions need updating.

Replace the "do not overlap" test with an anchor-alignment test:

Hmm, that anchor-alignment test is overly complex and the trigger→margin note mapping isn't straightforward since they're matched by sidenote id, not DOM order. Let me simplify — the key property to test is that each margin note's top matches its anchor's position relative to the container.

That's getting messy. The anchor→margin mapping is hard to do from the DOM because they're connected via React context, not DOM structure. The "positioned" test already verifies top >= 0. Let me just replace the overlap test with something cleaner — verify that distinct margin notes have distinct top values that track the document order of their anchors.

Now let me run the tests.

All 19 tests pass. Let me run once more to confirm stability.

Edit5Read3Bash2Write1
11:23 AM

Now they're overlapping. But wait. Another idea, which could be even cooler. Go back to the fixed height stacking but, when a sidenote is selected, slide it up into position and move the others up and out of the way! Does that sound doable? It would be pretty cool. Although, perhaps this is better once we fix the overlap.

(no content)

That sounds great. The approach:

  • At rest: stacked with no overlaps (restore the layout engine)
  • On hover: the active note slides from its stacked position to its anchor-aligned position via CSS transition, while the others fade out

The ref component already computes anchorTop — I just need to keep both it and the adjusted position, then swap between them when active. Let me implement this.

Good. I need three changes:

  1. Provider: Restore the overlap-avoidance layout engine
  2. Ref: Track both anchorTop and stackedTop, use stackedTop at rest but anchorTop when active
  3. CSS: Add transition: top so the slide animates

Let me do all three.

Read3Write1
11:25 AM

Sorry. I know that this is harder, but can the others rearrange themselves instead? I'd like to have a way to keep a sidenote open. So, if you have A, B, and C, and they are collapsed, but B is initially out of position, then on hover B should (1) expand (pushing C down) and (2) move up into position, pushing A upward. Is that doable? That would be wild.

(no content)

11:34 AM

Maybe this is too complicated?

11:36 AM

Let's go for it. Can't you set this up so that content naturally gets pushed down when the note expands, maybe by putting everything in the same container? Then you'd just have to recalculate positions for notes above, not below.

(no content)

11:37 AM

Why don't we plan first.

Let me read the existing plan file, then explore the current state of the code to design this properly.

That plan is from the previous task. This is a completely different task now — an animated margin note layout system. Let me explore the current state of all the relevant files.

The explore agent found the key issue — the overlap avoidance code exists in the provider but the ref component never calls it (it was disconnected during the earlier refactoring). Now let me design the animated layout system.

Good, the plan agent produced a thorough design. Let me read the key file to verify one detail about how scrollHeight will work for measuring expanded height when the note has max-height: 14rem and overflow: hidden.

Good — max-height: 14rem with overflow: hidden means scrollHeight will correctly report the full content height. Now let me write the final plan.

Read2Task2Write1AskUserQuestion1Edit1ExitPlanMode1