Skip to content

Revert Area connectNulls stacked overlap fix; migrate StackedAreaChartConnectNulls to website example#7073

Merged
PavelVanecek merged 3 commits into
mainfrom
copilot/revert-area-chart-pr-changes
Mar 3, 2026
Merged

Revert Area connectNulls stacked overlap fix; migrate StackedAreaChartConnectNulls to website example#7073
PavelVanecek merged 3 commits into
mainfrom
copilot/revert-area-chart-pr-changes

Conversation

Copilot AI commented Mar 2, 2026

Copy link
Copy Markdown
Contributor

PR #7041 fixed stacked AreaChart null handling but broke connectNulls={true} — null points became break points even when the user explicitly opted into connecting them.

Changes

  • Revert src/cartesian/Area.tsx: Restore connectNulls destructuring and !connectNulls && guard in isBreakPoint. With connectNulls={true} on a stacked chart, null entries are treated as 0 (connected through) rather than skipped.
  • Revert test/chart/AreaChart.spec.tsx: Restore original test name, expected d path, and assertions — pv area correctly curves through the null point at y=43 rather than jumping straight across.
  • Migrate stacked story to website example: StackedAreaChartConnectNulls storybook story removed; AreaChartConnectNulls.tsx website example now shows all four cases: simple with/without connectNulls and stacked with/without connectNulls.
  • Delete stale VR snapshots: Old screenshots for AreaChartConnectNulls removed so CI regenerates them against the updated component.

Related Issue

Fixes #7070

Reopens #6985

Motivation and Context

