Skip to content

fix(charts): prevent data points from being covered by zoomingTool #6490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions cypress/support/utils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,13 @@ export const cssVarToRgb = (cssVar) => {
const rgbVal = getRGBColor(cssVarValue);
return `rgb(${rgbVal.r}, ${rgbVal.g}, ${rgbVal.b})`;
};

export function testChartZoomingTool(Component, props) {
it('zoomingTool', () => {
cy.mount(<Component {...props} chartConfig={{ zoomingTool: true }} />);
cy.get('.recharts-brush').should('be.visible');

cy.mount(<Component {...props} chartConfig={{ zoomingTool: false }} />);
cy.get('.recharts-brush').should('not.exist');
});
}
4 changes: 3 additions & 1 deletion packages/charts/src/components/BarChart/BarChart.cy.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { complexDataSet } from '../../resources/DemoProps.js';
import { BarChart } from './BarChart.js';
import { cypressPassThroughTestsFactory } from '@/cypress/support/utils';
import { cypressPassThroughTestsFactory, testChartZoomingTool } from '@/cypress/support/utils';

const dimensions = [
{
Expand Down Expand Up @@ -88,5 +88,7 @@ describe('BarChart', () => {
cy.contains('Loading...').should('exist');
});

testChartZoomingTool(BarChart, { dataset: complexDataSet, dimensions, measures });

cypressPassThroughTestsFactory(BarChart, { dimensions: [], measures: [] });
});
11 changes: 2 additions & 9 deletions packages/charts/src/components/BarChart/BarChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import { ChartContainer } from '../../internal/ChartContainer.js';
import { ChartDataLabel } from '../../internal/ChartDataLabel.js';
import { defaultFormatter } from '../../internal/defaults.js';
import { tickLineConfig, tooltipContentStyle, tooltipFillOpacity } from '../../internal/staticProps.js';
import { brushProps, tickLineConfig, tooltipContentStyle, tooltipFillOpacity } from '../../internal/staticProps.js';
import { getCellColors, resolvePrimaryAndSecondaryMeasures } from '../../internal/Utils.js';
import { XAxisTicks } from '../../internal/XAxisTicks.js';
import { YAxisTicks } from '../../internal/YAxisTicks.js';
Expand Down Expand Up @@ -76,7 +76,7 @@
* @param measure {IChartMeasure} Current measure object
* @param dataElement {object} Current data element
*/
highlightColor?: (value: number, measure: MeasureConfig, dataElement: Record<string, any>) => CSSProperties['color'];

Check warning on line 79 in packages/charts/src/components/BarChart/BarChart.tsx

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
}

interface DimensionConfig extends IChartDimension {
Expand Down Expand Up @@ -185,7 +185,7 @@
? dataKeys.findIndex((key) => key === chartConfig.secondYAxis?.dataKey)
: 0;

const [componentRef, chartRef] = useSyncRef<any>(ref);

Check warning on line 188 in packages/charts/src/components/BarChart/BarChart.tsx

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

const onItemLegendClick = useLegendItemClick(onLegendClick);
const labelFormatter = useLabelFormatter(primaryDimension);
Expand Down Expand Up @@ -377,14 +377,7 @@
/>
)}
{chartConfig.zoomingTool && (
<Brush
y={10}
dataKey={primaryDimensionAccessor}
tickFormatter={primaryDimension?.formatter}
stroke={ThemingParameters.sapObjectHeader_BorderColor}
travellerWidth={10}
height={20}
/>
<Brush dataKey={primaryDimensionAccessor} tickFormatter={primaryDimension?.formatter} {...brushProps} />
)}
{children}
</BarChartLib>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { complexDataSet } from '../../resources/DemoProps.js';
import { BulletChart } from './BulletChart.js';
import { cypressPassThroughTestsFactory } from '@/cypress/support/utils';
import { cypressPassThroughTestsFactory, testChartZoomingTool } from '@/cypress/support/utils';

