Skip to content

Commit

Permalink
Always render static string as <Legend/> value (recharts#3758)
Browse files Browse the repository at this point in the history
<!--- Provide a general summary of your changes in the Title above -->
## Description
Always renders static string in `<Legend/>` when functions are passed as
a dataKey to sibling cartesian components. Prompts the API user to
include the `name` property when `entry.value` is a function.
<!--- Describe your changes in detail -->

## Related Issue
recharts#3757
<!--- 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: -->

## Motivation and Context

<!--- Why is this change required? What problem does it solve? -->
Addresses problems brought up in recharts#3750.
## How Has This Been Tested?
Includes unit tests checking that name prop is rendered when functions
are passed and when the name prop is missing.
<!--- 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. -->

## Screenshots (if appropriate):

## 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] All new and existing tests passed.
  • Loading branch information
chris-mcdonald-dev authored and Gabriel Mercier committed Nov 23, 2023
1 parent dde6ee1 commit 4705e8b
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 11 deletions.
10 changes: 8 additions & 2 deletions src/component/DefaultLegendContent.tsx
Expand Up @@ -4,6 +4,7 @@
import React, { PureComponent, ReactNode, MouseEvent, ReactText, ReactElement } from 'react';
import classNames from 'classnames';
import _ from 'lodash';
import { warn } from '../util/LogUtils';
import { Surface } from '../container/Surface';
import { Symbols } from '../shape/Symbols';
import {
Expand Down Expand Up @@ -170,9 +171,14 @@ export class DefaultLegendContent extends PureComponent<Props> {
return null;
}

const entryValue = _.isFunction(entry.value) ? entry.value() : entry.value;
const color = entry.inactive ? inactiveColor : entry.color;
// Do not render entry.value as functions. Always require static string properties.
const entryValue = !_.isFunction(entry.value) ? entry.value : null;
warn(
!_.isFunction(entry.value),
`The name property is also required when using a function for the dataKey of a chart's cartesian components. Ex: <Bar name="Name of my Data"/>`, // eslint-disable-line max-len
);

const color = entry.inactive ? inactiveColor : entry.color;
return (
<li
className={className}
Expand Down
43 changes: 34 additions & 9 deletions test/component/Legend.spec.tsx
Expand Up @@ -69,18 +69,43 @@ describe('<Legend />', () => {
});

test('Renders payload value correctly when passed as a static value', () => {
render(<Legend payload={[{ value: 'item name', type: 'line', id: 'ID01' }]} />);
render(<Legend payload={[{ value: 'item name' }]} />);
screen.getByText(/item name/i);
});

const legendItem = screen.getByText('item name');
expect(legendItem).toBeInTheDocument();
test('Renders name value of siblings when dataKey is a function', () => {
render(
<LineChart width={500} height={500} data={data}>
<Legend />
<Line dataKey={row => row.value} name="My Line Data" />
<Line dataKey={row => row.color} name="My Other Line Data" />
</LineChart>,
);

// Select the text that was passed into the siblings as a name prop, but rendered in the Legend component.
screen.getByText(/My Line Data/i);
screen.getByText(/My Other Line Data/i);
});

test('Calls function before rendering if provided as a payload value', () => {
const getItemName = jest.fn().mockImplementation(() => 'item name');
render(<Legend payload={[{ value: getItemName, type: 'line', id: 'ID01' }]} />);
test(`Renders '' if sibling's dataKey is a function and name is not provided`, () => {
// Warning should be logged. Spy on it so we can confirm it was called.
const consoleWarn = jest.spyOn(console, 'warn');

const legendItem = screen.getByText('item name');
expect(getItemName).toHaveBeenCalledTimes(1);
expect(legendItem).toBeInTheDocument();
render(
<LineChart width={500} height={500} data={data}>
<Legend />
<Line dataKey={row => row.value} />
<Line dataKey={row => row.color} />
</LineChart>,
);

const legendItems = screen.getAllByRole(/listitem/i);
legendItems.forEach(item => {
expect(item).toHaveTextContent('');
});

expect(consoleWarn).toHaveBeenCalledWith(
`The name property is also required when using a function for the dataKey of a chart's cartesian components. Ex: <Bar name="Name of my Data"/>`,
);
});
});

0 comments on commit 4705e8b

Please sign in to comment.