Skip to content
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

Chart Library: add bar chart component tests + improve data validation #41296

Merged
merged 9 commits into from
Jan 29, 2025
Prev Previous commit
Next Next commit
improve error handling + data validation
annacmc committed Jan 27, 2025
commit 70129d51d58fdc2a85ef7fc6e2728959159e72a6
62 changes: 43 additions & 19 deletions projects/js-packages/charts/src/components/bar-chart/bar-chart.tsx
Original file line number Diff line number Diff line change
@@ -23,21 +23,15 @@ type BarChartTooltipData = {

interface BarChartProps extends BaseChartProps< SeriesData[] > {}

const BarChart: FC< BarChartProps > = ( {
data,
margin = { top: 20, right: 20, bottom: 40, left: 40 },
withTooltips = false,
showLegend = false,
legendOrientation = 'horizontal',
className,
gridVisibility = 'x',
width,
height = 400,
} ) => {
const BarChart: FC< BarChartProps > = props => {
const theme = useChartTheme();
const { tooltipOpen, tooltipLeft, tooltipTop, tooltipData, hideTooltip, showTooltip } =
useTooltip< BarChartTooltipData >();

const handleMouseLeave = useCallback( () => {
annacmc marked this conversation as resolved.
Show resolved Hide resolved
hideTooltip();
}, [ hideTooltip ] );

const handleMouseMove = useCallback(
(
event: MouseEvent< SVGRectElement >,
@@ -48,7 +42,6 @@ const BarChart: FC< BarChartProps > = ( {
) => {
const coords = localPoint( event );
if ( ! coords ) return;

showTooltip( {
tooltipData: { value, xLabel, yLabel, seriesIndex },
tooltipLeft: coords.x,
@@ -58,12 +51,37 @@ const BarChart: FC< BarChartProps > = ( {
[ showTooltip ]
);

const handleMouseLeave = useCallback( () => {
hideTooltip();
}, [ hideTooltip ] );

const {
annacmc marked this conversation as resolved.
Show resolved Hide resolved
data,
margin = { top: 20, right: 20, bottom: 40, left: 40 },
withTooltips = false,
showLegend = false,
legendOrientation = 'horizontal',
className,
gridVisibility = 'x',
width,
height = 400,
} = props;

// Check for empty data
if ( ! data?.length ) {
return <div className={ clsx( 'bar-chart-empty', styles[ 'bat-chart-empty' ] ) }>Empty...</div>;
return <div className={ clsx( styles[ 'bar-chart-empty' ] ) }>No data available</div>;
}

// Add date validation to hasInvalidData check
const hasInvalidData = data.some( series =>
series.data.some(
d =>
d.value === null ||
d.value === undefined ||
isNaN( d.value ) ||
! d.label ||
( d.date && isNaN( d.date.getTime() ) ) // Add date validation
)
);

if ( hasInvalidData ) {
return <div className={ clsx( styles[ 'bar-chart-error' ] ) }>Invalid data</div>;
}

const margins = margin;
@@ -102,7 +120,13 @@ const BarChart: FC< BarChartProps > = ( {
} ) );

return (
<div className={ clsx( 'bar-chart', className, styles[ 'bar-chart' ] ) }>
annacmc marked this conversation as resolved.
Show resolved Hide resolved
<div
className={ clsx( styles[ 'bar-chart' ], className ) }
data-testid="bar-chart"
style={ { width, height } }
annacmc marked this conversation as resolved.
Show resolved Hide resolved
role="img"
aria-label="bar chart"
>
<svg width={ width } height={ height }>
<Group left={ margins.left } top={ margins.top }>
<GridControl
@@ -159,7 +183,7 @@ const BarChart: FC< BarChartProps > = ( {
<Legend
items={ legendItems }
orientation={ legendOrientation }
className={ styles[ 'bar-chart-legend' ] }
className={ styles[ 'bar-chart__legend' ] }
/>
) }
</div>
Original file line number Diff line number Diff line change
@@ -100,6 +100,22 @@ describe( 'BarChart', () => {
} );
expect( screen.getByText( /invalid data/i ) ).toBeInTheDocument();
} );

test( 'handles invalid label values', () => {
renderWithTheme( {
data: [
{
label: 'Series A',
data: [
{ label: '', value: 10 }, // Empty label
{ label: 'Label 2', value: 20 },
],
options: {},
},
],
} );
expect( screen.getByText( /invalid data/i ) ).toBeInTheDocument();
} );
} );

describe( 'Legend', () => {