Skip to content

Commit

Permalink
Fix: allow Brush to be controlled with start and end index (#4034)
Browse files Browse the repository at this point in the history
<!--- Provide a general summary of your changes in the Title above -->

## Description

<!--- Describe your changes in detail -->
Fixes #2404

Allow start and end index to update when passed in/updated externally

## Related Issue

<!--- This project only accepts pull requests related to open issues -->
<!--- If suggesting a new feature or change, please discuss it in an
issue first -->
<!--- If fixing a bug, there should be an issue describing it with steps
to reproduce -->
<!--- Please link to the issue here: -->
#2404

## Motivation and Context

<!--- Why is this change required? What problem does it solve? -->
Fixes a bug

## How Has This Been Tested?

<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
- See storybook entry integration test
- Everything else works as normal

## Screenshots (if appropriate):

![image](https://github.com/recharts/recharts/assets/25180830/6d1b8c5b-00ad-4ae5-a856-1956194ce54c)


## Types of changes

<!--- What types of changes does your code introduce? Put an `x` in all
the boxes that apply: -->

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)

## Checklist:

<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->

- [x] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] I have added tests to cover my changes.
- [x] I have added a storybook story or extended an existing story to
show my changes
- [x] All new and existing tests passed.

---------

Co-authored-by: Coltin Kifer <ckifer@amazon.com>
  • Loading branch information