const dimensions = [
{
Expand Down Expand Up @@ -81,5 +81,7 @@ describe('BulletChart', () => {
cy.contains('Loading...').should('exist');
});

testChartZoomingTool(BulletChart, { dataset: complexDataSet, dimensions, measures });

cypressPassThroughTestsFactory(BulletChart, { dimensions: [], measures: [] });
});
11 changes: 2 additions & 9 deletions packages/charts/src/components/BulletChart/BulletChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import { ChartContainer } from '../../internal/ChartContainer.js';
import { ChartDataLabel } from '../../internal/ChartDataLabel.js';
import { defaultFormatter } from '../../internal/defaults.js';
import { tickLineConfig, tooltipContentStyle, tooltipFillOpacity } from '../../internal/staticProps.js';
import { brushProps, tickLineConfig, tooltipContentStyle, tooltipFillOpacity } from '../../internal/staticProps.js';
import { getCellColors, resolvePrimaryAndSecondaryMeasures } from '../../internal/Utils.js';
import { XAxisTicks } from '../../internal/XAxisTicks.js';
import { YAxisTicks } from '../../internal/YAxisTicks.js';
Expand Down Expand Up @@ -66,7 +66,7 @@
* @param measure {IChartMeasure} Current measure object
* @param dataElement {object} Current data element
*/
highlightColor?: (value: number, measure: MeasureConfig, dataElement: Record<string, any>) => CSSProperties['color'];

Check warning on line 69 in packages/charts/src/components/BulletChart/BulletChart.tsx

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
}

interface DimensionConfig extends IChartDimension {
Expand Down Expand Up @@ -143,7 +143,7 @@
...rest
} = props;

const [componentRef, chartRef] = useSyncRef<any>(ref);

Check warning on line 146 in packages/charts/src/components/BulletChart/BulletChart.tsx

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type

