Skip to content

Commit

Permalink
Simplify append offset of legend (recharts#4723)
Browse files Browse the repository at this point in the history
  • Loading branch information
ckifer authored Jul 4, 2024
1 parent 22ea074 commit afd5dc3
Show file tree
Hide file tree
Showing 9 changed files with 57 additions and 195 deletions.
1 change: 0 additions & 1 deletion scripts/snapshots/es6Files.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
"es6/util/payload",
"es6/util/isWellBehavedNumber.js",
"es6/util/isDomainSpecifiedByUser.js",
"es6/util/getLegendProps.js",
"es6/util/getEveryNthWithCondition.js",
"es6/util/cursor",
"es6/util/calculateViewBox.js",
Expand Down
1 change: 0 additions & 1 deletion scripts/snapshots/libFiles.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
"lib/util/payload",
"lib/util/isWellBehavedNumber.js",
"lib/util/isDomainSpecifiedByUser.js",
"lib/util/getLegendProps.js",
"lib/util/getEveryNthWithCondition.js",
"lib/util/cursor",
"lib/util/calculateViewBox.js",
Expand Down
1 change: 0 additions & 1 deletion scripts/snapshots/typesFiles.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
"types/util/payload",
"types/util/isWellBehavedNumber.d.ts",
"types/util/isDomainSpecifiedByUser.d.ts",
"types/util/getLegendProps.d.ts",
"types/util/getEveryNthWithCondition.d.ts",
"types/util/cursor",
"types/util/calculateViewBox.d.ts",
Expand Down
3 changes: 1 addition & 2 deletions src/chart/generateCategoricalChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -736,8 +736,7 @@ const calculateOffset = (
}

if (legendItem && prevLegendBBox) {
// @ts-expect-error margin is optional in props but required in appendOffsetOfLegend
offset = appendOffsetOfLegend(offset, props, prevLegendBBox);
offset = appendOffsetOfLegend(offset, legendItem, prevLegendBBox);
}

const offsetWidth = width - offset.left - offset.right;
Expand Down
19 changes: 5 additions & 14 deletions src/util/ChartUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ import {
stackOrderNone,
} from 'victory-vendor/d3-shape';

import { ReactElement, ReactNode } from 'react';
import React, { ReactElement, ReactNode } from 'react';

import { Props as LegendProps } from '../component/Legend';
import { ErrorBar } from '../cartesian/ErrorBar';
import {
findEntryInArray,
Expand All @@ -32,12 +33,10 @@ import {
mathSign,
uniqueId,
} from './DataUtils';
import { filterProps, findAllByType, findChildByType, getDisplayName } from './ReactUtils';
import { filterProps, findAllByType, getDisplayName } from './ReactUtils';
// TODO: Cause of circular dependency. Needs refactor.
// import { RadiusAxisProps, AngleAxisProps } from '../polar/types';
import { Legend } from '../component/Legend';
import { TooltipEntrySettings, TooltipPayloadEntry } from '../state/tooltipSlice';
import { getLegendProps } from './getLegendProps';
import { getNiceTickValues, getTickValuesFixedDomain } from './scale';
import {
AxisDomainType,
Expand All @@ -49,7 +48,6 @@ import {
ChartOffset,
DataKey,
LayoutType,
Margin,
NumberDomain,
RangeObj,
ScaleType,
Expand All @@ -61,9 +59,6 @@ import { ValueType } from '../component/DefaultTooltipContent';
import { AxisMap, AxisObj, AxisPropsWithExtraComputedData } from '../chart/types';
import { inRangeOfSector, polarToCartesian } from './PolarUtils';

// Exported for backwards compatibility
export { getLegendProps };

export function getValueByDataKey<T>(obj: T, dataKey: DataKey<T>, defaultValue?: any): unknown {
if (isNil(obj) || isNil(dataKey)) {
return defaultValue;
Expand Down Expand Up @@ -419,16 +414,12 @@ export const getBarPosition = ({

export const appendOffsetOfLegend = (
offset: ChartOffset,
props: { width?: number; margin: Margin; children?: ReactNode[] },
legendItem: React.ReactElement<LegendProps>,
legendBox: BoundingBox | null,
): ChartOffset => {
const { children, width, margin } = props;
const legendItem = findChildByType(children, Legend);
if (legendItem) {
const legendWidth = width - (margin.left || 0) - (margin.right || 0);
const legendProps = getLegendProps({ legendItem, legendWidth });
const { width: boxWidth, height: boxHeight } = legendBox || {};
const { align, verticalAlign, layout } = legendProps;
const { align, verticalAlign, layout } = legendItem?.props;

if (
(layout === 'vertical' || (layout === 'horizontal' && verticalAlign === 'middle')) &&
Expand Down
20 changes: 0 additions & 20 deletions src/util/getLegendProps.ts

This file was deleted.

7 changes: 0 additions & 7 deletions test/util/ChartUtils.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
MIN_VALUE_REG,
parseSpecifiedDomain,
getTicksOfAxis,
getLegendProps,
isAxisLTR,
AxisPropsNeededForTicksGenerator,
isCategoricalAxis,
Expand Down Expand Up @@ -704,12 +703,6 @@ describe('getDomainOfErrorBars', () => {
});
});

describe('exports for backwards-compatibility', () => {
test('getLegendProps should be exported', () => {
expect(getLegendProps).toBeInstanceOf(Function);
});
});

test('isLTR', () => {
// Axis with reversed=false
expect(
Expand Down
117 changes: 51 additions & 66 deletions test/util/ChartUtils/appendOffsetOfLegend.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,26 +1,10 @@
import { ReactNode } from 'react';
import { vi } from 'vitest';
import { appendOffsetOfLegend } from '../../../src/util/ChartUtils';
import { ChartOffset } from '../../../src/util/types';
import { getLegendProps } from '../../../src/util/getLegendProps';
import { BoundingBox } from '../../../src/util/useGetBoundingClientRect';
import { findChildByType } from '../../../src/util/ReactUtils';

vi.mock('../../../src/util/ReactUtils');

const spy = vi.mocked(findChildByType);

function mockDomElement(item: ReactNode) {
// @ts-expect-error I cannot find a way to type DetailedReactHTMLElement properly
spy.mockReturnValueOnce(item);
}

function mockLegendProps(props: ReturnType<typeof getLegendProps>): ReactNode[] {
const mockLegendItem: ReactNode = { props } as ReactNode;
mockDomElement(mockLegendItem);
return [mockLegendItem];
}

function createMockDomRect(width: number, height: number): BoundingBox {
return { width, height };
}
Expand All @@ -31,9 +15,8 @@ describe('appendOffsetOfLegend', () => {
bottom: 9,
left: 5,
};
const children: ReactNode[] = [];
const legendBox = createMockDomRect(100, 200);
const result = appendOffsetOfLegend(offset, { margin: {}, children }, legendBox);
const result = appendOffsetOfLegend(offset, { props: {}, type: '', key: 'keys' }, legendBox);
expect(result).toBe(offset);
expect(result).toEqual({
bottom: 9,
Expand All @@ -46,11 +29,11 @@ describe('appendOffsetOfLegend', () => {
bottom: 9,
left: 5,
};
const children = mockLegendProps({
layout: 'vertical',
align: 'left',
});
const result = appendOffsetOfLegend(offset, { margin: {}, children }, null);
const props = {
layout: 'vertical' as const,
align: 'left' as const,
};
const result = appendOffsetOfLegend(offset, { props, type: '', key: 'keys' }, null);
expect(result).toEqual({
bottom: 9,
left: 5,
Expand All @@ -62,12 +45,12 @@ describe('appendOffsetOfLegend', () => {
bottom: 9,
left: 5,
};
const children = mockLegendProps({
layout: 'vertical',
align: 'left',
});
const props = {
layout: 'vertical' as const,
align: 'left' as const,
};
const legendBox = createMockDomRect(100, 200);
const result = appendOffsetOfLegend(offset, { margin: {}, children }, legendBox);
const result = appendOffsetOfLegend(offset, { props, type: '', key: 'keys' }, legendBox);
expect(result).toEqual({ bottom: 9, left: 105 });
});

Expand All @@ -76,13 +59,13 @@ describe('appendOffsetOfLegend', () => {
bottom: 9,
left: 5,
};
const children = mockLegendProps({
layout: 'horizontal',
verticalAlign: 'middle',
align: 'left',
});
const props = {
layout: 'horizontal' as const,
verticalAlign: 'middle' as const,
align: 'left' as const,
};
const legendBox = createMockDomRect(100, 200);
const result = appendOffsetOfLegend(offset, { margin: {}, children }, legendBox);
const result = appendOffsetOfLegend(offset, { props, type: '', key: 'keys' }, legendBox);
expect(result).toEqual({ bottom: 9, left: 105 });
});

Expand All @@ -91,12 +74,12 @@ describe('appendOffsetOfLegend', () => {
bottom: 9,
left: 5,
};
const children = mockLegendProps({
layout: 'horizontal',
verticalAlign: 'middle',
});
const props = {
layout: 'horizontal' as const,
verticalAlign: 'middle' as const,
};
const legendBox = createMockDomRect(100, 200);
const result = appendOffsetOfLegend(offset, { margin: {}, children }, legendBox);
const result = appendOffsetOfLegend(offset, { props, type: '', key: 'keys' }, legendBox);
expect(result).toEqual({ bottom: 9, left: 5 });
});

Expand All @@ -105,13 +88,14 @@ describe('appendOffsetOfLegend', () => {
bottom: 9,
left: 5,
};
const children = mockLegendProps({
layout: 'horizontal',
verticalAlign: 'middle',
align: 'left',
});
const props = {
layout: 'horizontal' as const,
verticalAlign: 'middle' as const,
align: 'left' as const,
};
const legendBox = createMockDomRect(100, 200);
appendOffsetOfLegend(offset, { margin: {}, children }, legendBox);
appendOffsetOfLegend(offset, { props, type: '', key: 'keys' }, legendBox);

expect(offset).toEqual({ bottom: 9, left: 5 });
});

Expand All @@ -120,12 +104,12 @@ describe('appendOffsetOfLegend', () => {
bottom: 9,
right: 14,
};
const children = mockLegendProps({
layout: 'horizontal',
verticalAlign: 'bottom',
});
const props = {
layout: 'horizontal' as const,
verticalAlign: 'bottom' as const,
};
const legendBox = createMockDomRect(100, 200);
const result = appendOffsetOfLegend(offset, { margin: {}, children }, legendBox);
const result = appendOffsetOfLegend(offset, { props, type: '', key: 'keys' }, legendBox);
expect(result).toEqual({ bottom: 209, right: 14 });
});

Expand All @@ -134,13 +118,13 @@ describe('appendOffsetOfLegend', () => {
bottom: 9,
right: 14,
};
const children = mockLegendProps({
layout: 'vertical',
align: 'center',
verticalAlign: 'bottom',
});
const props = {
layout: 'horizontal' as const,
verticalAlign: 'bottom' as const,
align: 'center' as const,
};
const legendBox = createMockDomRect(100, 200);
const result = appendOffsetOfLegend(offset, { margin: {}, children }, legendBox);
const result = appendOffsetOfLegend(offset, { props, type: '', key: 'keys' }, legendBox);
expect(result).toEqual({ bottom: 209, right: 14 });
});

Expand All @@ -149,12 +133,12 @@ describe('appendOffsetOfLegend', () => {
bottom: 9,
right: 14,
};
const children = mockLegendProps({
layout: 'vertical',
align: 'center',
});
const props = {
layout: 'vertical' as const,
align: 'center' as const,
};
const legendBox = createMockDomRect(100, 200);
const result = appendOffsetOfLegend(offset, { margin: {}, children }, legendBox);
const result = appendOffsetOfLegend(offset, { props, type: '', key: 'keys' }, legendBox);
expect(result).toEqual({ bottom: 9, right: 14 });
});

Expand All @@ -163,12 +147,13 @@ describe('appendOffsetOfLegend', () => {
bottom: 9,
right: 14,
};
const children = mockLegendProps({
layout: 'horizontal',
verticalAlign: 'middle',
});
const props = {
layout: 'horizontal' as const,
verticalAlign: 'middle' as const,
};

const legendBox = createMockDomRect(100, 200);
const result = appendOffsetOfLegend(offset, { margin: {}, children }, legendBox);
const result = appendOffsetOfLegend(offset, { props, type: '', key: 'keys' }, legendBox);
expect(result).toEqual({ bottom: 9, right: 14 });
});
});
Loading

0 comments on commit afd5dc3

Please sign in to comment.