ckifer and Coltin Kifer committed Jan 3, 2024
1 parent 7a68e5c commit 048d006
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 8 deletions.
15 changes: 13 additions & 2 deletions src/chart/generateCategoricalChart.tsx
Expand Up @@ -1199,10 +1199,10 @@ export const generateCategoricalChart = ({
prevState: CategoricalChartState,
): CategoricalChartState => {
const { dataKey, data, children, width, height, layout, stackOffset, margin } = nextProps;
const { dataStartIndex, dataEndIndex } = prevState;

if (prevState.updateId === undefined) {
const defaultState = createDefaultState(nextProps);

return {
...defaultState,
updateId: 0,
Expand Down Expand Up @@ -1280,9 +1280,16 @@ export const generateCategoricalChart = ({
};
}
if (!isChildrenEqual(children, prevState.prevChildren)) {
// specifically check for Brush - if it exists and the start and end indexes are different, re-render with the new ones
const brush = findChildByType(children, Brush);

const startIndex = brush ? brush.props?.startIndex ?? dataStartIndex : dataStartIndex;
const endIndex = brush ? brush.props?.endIndex ?? dataEndIndex : dataEndIndex;
const hasDifferentStartOrEndIndex = startIndex !== dataStartIndex || endIndex !== dataEndIndex;

// update configuration in children
const hasGlobalData = !isNil(data);
const newUpdateId = hasGlobalData ? prevState.updateId : prevState.updateId + 1;
const newUpdateId = hasGlobalData && !hasDifferentStartOrEndIndex ? prevState.updateId : prevState.updateId + 1;

return {
updateId: newUpdateId,
Expand All @@ -1291,10 +1298,14 @@ export const generateCategoricalChart = ({
props: nextProps,
...prevState,
updateId: newUpdateId,
dataStartIndex: startIndex,
dataEndIndex: endIndex,
},
prevState,
),
prevChildren: children,
dataStartIndex: startIndex,
dataEndIndex: endIndex,
};
}

Expand Down
34 changes: 30 additions & 4 deletions storybook/stories/Examples/cartesian/Brush/Brush.stories.tsx
@@ -1,4 +1,6 @@
import { expect } from '@storybook/jest';
import React, { useState } from 'react';
import { within, userEvent } from '@storybook/testing-library';
import { ComposedChart, ResponsiveContainer, Line, Brush } from '../../../../../src';
import { pageData } from '../../../data';

Expand All @@ -15,7 +17,7 @@ export const ControlledBrush = {
<>
<ResponsiveContainer width="100%" height={400}>
<ComposedChart data={pageData}>
<Line dataKey="uv" />
<Line dataKey="uv" isAnimationActive={false} />

<Brush
startIndex={startIndex}
Expand All @@ -24,22 +26,46 @@ export const ControlledBrush = {
setEndIndex(e.endIndex);
setStartIndex(e.startIndex);
}}
alwaysShowText
/>
</ComposedChart>
</ResponsiveContainer>
<input
type="number"
aria-label="startIndex"
value={startIndex}
onChange={evt => setStartIndex(Number(evt.target.value))}
onChange={evt => {
const num = Number(evt.target.value);
if (Number.isInteger(num)) setStartIndex(num);
}}
/>
<input
type="number"
aria-label="endIndex"
value={endIndex}
onChange={evt => setEndIndex(Number(evt.target.value))}
onChange={evt => {
const num = Number(evt.target.value);
if (Number.isInteger(num)) setEndIndex(num);
}}
/>
</>
);
},
play: async ({ canvasElement }: { canvasElement: HTMLElement }) => {
const user = userEvent.setup();
const { getByLabelText } = within(canvasElement);

const startIndexInput = getByLabelText<HTMLInputElement>('startIndex');
const endIndexInput = getByLabelText<HTMLInputElement>('endIndex');

await user.clear(startIndexInput);
await user.type(startIndexInput, '2');
await user.clear(endIndexInput);
await user.type(endIndexInput, '5');

const brushTexts = document.getElementsByClassName('recharts-brush-texts').item(0).children;
expect(brushTexts.item(0)).toBeInTheDocument();

expect(brushTexts.item(0).textContent).toContain('2');
expect(brushTexts.item(1).textContent).toContain('5');
},
};
68 changes: 66 additions & 2 deletions test/cartesian/Brush.spec.tsx
@@ -1,6 +1,8 @@
import React from 'react';
/* eslint-disable @typescript-eslint/no-non-null-assertion */
import React, { useState } from 'react';
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import { Brush, LineChart, Line, BarChart } from '../../src';
import userEvent from '@testing-library/user-event';
import { Brush, LineChart, Line, BarChart, ComposedChart } from '../../src';
import { assertNotNull } from '../helper/assertNotNull';

describe('<Brush />', () => {
Expand Down Expand Up @@ -182,5 +184,67 @@ describe('<Brush />', () => {
expect(text.textContent).toBe('10');
});
});

const ControlledBrush = () => {
const [startIndex, setStartIndex] = useState<number | undefined>(3);
const [endIndex, setEndIndex] = useState<number | undefined>(data.length - 1);

return (
<>
<ComposedChart data={data} height={400} width={400}>
<Line dataKey="uv" isAnimationActive={false} />

<Brush
startIndex={startIndex}
endIndex={endIndex}
onChange={e => {
setEndIndex(e.endIndex);
setStartIndex(e.startIndex);
}}
alwaysShowText
/>
</ComposedChart>
<input
type="number"
aria-label="startIndex"
value={startIndex}
onChange={evt => {
const num = Number(evt.target.value);
if (Number.isInteger(num)) setStartIndex(num);
}}
/>
<input
aria-label="endIndex"
value={endIndex}
onChange={evt => {
const num = Number(evt.target.value);
if (Number.isInteger(num)) setEndIndex(num);
}}
/>
</>
);
};

test('Travellers should move and chart should update when brush start and end indexes are controlled', async () => {
const user = userEvent.setup();
const { container } = render(<ControlledBrush />);

const traveller = container.querySelector('.recharts-brush-traveller') as SVGGElement;
fireEvent.focus(traveller);

const startIndexInput = screen.getByLabelText<HTMLInputElement>('startIndex');
const endIndexInput = screen.getByLabelText<HTMLInputElement>('endIndex');

await user.clear(startIndexInput);
await user.type(startIndexInput, '2');
await user.clear(endIndexInput);
await user.type(endIndexInput, '5');

const brushTexts = container.getElementsByClassName('recharts-brush-texts').item(0)!.children;
expect(brushTexts.item(0)).toBeInTheDocument();

expect(brushTexts.item(0)?.textContent).toContain('2');
expect(brushTexts.item(1)?.textContent).toContain('5');
});
});
});

0 comments on commit 048d006

Please sign in to comment.