const chartConfig = {
yAxisVisible: false,
Expand Down Expand Up @@ -222,7 +222,7 @@
);
} else {
onDataPointClick(
enrichEventWithDetails({} as any, {

Check warning on line 225 in packages/charts/src/components/BulletChart/BulletChart.tsx

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
value: eventOrIndex.value,
dataKey: eventOrIndex.dataKey,
dataIndex: eventOrIndex.index,
Expand Down Expand Up @@ -296,7 +296,7 @@
{chartConfig.xAxisVisible &&
dimensions.map((dimension, index) => {
let AxisComponent;
const axisProps: any = {

Check warning on line 299 in packages/charts/src/components/BulletChart/BulletChart.tsx

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
dataKey: dimension.accessor,
interval: dimension?.interval ?? (isBigDataSet ? 'preserveStart' : 0),
tickLine: index < 1,
Expand Down Expand Up @@ -434,7 +434,7 @@
/>
)}
{sortedMeasures?.map((element, index) => {
const chartElementProps: any = {

Check warning on line 437 in packages/charts/src/components/BulletChart/BulletChart.tsx

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
isAnimationActive: noAnimation === false
};
let labelPosition = 'top';
Expand Down Expand Up @@ -499,14 +499,7 @@
);
})}
{chartConfig.zoomingTool && (
<Brush
y={10}
dataKey={primaryDimensionAccessor}
tickFormatter={primaryDimension?.formatter}
stroke={ThemingParameters.sapObjectHeader_BorderColor}
travellerWidth={10}
height={20}
/>
<Brush dataKey={primaryDimensionAccessor} tickFormatter={primaryDimension?.formatter} {...brushProps} />
)}
{children}
</ComposedChartLib>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { complexDataSet } from '../../resources/DemoProps.js';
import { ColumnChart } from './ColumnChart.js';
import { cypressPassThroughTestsFactory } from '@/cypress/support/utils';
import { cypressPassThroughTestsFactory, testChartZoomingTool } from '@/cypress/support/utils';

const dimensions = [
{
Expand Down Expand Up @@ -78,5 +78,7 @@ describe('ColumnChart', () => {
cy.contains('Loading...').should('exist');
});

testChartZoomingTool(ColumnChart, { dataset: complexDataSet, dimensions, measures });

cypressPassThroughTestsFactory(ColumnChart, { dimensions: [], measures: [] });
});
11 changes: 2 additions & 9 deletions packages/charts/src/components/ColumnChart/ColumnChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import type { IChartMeasure } from '../../interfaces/IChartMeasure.js';
import { ChartContainer } from '../../internal/ChartContainer.js';
import { ChartDataLabel } from '../../internal/ChartDataLabel.js';
import { defaultFormatter } from '../../internal/defaults.js';
import { tickLineConfig, tooltipContentStyle, tooltipFillOpacity } from '../../internal/staticProps.js';
import { brushProps, tickLineConfig, tooltipContentStyle, tooltipFillOpacity } from '../../internal/staticProps.js';
import { getCellColors, resolvePrimaryAndSecondaryMeasures } from '../../internal/Utils.js';
import { XAxisTicks } from '../../internal/XAxisTicks.js';
import { YAxisTicks } from '../../internal/YAxisTicks.js';
Expand Down Expand Up @@ -372,14 +372,7 @@ const ColumnChart = forwardRef<HTMLDivElement, ColumnChartProps>((props, ref) =>
/>
)}
{chartConfig.zoomingTool && (
<Brush
y={10}
dataKey={primaryDimensionAccessor}
tickFormatter={primaryDimension?.formatter}
stroke={ThemingParameters.sapObjectHeader_BorderColor}
travellerWidth={10}
height={20}
/>
<Brush dataKey={primaryDimensionAccessor} tickFormatter={primaryDimension?.formatter} {...brushProps} />
)}
{children}
</ColumnChartLib>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { complexDataSet } from '../../resources/DemoProps.js';
import { ColumnChartWithTrend } from './ColumnChartWithTrend.js';
import { cypressPassThroughTestsFactory } from '@/cypress/support/utils';
import { cypressPassThroughTestsFactory, testChartZoomingTool } from '@/cypress/support/utils';

const dimensions = [
{
Expand Down Expand Up @@ -85,5 +85,7 @@ describe('ColumnChartWithTrend', () => {
cy.contains('Loading...').should('exist');
});

testChartZoomingTool(ColumnChartWithTrend, { dataset: complexDataSet, dimensions, measures });

cypressPassThroughTestsFactory(ColumnChartWithTrend, { dimensions: [], measures: [] });
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { complexDataSet } from '../../resources/DemoProps.js';
import { ComposedChart } from './index.js';
import { cypressPassThroughTestsFactory } from '@/cypress/support/utils';
import { cypressPassThroughTestsFactory, testChartZoomingTool } from '@/cypress/support/utils';

const dimensions = [
{
Expand Down Expand Up @@ -85,5 +85,7 @@ describe('ComposedChart', () => {
cy.contains('Loading...').should('exist');
});

testChartZoomingTool(ComposedChart, { dataset: complexDataSet, dimensions, measures });

cypressPassThroughTestsFactory(ComposedChart, { dimensions: [], measures: [] });
});
11 changes: 2 additions & 9 deletions packages/charts/src/components/ComposedChart/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import type { IChartMeasure } from '../../interfaces/IChartMeasure.js';
import { ChartContainer } from '../../internal/ChartContainer.js';
import { ChartDataLabel } from '../../internal/ChartDataLabel.js';
import { defaultFormatter } from '../../internal/defaults.js';
import { tickLineConfig, tooltipContentStyle, tooltipFillOpacity } from '../../internal/staticProps.js';
import { brushProps, tickLineConfig, tooltipContentStyle, tooltipFillOpacity } from '../../internal/staticProps.js';
import { getCellColors, resolvePrimaryAndSecondaryMeasures } from '../../internal/Utils.js';
import { XAxisTicks } from '../../internal/XAxisTicks.js';
import { YAxisTicks } from '../../internal/YAxisTicks.js';
Expand Down Expand Up @@ -523,14 +523,7 @@ const ComposedChart = forwardRef<HTMLDivElement, ComposedChartProps>((props, ref
);
})}
{chartConfig.zoomingTool && (
<Brush
y={10}
dataKey={primaryDimensionAccessor}
tickFormatter={primaryDimension?.formatter}
stroke={ThemingParameters.sapObjectHeader_BorderColor}
travellerWidth={10}
height={20}
/>
<Brush dataKey={primaryDimensionAccessor} tickFormatter={primaryDimension?.formatter} {...brushProps} />
)}
{children}
</ComposedChartLib>
Expand Down
5 changes: 4 additions & 1 deletion packages/charts/src/components/LineChart/LineChart.cy.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { complexDataSet } from '../../resources/DemoProps.js';
import { LineChart } from './LineChart.js';
import { cypressPassThroughTestsFactory } from '@/cypress/support/utils';
import { cypressPassThroughTestsFactory, testChartZoomingTool } from '@/cypress/support/utils';

const dimensions = [
{
Expand Down Expand Up @@ -32,6 +32,7 @@ describe('LineChart', () => {
cy.get('.recharts-responsive-container').should('be.visible');
cy.get('.recharts-line').should('have.length', 3);
cy.get('.recharts-line-curve').should('have.length', 3);
cy.get('.recharts-brush').should('not.exist');
});

it('click handlers', () => {
Expand Down Expand Up @@ -77,5 +78,7 @@ describe('LineChart', () => {
cy.contains('Loading...').should('exist');
});

testChartZoomingTool(LineChart, { dataset: complexDataSet, dimensions, measures });

cypressPassThroughTestsFactory(LineChart, { dimensions: [], measures: [] });
});
17 changes: 8 additions & 9 deletions packages/charts/src/components/LineChart/LineChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,13 @@ import type { IChartMeasure } from '../../interfaces/IChartMeasure.js';
import { ChartContainer } from '../../internal/ChartContainer.js';
import { ChartDataLabel } from '../../internal/ChartDataLabel.js';
import { defaultFormatter } from '../../internal/defaults.js';
import { tickLineConfig, tooltipContentStyle, tooltipFillOpacity, xAxisPadding } from '../../internal/staticProps.js';
import {
brushProps,
tickLineConfig,
tooltipContentStyle,
tooltipFillOpacity,
xAxisPadding
} from '../../internal/staticProps.js';
import { resolvePrimaryAndSecondaryMeasures } from '../../internal/Utils.js';
import { XAxisTicks } from '../../internal/XAxisTicks.js';
import { YAxisTicks } from '../../internal/YAxisTicks.js';
Expand Down Expand Up @@ -340,14 +346,7 @@ const LineChart = forwardRef<HTMLDivElement, LineChartProps>((props, ref) => {
/>
)}
{chartConfig.zoomingTool && (
<Brush
y={10}
dataKey={primaryDimensionAccessor}
tickFormatter={primaryDimension?.formatter}
stroke={ThemingParameters.sapObjectHeader_BorderColor}
travellerWidth={10}
height={20}
/>
<Brush dataKey={primaryDimensionAccessor} tickFormatter={primaryDimension?.formatter} {...brushProps} />
)}
{children}
</LineChartLib>
Expand Down
8 changes: 7 additions & 1 deletion packages/charts/src/internal/staticProps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,11 @@ import { ThemingParameters } from '@ui5/webcomponents-react-base';

export const tickLineConfig = { stroke: 'transparent' };
export const tooltipContentStyle = { backgroundColor: ThemingParameters.sapBackgroundColor };
export const tooltipFillOpacity: any = { fillOpacity: 0.3 }; // we need type any due to recharts typing error
export const tooltipFillOpacity = { fillOpacity: 0.3 };
export const xAxisPadding = { left: 25, right: 25 };
export const brushProps = {
y: 4,
stroke: ThemingParameters.sapObjectHeader_BorderColor,
travellerWidth: 10,
height: 20
};
2 changes: 1 addition & 1 deletion packages/charts/src/resources/DemoProps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const complexDataSet = [
name: 'September',
users: 200,
sessions: 360,
volume: 879
volume: 1000
},
{
name: 'October',
Expand Down
Loading