fix(brush): preserve Brush selection when data array reference changes#7021
fix(brush): preserve Brush selection when data array reference changes#7021pierreeurope wants to merge 4 commits into
Conversation
When a parent component re-renders and creates a new data array reference without changing the number of data items (e.g. due to unrelated state updates), the Brush component was incorrectly resetting its start/end indices to show all bars. Root cause: the setChartData reducer was unconditionally resetting dataEndIndex to the last data index whenever the current dataEndIndex did not equal data.length - 1. This meant any user-defined Brush selection was wiped on every parent re-render that produced a new array reference. Fix: only reset the start/end indices when the data *length* actually changes (initial load or genuine data size change). When the length is unchanged, the current selection is preserved. Fixes recharts#6984
WalkthroughReducer Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/state/chartDataSlice.spec.ts (1)
63-78: Consider adding a data-shrink variant to this testThe test covers data growth (3 → 5 items), which exercises the
prevDataLength !== action.payload.lengthtrue-branch. The symmetric shrink case (5 → 3 items) is the one where the pre-existingdataEndIndex(e.g. 4) would be out-of-bounds in the new dataset — the most error-prone scenario — yet it has no explicit regression guard.Per the coding guidelines "Aim for 100% unit test code coverage when writing new code", a separate
itblock or an extension of this test covering the shrink path would be a worthwhile addition.✅ Suggested additional test case
+ it('should reset indices to full range when data length shrinks', () => { + const chartData = [{ value: 1 }, { value: 2 }, { value: 3 }, { value: 4 }, { value: 5 }]; + const state1 = chartDataReducer(undefined, setChartData(chartData)); + + // User selects the last two items (indices 3..4) + const state2 = chartDataReducer(state1, setDataStartEndIndexes({ startIndex: 3, endIndex: 4 })); + expect(state2.dataStartIndex).toBe(3); + expect(state2.dataEndIndex).toBe(4); + + // Data shrinks: previous endIndex (4) would be out-of-bounds → must reset + const shorterData = [{ value: 1 }, { value: 2 }, { value: 3 }]; + const state3 = chartDataReducer(state2, setChartData(shorterData)); + expect(state3.dataStartIndex).toBe(0); + expect(state3.dataEndIndex).toBe(2); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/state/chartDataSlice.spec.ts` around lines 63 - 78, Add a complementary test that exercises the data-shrink path so the reducer handles out-of-bounds indices: using chartDataReducer, start by setting a longer dataset (e.g., 5 items) with setChartData, narrow the selection via setDataStartEndIndexes so dataEndIndex is near the end (e.g., 4), then dispatch setChartData with a shorter payload (e.g., 3 items) and assert that chartDataReducer resets or clamps dataStartIndex/dataEndIndex into the new valid range (e.g., dataStartIndex === 0 and dataEndIndex === 2) to prevent out-of-bounds indices.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/state/chartDataSlice.spec.ts`:
- Around line 63-78: Add a complementary test that exercises the data-shrink
path so the reducer handles out-of-bounds indices: using chartDataReducer, start
by setting a longer dataset (e.g., 5 items) with setChartData, narrow the
selection via setDataStartEndIndexes so dataEndIndex is near the end (e.g., 4),
then dispatch setChartData with a shorter payload (e.g., 3 items) and assert
that chartDataReducer resets or clamps dataStartIndex/dataEndIndex into the new
valid range (e.g., dataStartIndex === 0 and dataEndIndex === 2) to prevent
out-of-bounds indices.
|
What if the data array changes completely but it just so happens to have the same length? If the array is the same then it should be the same instance. Wrap it in useMemo? |
|
Good question. If the data changes completely but keeps the same length, the brush selection stays but the indices are still valid (within bounds), the user just sees a different slice. Comparing by reference would fix that edge case but it re-introduces the original bug for anyone not wrapping their data in useMemo, which is most users based on the issue report. That said, if you'd prefer full correctness here, we could do a shallow comparison of the first and last elements (or a hash of the array) to detect actual content changes without the cost of a deep compare. Something like checking |
There is one user. I would prefer to keep it the way it is - referential equality means same array, different instances means different arrays. Document that decision, maybe show how to use useMemo to keep the data instance stable. |
Revert the length-based referential equality workaround in the setChartData reducer. The original behavior (reset indices on any new data reference) is kept so that Brush resets on data changes as expected. Add JSDoc on the DataProvider.data prop explaining that an unstable array reference will reset the Brush selection and that useMemo should be used to stabilize it.
|
Makes sense, updated. Reverted the reducer logic and added documentation about stabilizing the data reference with useMemo. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/state/chartDataSlice.ts (1)
65-68: UsingdataEndIndexas a proxy for "data length changed" triggers a false positive when re-rendering with the same dataThe condition
state.dataEndIndex !== action.payload.length - 1resets indices whenever the user has narrowed the Brush so its right edge isn't on the last element — even on re-render with identical data and length. For example: 5 data items, user narrows to[0, 1], data re-renders with the same 5 items →1 ≠ 4→ indices reset to[0, 4].The fix is to compare actual data lengths instead:
♻️ Proposed fix: compare actual data lengths instead of using `dataEndIndex` as a proxy
setChartData(state, action: PayloadAction<ChartData | undefined>) { + const prevDataLength = state.chartData?.length; state.chartData = castDraft(action.payload); if (action.payload == null) { state.dataStartIndex = 0; state.dataEndIndex = 0; return; } - if (action.payload.length > 0 && state.dataEndIndex !== action.payload.length - 1) { + if (action.payload.length > 0 && prevDataLength !== action.payload.length) { state.dataStartIndex = 0; state.dataEndIndex = action.payload.length - 1; } },Note:
setDataStartEndIndexesis not currently part of the public API (not exported fromsrc/index.ts), so no action needed there.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/state/chartDataSlice.ts` around lines 65 - 68, The check that uses state.dataEndIndex to detect changed data length is causing false positives (e.g., when the Brush is narrowed) — update the condition in the reducer that currently reads `if (action.payload.length > 0 && state.dataEndIndex !== action.payload.length - 1)` to instead compare the previous payload length to the new payload length (compare stored data length or a dedicated state field for dataLength against `action.payload.length`) so indices are only reset when the actual data length changes; look for and update logic around the symbols `state.dataEndIndex`, `state.dataStartIndex`, and the reducer handling `action.payload` (and keep `setDataStartEndIndexes` behavior unchanged since it isn't public).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/chart/SunburstChart.tsx`:
- Around line 58-66: Remove the inapplicable Brush/useMemo paragraph from the
JSDoc for SunburstChartProps.data: the SunburstChart expects a single
hierarchical SunburstData root, not a flat array used with Brush, so delete the
brush/referential-equality guidance and either leave a concise note that data is
a hierarchical SunburstData root or replace it with Sunburst-specific guidance
if needed; if you want the original Brush note preserved for the flat-array
case, move it to the DataProvider.data JSDoc (referencing DataProvider.data)
instead.
In `@test/state/chartDataSlice.spec.ts`:
- Around line 42-57: The current test falsely passes because
setDataStartEndIndexes({ startIndex: 0, endIndex: 1 }) leaves dataStartIndex at
0; change the narrowed selection to a non-zero start (e.g.,
setDataStartEndIndexes({ startIndex: 1, endIndex: 2 })) so the subsequent call
to setChartData(longerData) actually verifies the reset of dataStartIndex to 0
in chartDataReducer, and add a complementary test that uses setChartData with a
new array reference but identical length to assert that dataStartIndex and
dataEndIndex are preserved (use setChartData, chartDataReducer, dataStartIndex,
dataEndIndex to locate relevant logic).
---
Nitpick comments:
In `@src/state/chartDataSlice.ts`:
- Around line 65-68: The check that uses state.dataEndIndex to detect changed
data length is causing false positives (e.g., when the Brush is narrowed) —
update the condition in the reducer that currently reads `if
(action.payload.length > 0 && state.dataEndIndex !== action.payload.length - 1)`
to instead compare the previous payload length to the new payload length
(compare stored data length or a dedicated state field for dataLength against
`action.payload.length`) so indices are only reset when the actual data length
changes; look for and update logic around the symbols `state.dataEndIndex`,
`state.dataStartIndex`, and the reducer handling `action.payload` (and keep
`setDataStartEndIndexes` behavior unchanged since it isn't public).
| * **Brush and referential equality:** Recharts compares the `data` prop by reference on every | ||
| * render. Passing an inline array literal or creating a new array inside the render function | ||
| * causes the Brush selection (start/end indices) to reset on every re-render because each | ||
| * render produces a new array reference. To preserve the Brush selection across re-renders, | ||
| * stabilize the reference with `useMemo` so the same array instance is reused as long as the | ||
| * underlying data has not changed. | ||
| * | ||
| * @example data={[{ name: 'a', value: 12, fill: '#8884d8' }, { name: 'b', value: 5, fill: '#83a6ed' }]} | ||
| */ |
There was a problem hiding this comment.
Brush/referential-equality documentation is inapplicable to SunburstChartProps.data
SunburstChart has no Brush component and data is typed as SunburstData — a single hierarchical root object, not the flat ChartData array that feeds setChartData and drives Brush index state. The added JSDoc block is therefore both factually incorrect and likely to confuse readers.
Per the AI summary, this documentation belongs on DataProvider.data in src/util/types.ts. Please remove the Brush/useMemo paragraph here (or replace it with SunburstChart-relevant guidance if there's any actual reference-equality concern for this prop).
🛠️ Proposed fix: remove the inapplicable Brush note
/**
* The source data. Each element should be an object.
* The properties of each object represent the values of different data dimensions.
*
* Use the `dataKey` prop to specify which properties to use.
*
- * **Brush and referential equality:** Recharts compares the `data` prop by reference on every
- * render. Passing an inline array literal or creating a new array inside the render function
- * causes the Brush selection (start/end indices) to reset on every re-render because each
- * render produces a new array reference. To preserve the Brush selection across re-renders,
- * stabilize the reference with `useMemo` so the same array instance is reused as long as the
- * underlying data has not changed.
- *
* `@example` data={[{ name: 'a', value: 12, fill: '#8884d8' }, { name: 'b', value: 5, fill: '#83a6ed' }]}
*/
data: SunburstData;As per coding guidelines: "JSDoc comments and TypeScript definitions in source files must be kept up-to-date as they are used to autogenerate API documentation via npm run omnidoc."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * **Brush and referential equality:** Recharts compares the `data` prop by reference on every | |
| * render. Passing an inline array literal or creating a new array inside the render function | |
| * causes the Brush selection (start/end indices) to reset on every re-render because each | |
| * render produces a new array reference. To preserve the Brush selection across re-renders, | |
| * stabilize the reference with `useMemo` so the same array instance is reused as long as the | |
| * underlying data has not changed. | |
| * | |
| * @example data={[{ name: 'a', value: 12, fill: '#8884d8' }, { name: 'b', value: 5, fill: '#83a6ed' }]} | |
| */ | |
| /** | |
| * The source data. Each element should be an object. | |
| * The properties of each object represent the values of different data dimensions. | |
| * | |
| * Use the `dataKey` prop to specify which properties to use. | |
| * | |
| * `@example` data={[{ name: 'a', value: 12, fill: '#8884d8' }, { name: 'b', value: 5, fill: '#83a6ed' }]} | |
| */ | |
| data: SunburstData; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/chart/SunburstChart.tsx` around lines 58 - 66, Remove the inapplicable
Brush/useMemo paragraph from the JSDoc for SunburstChartProps.data: the
SunburstChart expects a single hierarchical SunburstData root, not a flat array
used with Brush, so delete the brush/referential-equality guidance and either
leave a concise note that data is a hierarchical SunburstData root or replace it
with Sunburst-specific guidance if needed; if you want the original Brush note
preserved for the flat-array case, move it to the DataProvider.data JSDoc
(referencing DataProvider.data) instead.
There was a problem hiding this comment.
Yeah this shouldn't be here, it doesn't apply
There was a problem hiding this comment.
@ckifer Thanks for confirming! I appreciate you catching that—the Brush documentation belongs in the flat-array chart components, not here. 👍
🧠 Learnings used
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-29T02:45:55.748Z
Learning: Provide a simple, declarative, and composable way to build charts in Recharts
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-29T02:45:55.748Z
Learning: Value consistency, usability, and performance in Recharts implementation
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6896
File: src/chart/AreaChart.tsx:0-0
Timestamp: 2026-01-17T03:23:05.586Z
Learning: In recharts, generic forwardRef chart components are an exception to the "no `as` assertions" guideline. Due to TypeScript 4.x compatibility requirements, the pattern `export const Chart = forwardRef<El, Props<unknown>>(...) as <T>(props: Props<T> & { ref?: Ref<El> }) => React.ReactElement` is acceptable until the minimum TypeScript version can be bumped to 5.1+ (which requires ReactNode return type support). This will be addressed in a future major version (tracked in issue `#6898`).
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6659
File: www/src/components/GuideView/Performance/index.tsx:232-250
Timestamp: 2025-11-19T14:08:01.728Z
Learning: In Recharts version 3.4.2, object-as-prop optimizations were introduced to reduce unnecessary re-renders when new object references are passed as props. This affects the recommendation for the `react-perf/jsx-no-new-object-as-prop` ESLint rule.
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6896
File: src/chart/AreaChart.tsx:0-0
Timestamp: 2026-01-17T03:11:01.347Z
Learning: In recharts (TypeScript 5.7.2+), generic forwardRef chart components use explicit type annotations with ReactNode return type to avoid `as` assertions. Pattern: `export const Chart: <T>(props: Props<T> & { ref?: Ref<El> }) => ReactNode = forwardRef<El, Props<unknown>>((props, ref) => ...)`. This is acceptable because TS 5.1+ allows ReactNode as valid JSX return type.
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-29T02:45:55.748Z
Learning: Ensure accessibility is important in Recharts components and implementation
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6855
File: www/src/docs/api/AreaChartAPI.tsx:422-438
Timestamp: 2026-01-06T22:34:01.675Z
Learning: In Recharts 3, components can render inside any chart type as long as the parent provides the necessary context. For example, Funnel can render inside AreaChart, BarChart, ComposedChart, LineChart, and ScatterChart because they all provide cartesian context. This is a change from Recharts 2, which had stricter parent-child component limitations.
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6750
File: src/state/selectors/axisSelectors.ts:593-602
Timestamp: 2025-12-08T08:23:26.261Z
Learning: In the recharts codebase, `DataKey<any>` is an intentional exception to the "no any" rule while proper typing is being developed. This should not be flagged in reviews.
Learnt from: PavelVanecek
Repo: recharts/recharts PR: 6669
File: www/src/docs/exampleComponents/ScatterChart/ScatterChartWithLabels.tsx:2-2
Timestamp: 2025-11-23T13:30:10.395Z
Learning: The `TooltipIndex` type from recharts is defined in `src/state/tooltipSlice.ts` but is not currently exported from the public API entry points. It should not be imported from `recharts/types/state/tooltipSlice` as this is an internal implementation path. An ESLint rule is needed to prevent regressions.
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-29T02:45:55.748Z
Learning: When making changes to Recharts code, prefer current best practices as described in CONTRIBUTING.md and try to improve code style where relevant to the current task
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-29T02:45:55.748Z
Learning: Do not attempt to fix too much style inconsistency at once in Recharts, or when it is not related to the current changes
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-29T02:45:55.748Z
Learning: Do not focus too much on fitting the existing Recharts coding style if it is not ideal; prefer current best practices
Learnt from: CR
Repo: recharts/recharts PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-29T02:45:55.748Z
Learning: Do not hardcode any strings or formatting choices in Recharts library code; expect users to provide localized strings
Learnt from: cloud-walker
Repo: recharts/recharts PR: 6882
File: www/src/docs/apiExamples/ReferenceDot/ReferenceDotExample.tsx:55-55
Timestamp: 2026-01-19T20:58:56.677Z
Learning: In recharts documentation examples (files under www/src/docs/), hardcoded color values like "red", "blue", etc. are sometimes intentionally used alongside CSS variables to show variety in styling approaches. These should not be flagged as inconsistent with theming.
| it('should reset indices to full range when data length changes', () => { | ||
| const chartData = [{ value: 1 }, { value: 2 }, { value: 3 }]; | ||
| const action1 = setChartData(chartData); | ||
| const state1 = chartDataReducer(undefined, action1); | ||
|
|
||
| // User narrows selection | ||
| const state2 = chartDataReducer(state1, setDataStartEndIndexes({ startIndex: 0, endIndex: 1 })); | ||
| expect(state2.dataEndIndex).toBe(1); | ||
|
|
||
| // Data grows: indices reset to cover full new range | ||
| const longerData = [{ value: 1 }, { value: 2 }, { value: 3 }, { value: 4 }, { value: 5 }]; | ||
| const action3 = setChartData(longerData); | ||
| const state3 = chartDataReducer(state2, action3); | ||
| expect(state3.dataStartIndex).toBe(0); | ||
| expect(state3.dataEndIndex).toBe(4); | ||
| }); |
There was a problem hiding this comment.
The test does not actually exercise the line 66 fix (state.dataStartIndex = 0)
setDataStartEndIndexes({ startIndex: 0, endIndex: 1 }) leaves dataStartIndex at 0. The subsequent assertion expect(state3.dataStartIndex).toBe(0) therefore passes even if line 66 is deleted — dataStartIndex was already 0 and would simply remain untouched. To make the test sensitive to the new reset logic, the narrowed selection must start at a non-zero index.
🛠️ Proposed fix: use a non-zero `startIndex` to make the reset observable
- // User narrows selection
- const state2 = chartDataReducer(state1, setDataStartEndIndexes({ startIndex: 0, endIndex: 1 }));
- expect(state2.dataEndIndex).toBe(1);
+ // User narrows selection — start at 1 so the reset to 0 is observable
+ const state2 = chartDataReducer(state1, setDataStartEndIndexes({ startIndex: 1, endIndex: 1 }));
+ expect(state2.dataStartIndex).toBe(1);
+ expect(state2.dataEndIndex).toBe(1);Additionally, a complementary test covering the "same length, new reference, full-range brush → no reset" path (the original #6984 scenario) is absent from the file despite being mentioned in the PR description. Consider adding:
it('should preserve Brush selection when data array reference changes but length stays the same', () => {
const chartData = [{ value: 1 }, { value: 2 }, { value: 3 }];
const state1 = chartDataReducer(undefined, setChartData(chartData));
// Re-render with a new reference but identical length — indices must not reset
const newRef = [{ value: 1 }, { value: 2 }, { value: 3 }];
const state2 = chartDataReducer(state1, setChartData(newRef));
expect(state2.dataStartIndex).toBe(0);
expect(state2.dataEndIndex).toBe(2);
});As per coding guidelines: "Aim for 100% unit test code coverage when writing new code."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/state/chartDataSlice.spec.ts` around lines 42 - 57, The current test
falsely passes because setDataStartEndIndexes({ startIndex: 0, endIndex: 1 })
leaves dataStartIndex at 0; change the narrowed selection to a non-zero start
(e.g., setDataStartEndIndexes({ startIndex: 1, endIndex: 2 })) so the subsequent
call to setChartData(longerData) actually verifies the reset of dataStartIndex
to 0 in chartDataReducer, and add a complementary test that uses setChartData
with a new array reference but identical length to assert that dataStartIndex
and dataEndIndex are preserved (use setChartData, chartDataReducer,
dataStartIndex, dataEndIndex to locate relevant logic).
|
Regarding the SunburstChart note: yes, Brush doesn't apply to SunburstChart (it uses hierarchical SunburstData rather than a flat array). I removed it, but recharts enforces JSDoc consistency across all components that share the For now I've kept it consistent with the other charts. If you'd like it removed, the test would need to be updated to exclude SunburstChart from the |
|
I'm confused, we didn't actually accomplish anything here |
|
@pierreeurope put a new exception in |
|
Fair point. Referential equality is the right contract here, and the fix should be on the consumer side with useMemo rather than changing how Brush handles it internally. Closing this. |
|
I still find it useful to have this decision documented. If you add the exception then we can merge the test and jsdoc. |
## Description Controlled `startIndex`/`endIndex` on `<Brush>` get silently overwritten when the chart `data` array changes. The sync effect that copies the props into Redux only depended on the index props themselves, so once `chartDataSlice` reset `dataStartIndex`/`dataEndIndex` on a data update, nothing re-applied the controlled values. Adding `chartData` to that effect's dependency array makes it re-assert the controlled indices any time the data changes, not just when the index props change. This does not touch `chartDataSlice.ts` or the reducer's reset behavior at all, and does not affect uncontrolled Brush usage in any way (`setDataStartEndIndexes` is a no-op when both indices are undefined). It's unrelated to the referential-equality discussion in #7021 - that was about whether Brush should treat a same-length new array reference as "unchanged" for internally-managed selection state, which is a separate question from what a controlled component does with props it was explicitly given. This PR doesn't reopen that; it just makes controlled props actually win, the same way a controlled `<input value>` would. ## Related Issue Fixes #7462 ## Motivation and Context Bug fix. Controlled Brush is a documented pattern and currently doesn't work reliably across data updates. ## How Has This Been Tested? Added a unit test in test/cartesian/Brush.spec.tsx that renders a controlled Brush, changes the data array to a new reference of the same length, and asserts the controlled indices are preserved. Ran the full unit suite locally, all green. ## Types of changes - [x] Bug fix (non-breaking change which fixes an issue) ## Checklist: - [ ] My change requires a change to the documentation. - [x] I have added tests to cover my changes. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Brush start/end range controls now remain consistent when the chart updates its underlying data, avoiding unintended resets of the selected brush positions. * Controlled brush indices are preserved across data re-renders, even when the chart receives a new data array instance. * **Tests** * Added coverage to verify controlled Brush indices stay stable after parent chart re-renders with updated data identity. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
3 participants
Problem
When a parent component re-renders and creates a new data array reference — without changing the actual data or its length — the Brush component incorrectly resets its
startIndex/endIndexback to the full range, showing all bars instead of the user-defined selection.This was reported in #6984 with a live Stackblitz reproduction.
Root Cause
In
src/state/chartDataSlice.ts, thesetChartDatareducer contained this logic:The condition
state.dataEndIndex !== action.payload.length - 1istruewhenever the user has narrowed the Brush selection (e.g. endIndex = 5 with 10 data points). So any parent re-render that dispatchessetChartDatawith the same data reference count resets the selection to show all bars.Fix
Only reset
dataStartIndex/dataEndIndexwhen the data length actually changes (initial load or genuine data size change). When the length is unchanged the user's Brush selection is preserved.This matches the existing behaviour for every existing test case:
prevDataLength = undefined): resets to full range ✅Tests
Added two new unit tests to
test/state/chartDataSlice.spec.ts:should preserve Brush selection when data array reference changes but length stays the sameshould reset indices to full range when data length changesAll 13,960 existing tests continue to pass.
Fixes #6984
Summary by CodeRabbit
Bug Fixes
Documentation