Skip to content

Fix area fill curve to match line stroke in LineChart - #124

Merged
jalexw merged 1 commit into
mainfrom
claude/funny-goldberg-thb0gi
Sep 26, 2026
Merged

jalexw merged 1 commit into
mainfrom
claude/funny-goldberg-thb0gi

Conversation

@jalexw

@jalexw jalexw commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixed a bug in the LineChart component where the area fill's top edge did not match the smoothed line stroke when using the curve="smooth" option. The fill was always drawn with straight lines, causing a visual mismatch between the stroke and the fill boundary.

Changes

  • Refactored curve path building: Extracted the curve selection logic into a new buildCurvePath() function that returns either a smooth or linear path based on the curve type.
  • Updated buildAreaPath() signature: Changed to accept points and curve parameters instead of a pre-built linePath, allowing the function to build the correct curve for each segment.
  • Simplified area path construction: Removed the intermediate linePath parameter and now build the curve path directly within buildAreaPath() for each segment, ensuring the fill's top edge traces the same curve as the stroke.
  • Updated LineChart render logic: Simplified the line and area path building to use the new buildCurvePath() function.
  • Added Storybook test: Added a play() interaction test to the AreaFill story that verifies the area path contains cubic Bézier curve commands (C) and that the fill's top edge matches the line stroke path.

Implementation Details

The key insight is that buildAreaPath() needs to know which curve type to use when re-tracing each segment, rather than relying on a pre-built line path. This ensures the fill's top edge sits exactly under the stroke, regardless of whether smooth or linear curves are used.

https://claude.ai/code/session_01Auhd9pkaf5zD88BE9Q7Nbg

…ead of straight segments

buildAreaPath always re-traced each segment with buildLinePath, so a series
with curve="smooth" and area=true drew a Bezier stroke over a polygonal fill.
It now takes the series curve and traces each segment with the same builder
as the line. Added a play() test to the AreaFill story as a regression check.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Auhd9pkaf5zD88BE9Q7Nbg
@vercel

vercel Bot commented Sep 26, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
ui Ready Ready Preview Sep 26, 2026 2:04pm UTC

Request Review

@jalexw jalexw self-assigned this Sep 26, 2026

jalexw commented Sep 26, 2026

Copy link
Copy Markdown
Contributor Author

Storybook Tests failed on the push run (36247173964), but the failure isn't from this PR. The pull_request run on the same commit (36247238360) passed.

The failing test is Layout/Dashboard Layout › RespectsPrefersReducedMotion: the wordmark's opacity was 0.218928, but the test expected 1. This PR only changes LineChart. All LineChart stories passed in both runs, including the new AreaFill play test.

Root cause: a timing race in .storybook/test-runner.ts. The runner reuses one page for all the stories in a file, and preVisit switches the emulation from no-preference to reduce just before this story renders. The problem is when each part of the page notices the switch:

  • matchMedia(...).matches changes straight away, so the story's play() guard and the motion-reduce: CSS both see reduced motion.
  • The query's change event only fires about 25ms (3 frames) later.
  • Framer Motion caches prefersReducedMotion from that event. So a wordmark that mounts before the event still runs the full 0.2s delay + 0.3s fade, and 300ms after the click it is part-way through.

I reproduced this locally, both ways:

  • The DashboardLayout suite failed 1 time in 32 runs, which is too rare for an A/B comparison to prove anything.
  • A deterministic probe: switch to reduce, open the sidebar immediately, and read the wordmark's opacity 300ms later. Result: 0.14–0.19 every time, the same pattern as CI. After waiting for the change event first, the opacity is 1 every time.

Proposed fix: wait in preVisit for the change event to be delivered before the story renders. I'm not adding it to this PR, because it's outside the LineChart change.

     const emulateReducedMotion: boolean =
       storyContext.parameters?.["emulateReducedMotion"] === true;
 
+    // `matchMedia(...).matches` flips as soon as the emulation changes, but
+    // the query's `change` event is only dispatched a few frames later. Framer
+    // Motion caches the preference from that event, so a story that mounts in
+    // between reads reduced motion in CSS and in `play()` but still animates
+    // in JavaScript. Wait for the event before the story renders.
+    const changing: boolean = await page.evaluate(
+      (reduce: boolean): boolean => {
+        const query: MediaQueryList = window.matchMedia(
+          "(prefers-reduced-motion: reduce)",
+        );
+        if (query.matches === reduce) {
+          return false;
+        }
+        const flagged = window as Window & { __reducedMotionChanged?: boolean };
+        flagged.__reducedMotionChanged = false;
+        query.addEventListener(
+          "change",
+          (): void => {
+            flagged.__reducedMotionChanged = true;
+          },
+          { once: true },
+        );
+        return true;
+      },
+      emulateReducedMotion,
+    );
+
     await page.emulateMedia({
       reducedMotion: emulateReducedMotion ? "reduce" : "no-preference",
     });
+
+    if (changing) {
+      await page.waitForFunction(
+        (): boolean =>
+          (window as Window & { __reducedMotionChanged?: boolean })
+            .__reducedMotionChanged === true,
+      );
+    }

With this patch the DashboardLayout suite passed 24 out of 24 runs. I've re-run the failed job once.


Generated by Claude Code

@jalexw
jalexw merged commit 9865268 into main Sep 26, 2026
16 of 17 checks passed
@jalexw
jalexw deleted the claude/funny-goldberg-thb0gi branch September 26, 2026 14:52

This branch was successfully deployed

1 active deployment
Preview — e04113a0 Deployed Sep 26, 2026 by vercel[bot]
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.

2 participants