!connectNulls && was the intentional guard that let users opt into connecting through null values in stacked charts. Removing it (PR #7041) meant connectNulls={true} had no effect in stacked mode, producing a single straight interpolated line instead of a smooth stacked curve, causing visible visual artifacts.

How Has This Been Tested?

Unit test 'Renders null points as 0 if stacked and connectNulls is true' in test/chart/AreaChart.spec.tsx verifies the correct path d values. VR test for AreaChartConnectNulls will generate new screenshots covering all four chart variants.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • I have added a storybook story or VR test, or extended an existing story or VR test to show my changes
Original prompt

This section details on the original issue you should resolve

<issue_title>[Unreleased] Area connect nulls overlaps</issue_title>
<issue_description>## What is the current behavior?

What is the expected behavior?

https://www.chromatic.com/test?appId=63da8268a0da9970db6992aa&id=69a3a5c4a8428a9961c0a8ca

#7041 fixed some problems but created new different problems.</issue_description>

<agent_instructions>Please revert changes from the PR that caused this problem. Then, migrate the AreaChartConnectNulls.tsx storybook from stories to a website example, and then add a VR test for it.</agent_instructions>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed stacked area charts to properly treat missing data points as zero when the connectNulls option is enabled, preventing unwanted breakpoints in the chart visualization.
  • Documentation

    • Added examples demonstrating stacked area chart behavior with and without connectNulls for handling missing data.

Copilot AI changed the title [WIP] Revert changes from PR that caused new issues Revert Area connectNulls stacked overlap fix; migrate StackedAreaChartConnectNulls to website example Mar 2, 2026
@codecov

codecov Bot commented Mar 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.81%. Comparing base (6ebdad8) to head (d7aa03d).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7073   +/-   ##
=======================================
  Coverage   89.80%   89.81%           
=======================================
  Files         529      529           
  Lines       39911    39949   +38     
  Branches     5447     5447           
=======================================
+ Hits        35843    35881   +38     
  Misses       4059     4059           
  Partials        9        9           

☔ 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.

@github-actions

github-actions Bot commented Mar 2, 2026

Copy link
Copy Markdown
Contributor

Staging Deployment Details

These deployments will remain available for 30 days.

To update snapshots: Comment /update-snapshots on this PR to automatically update the baseline screenshots.

@PavelVanecek

Copy link
Copy Markdown
Collaborator

/update-snapshots

@github-actions

github-actions Bot commented Mar 2, 2026

Copy link
Copy Markdown
Contributor

✅ Visual regression snapshots have been updated and committed to this PR.

@codecov

codecov Bot commented Mar 2, 2026

Copy link
Copy Markdown

Bundle Report

Changes will increase total bundle size by 130 bytes (0.0%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
recharts/bundle-cjs 1.27MB 37 bytes (0.0%) ⬆️
recharts/bundle-es6 1.1MB 37 bytes (0.0%) ⬆️
recharts/bundle-umd 541.15kB 19 bytes (0.0%) ⬆️
recharts/bundle-treeshaking-cartesian 635.61kB 37 bytes (0.01%) ⬆️

Affected Assets, Files, and Routes:

view changes for bundle: recharts/bundle-umd

Assets Changed:

Asset Name Size Change Total Size Change (%)
Recharts.js 19 bytes 541.15kB 0.0%
view changes for bundle: recharts/bundle-es6

Assets Changed:

Asset Name Size Change Total Size Change (%)
cartesian/Area.js 37 bytes 27.34kB 0.14%
view changes for bundle: recharts/bundle-cjs

Assets Changed:

Asset Name Size Change Total Size Change (%)
cartesian/Area.js 37 bytes 29.18kB 0.13%
view changes for bundle: recharts/bundle-treeshaking-cartesian

Assets Changed:

Asset Name Size Change Total Size Change (%)
bundle.js 37 bytes 635.61kB 0.01%

@PavelVanecek
PavelVanecek marked this pull request as ready for review March 3, 2026 11:40
@PavelVanecek

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Mar 3, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

The pull request modifies how stacked area charts handle null values when connectNulls is enabled. A guard condition now prevents breakpoints from being triggered by null values in stacked data when connectNulls is true, effectively treating missing values as zero. Supporting changes include test updates, documentation examples, and removal of a story.

Changes

Cohort / File(s) Summary
Core Logic Update
src/cartesian/Area.tsx
Modified computeArea function to extract connectNulls from areaSettings and updated isBreakPoint calculation to guard against null values when connectNulls is enabled in stacked charts, preventing breakpoints from missing data.
Test Updates
test/chart/AreaChart.spec.tsx
Updated test expectations for stacked area charts with connectNulls—null values are now treated as 0 instead of triggering breaks. Simplified path parsing logic and adjusted assertions for curve command types.
Documentation Examples
www/src/docs/exampleComponents/AreaChart/AreaChartConnectNulls.tsx
Added two new stacked AreaChart examples demonstrating behavior with and without connectNulls enabled.
Story Removal
storybook/stories/Examples/AreaChart.stories.tsx
Removed StackedAreaChartConnectNulls story export, including its render function and args definitions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • ckifer
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main changes: reverting the Area connectNulls fix and migrating a story to website examples.
Linked Issues check ✅ Passed All code changes directly address issue #7070 by reverting PR #7041's breaking changes and restoring connectNulls behavior for stacked AreaCharts.
Out of Scope Changes check ✅ Passed All changes are within scope: reverting Area.tsx and test changes, migrating the storybook story, and updating website examples and snapshots.
Description check ✅ Passed The pull request description is comprehensive and well-structured, covering all required template sections including motivation, testing approach, and change types.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch copilot/revert-area-chart-pr-changes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@PavelVanecek
PavelVanecek merged commit 27db8f1 into main Mar 3, 2026
3 checks passed
@PavelVanecek
PavelVanecek deleted the copilot/revert-area-chart-pr-changes branch March 3, 2026 11:48
PavelVanecek pushed a commit that referenced this pull request Jul 20, 2026
…ries is null (#7566)

Fixes #6985.

![Stacked AreaChart with connectNulls, dataWithGaps dataset. Before:
all-null point at Page C collapses to 0. After: Page C connects across,
and the partial-null uv at Page F still renders as
0.](https://raw.githubusercontent.com/kimlj/recharts/pr-6985-assets/6985-before-after.png)

Top chart is master, bottom is this PR. The all-null point (Page C)
connects across; the partial-null point (Page F, only `uv` missing)
still renders as 0, so the #7073 regression does not return.

## Problem

In a stacked AreaChart with `connectNulls`, a data point where every
series is null renders as a drop to 0 instead of connecting across the
gap. d3-stack defaults nulls to 0, so `value1` is never null for stacked
data, and the `!connectNulls` guard is the only thing controlling null
behavior there.

## Change

`computeArea` in `src/cartesian/Area.tsx` marks a point as a break point
when every dataKey in the stack group is null (`wholeStackIsNull`). The
stack dataKeys are supplied by a new `selectStackDataKeys` selector in
`src/state/selectors/areaSelectors.ts`.

The `!connectNulls && rawValue == null` guard is unchanged, so a point
where only some series are null still renders as 0 in the stack.

- Every series null at a point: connects across (new behavior).
- Some series null at a point: unchanged, renders as 0 to preserve stack
offsets.

## Prior art

This is the design proposed by @Harikrushn9118 in #7085: keep the
existing guard, add a separate all-null check, and pass the stack
dataKeys into `computeArea`. #7085 was closed by its author before
completion. This PR implements that design and commits the VR snapshots
#7085 was missing.

## History

#7041 removed the `!connectNulls` guard and was merged, then reverted in
#7073 because it made stacked charts with a single null series render
incorrectly. This PR keeps that guard and scopes the new behavior to the
all-null case, so the #7073 regression does not return.

## Tests

- `test/chart/AreaChart.spec.tsx`: two regression tests, "connects
across points where every stacked series is null if connectNulls is
true" and "breaks at points where every stacked series is null if
connectNulls is false". Both fail without the source change. The
existing "Renders null points as 0 if stacked and connectNulls is true"
test covers the partial-null case that must stay unchanged.
- `www` example `AreaChartConnectNulls.tsx`: adds a `dataWithGaps`
dataset with an all-null point at Page C and a partial-null point at
Page F.
- VR snapshots committed for chromium, firefox, and webkit.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Improved stacked area gap/break detection when an entire stacked point
has all series values missing, so gaps are handled correctly.
* `connectNulls` now reliably bridges fully missing stacked points,
while keeping breaks when `connectNulls` is disabled.
* Partial missing values still render with correct stacked
contributions.
* **Documentation**
* Added stacked area examples showing how gaps are rendered with and
without `connectNulls`.
* **Tests**
* Added coverage validating `connectNulls` behavior for all-missing
stacked points in area charts.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
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.

[Unreleased] Area connect nulls overlaps

2 participants