Skip to content
Open
Show file tree
Hide file tree
Changes from 9 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: added

Charts: legends - extract common container logic to shared helper
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
.bar-chart {
display: flex;
flex-direction: column;

svg {
overflow: visible;
}

&-legend {
&__legend {
margin-top: 1rem;
}
}
35 changes: 22 additions & 13 deletions projects/js-packages/charts/src/components/bar-chart/bar-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
useZeroValueDisplay,
useChartMargin,
useElementHeight,
useLegendLayout,
} from '../../hooks';
import {
GlobalChartsProvider,
Expand All @@ -18,6 +19,7 @@
useGlobalChartsTheme,
GlobalChartsContext,
} from '../../providers';
import containerStyles from '../../styles/chart-container.module.scss';
import { attachSubComponents } from '../../utils';
import { Legend, useChartLegendItems } from '../legend';
import { SingleChartContext } from '../private/single-chart-context';
Expand Down Expand Up @@ -276,6 +278,15 @@
return generatedStyles;
}, [ selectedIndex, data, chartId ] );

// Use the legend layout hook for consistent calculations
const { adjustedChartHeight, adjustedMargin, containerClassName } = useLegendLayout( {
height,
showLegend,
legendPosition,
legendHeight,
defaultMargin: { ...defaultMargin, ...margin },
} );

// Validate data first
const error = validateData( dataSorted );
const isDataValid = ! error;
Expand Down Expand Up @@ -310,19 +321,23 @@
value={ {
chartId,
chartWidth: width,
chartHeight: height - ( showLegend ? legendHeight : 0 ),
chartHeight: adjustedChartHeight,
} }
>
<div
className={ clsx( 'bar-chart', styles[ 'bar-chart' ], className ) }
className={ clsx(
'bar-chart',
styles[ 'bar-chart' ],
containerStyles[ 'chart-container' ],
containerStyles[ containerClassName ],
className
) }
data-testid="bar-chart"
role="grid"
aria-label={ __( 'Bar chart', 'jetpack-charts' ) }
style={ {
width,
height,
display: 'flex',
flexDirection: showLegend && legendPosition === 'top' ? 'column-reverse' : 'column',
} }
tabIndex={ 0 }
onKeyDown={ onChartKeyDown }
Expand All @@ -334,14 +349,8 @@
<XYChart
theme={ theme }
width={ width }
height={ height - ( showLegend ? legendHeight : 0 ) }
margin={ {
...defaultMargin,
...margin,
...( showLegend && legendPosition === 'top'
? { top: ( defaultMargin.top || 0 ) + legendHeight }
: {} ),
} }
height={ adjustedChartHeight }
margin={ adjustedMargin }

Check failure on line 353 in projects/js-packages/charts/src/components/bar-chart/bar-chart.tsx

View workflow job for this annotation

GitHub Actions / Type checking

Type '{ top?: number; right?: number; bottom?: number; left?: number; }' is not assignable to type 'Margin'.

Check failure on line 353 in projects/js-packages/charts/src/components/bar-chart/bar-chart.tsx

View workflow job for this annotation

GitHub Actions / Type checking

Type '{ top?: number; right?: number; bottom?: number; left?: number; }' is not assignable to type 'Margin'.
xScale={ chartOptions.xScale }
yScale={ chartOptions.yScale }
horizontal={ horizontal }
Expand Down Expand Up @@ -376,7 +385,7 @@
{ allSeriesHidden ? (
<text
x={ width / 2 }
y={ ( height - ( showLegend ? legendHeight : 0 ) ) / 2 }
y={ adjustedChartHeight / 2 }
textAnchor="middle"
fill={ providerTheme.gridStyles?.stroke || '#ccc' }
fontSize="14"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
useGlobalChartsContext,
useGlobalChartsTheme,
} from '../../providers';
import containerStyles from '../../styles/chart-container.module.scss';
import { formatMetricValue, attachSubComponents } from '../../utils';
import { Legend } from '../legend';
import { useChartChildren } from '../private/chart-composition';
Expand Down Expand Up @@ -234,11 +235,19 @@ const LeaderboardChartInternal: FC< LeaderboardChartProps > = ( {
} }
>
<div
className={ clsx( styles.leaderboardChart, loading && styles.loading, className ) }
className={ clsx(
styles.leaderboardChart,
containerStyles[ 'chart-container' ],
containerStyles[
showLegend && legendPosition === 'top'
? 'chart-container--legend-top'
: 'chart-container--legend-bottom'
],
loading && styles.loading,
className
) }
style={ {
...style,
display: 'flex',
flexDirection: showLegend && legendPosition === 'top' ? 'column-reverse' : 'column',
gap: showLegend ? '16px' : '0',
} }
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
useChartDataTransform,
useChartMargin,
useElementHeight,
useLegendLayout,
} from '../../hooks';
import {
GlobalChartsProvider,
Expand All @@ -21,6 +22,7 @@
useGlobalChartsContext,
useGlobalChartsTheme,
} from '../../providers';
import containerStyles from '../../styles/chart-container.module.scss';
import { attachSubComponents } from '../../utils';
import { Legend, useChartLegendItems } from '../legend';
import { DefaultGlyph } from '../private/default-glyph';
Expand Down Expand Up @@ -379,6 +381,15 @@

const defaultMargin = useChartMargin( height, chartOptions, dataSorted, theme );

// Use the legend layout hook for consistent calculations
const { adjustedChartHeight, adjustedMargin, containerClassName } = useLegendLayout( {
height,
showLegend,
legendPosition,
legendHeight,
defaultMargin: { ...defaultMargin, ...margin },
} );

const error = validateData( dataSorted );
const isDataValid = ! error;

Expand Down Expand Up @@ -433,17 +444,21 @@
chartId,
chartRef: internalChartRef,
chartWidth: width,
chartHeight: height - ( showLegend ? legendHeight : 0 ),
chartHeight: adjustedChartHeight,
} }
>
<div
className={ clsx( 'line-chart', styles[ 'line-chart' ], className ) }
className={ clsx(
'line-chart',
styles[ 'line-chart' ],
containerStyles[ 'chart-container' ],
containerStyles[ containerClassName ],
className
) }
data-testid="line-chart"
style={ {
width,
height,
display: 'flex',
flexDirection: showLegend && legendPosition === 'top' ? 'column-reverse' : 'column',
position: 'relative',
} }
>
Expand All @@ -459,14 +474,8 @@
<XYChart
theme={ theme }
width={ width }
height={ height - ( showLegend ? legendHeight : 0 ) }
margin={ {
...defaultMargin,
...margin,
...( showLegend && legendPosition === 'top'
? { top: ( defaultMargin.top || 0 ) + legendHeight }
: {} ),
} }
height={ adjustedChartHeight }
margin={ adjustedMargin }

Check failure on line 478 in projects/js-packages/charts/src/components/line-chart/line-chart.tsx

View workflow job for this annotation

GitHub Actions / Type checking

Type '{ top?: number; right?: number; bottom?: number; left?: number; }' is not assignable to type 'Margin'.

Check failure on line 478 in projects/js-packages/charts/src/components/line-chart/line-chart.tsx

View workflow job for this annotation

GitHub Actions / Type checking

Type '{ top?: number; right?: number; bottom?: number; left?: number; }' is not assignable to type 'Margin'.
// xScale and yScale could be set in Axis as well, but they are `scale` props there.
xScale={ chartOptions.xScale }
yScale={ chartOptions.yScale }
Expand All @@ -483,7 +492,7 @@
{ allSeriesHidden ? (
<text
x={ width / 2 }
y={ ( height - ( showLegend ? legendHeight : 0 ) ) / 2 }
y={ adjustedChartHeight / 2 }
textAnchor="middle"
fill={ providerTheme.gridStyles?.stroke || '#ccc' }
fontSize="14"
Expand Down
32 changes: 22 additions & 10 deletions projects/js-packages/charts/src/components/pie-chart/pie-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { useTooltip, useTooltipInPortal } from '@visx/tooltip';
import { __ } from '@wordpress/i18n';
import clsx from 'clsx';
import { useCallback, useContext, useMemo } from 'react';
import { useElementHeight, useInteractiveLegendData } from '../../hooks';
import { useElementHeight, useInteractiveLegendData, useLegendLayout } from '../../hooks';
import {
GlobalChartsProvider,
useChartId,
Expand All @@ -14,6 +14,7 @@ import {
useGlobalChartsTheme,
GlobalChartsContext,
} from '../../providers';
import containerStyles from '../../styles/chart-container.module.scss';
import { attachSubComponents } from '../../utils';
import { getStringWidth } from '../../visx/text';
import { Legend, useChartLegendItems } from '../legend';
Expand Down Expand Up @@ -223,6 +224,19 @@ const PieChartInternal = ( {
metadata: chartMetadata,
} );

const width = size;
const height = size;

// Use the legend layout hook for consistent calculations
const { adjustedChartHeight, containerClassName } = useLegendLayout( {
height,
showLegend,
legendPosition,
legendHeight,
} );

const adjustedHeight = adjustedChartHeight;

if ( ! isValid ) {
return (
<div className={ clsx( 'pie-chart', styles[ 'pie-chart' ], className ) }>
Expand All @@ -231,10 +245,6 @@ const PieChartInternal = ( {
);
}

const width = size;
const height = size;
const adjustedHeight = showLegend && legendPosition === 'top' ? height - legendHeight : height;

// Calculate radius based on width/height
const radius = Math.min( width, adjustedHeight ) / 2;

Expand Down Expand Up @@ -278,11 +288,13 @@ const PieChartInternal = ( {
>
<div
ref={ containerRef }
className={ clsx( 'pie-chart', styles[ 'pie-chart' ], className ) }
style={ {
display: 'flex',
flexDirection: showLegend && legendPosition === 'top' ? 'column-reverse' : 'column',
} }
className={ clsx(
'pie-chart',
styles[ 'pie-chart' ],
containerStyles[ 'chart-container' ],
containerStyles[ containerClassName ],
className
) }
>
<svg
viewBox={ `0 0 ${ width } ${ adjustedHeight }` }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ import { useTooltip, useTooltipInPortal } from '@visx/tooltip';
import { __ } from '@wordpress/i18n';
import clsx from 'clsx';
import { useCallback, useContext, useMemo } from 'react';
import { useElementHeight, useInteractiveLegendData } from '../../hooks';
import { useElementHeight, useInteractiveLegendData, useLegendLayout } from '../../hooks';
import {
GlobalChartsProvider,
useChartId,
useChartRegistration,
useGlobalChartsContext,
GlobalChartsContext,
} from '../../providers';
import containerStyles from '../../styles/chart-container.module.scss';
import { attachSubComponents } from '../../utils';
import { Legend, useChartLegendItems } from '../legend';
import { ChartSVG, ChartHTML, useChartChildren } from '../private/chart-composition';
Expand Down Expand Up @@ -249,6 +250,20 @@ const PieSemiCircleChartInternal: FC< PieSemiCircleChartProps > = ( {
metadata: chartMetadata,
} );

// Calculate chart dimensions
// TODO: we might want to accept height as a prop in the future, because the height of container might not always be enough.
const height = width / 2;

// Use the legend layout hook for consistent calculations
const { adjustedChartHeight, containerClassName } = useLegendLayout( {
height,
showLegend,
legendPosition,
legendHeight,
} );

const chartHeight = adjustedChartHeight;

if ( ! isValid ) {
return (
<div className={ styles[ 'pie-semi-circle-chart' ] }>
Expand All @@ -260,12 +275,6 @@ const PieSemiCircleChartInternal: FC< PieSemiCircleChartProps > = ( {
</div>
);
}

// Calculate chart dimensions
// TODO: we might want to accept height as a prop in the future, because the height of container might not always be enough.
const height = width / 2;
// The chart only takes the height minus the legend height.
const chartHeight = height - ( showLegend && legendPosition === 'top' ? legendHeight : 0 );
const radius = Math.min( width / 2, chartHeight );
const innerRadius = radius * ( 1 - thickness );

Expand Down Expand Up @@ -293,12 +302,14 @@ const PieSemiCircleChartInternal: FC< PieSemiCircleChartProps > = ( {
>
<div
ref={ containerRef }
className={ clsx( 'pie-semi-circle-chart', styles[ 'pie-semi-circle-chart' ], className ) }
className={ clsx(
'pie-semi-circle-chart',
styles[ 'pie-semi-circle-chart' ],
containerStyles[ 'chart-container' ],
containerStyles[ containerClassName ],
className
) }
data-testid="pie-chart-container"
style={ {
display: 'flex',
flexDirection: showLegend && legendPosition === 'top' ? 'column-reverse' : 'column',
} }
>
<svg
width={ width }
Expand Down
1 change: 1 addition & 0 deletions projects/js-packages/charts/src/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ export { useElementHeight } from './use-element-height';
export { useTextTruncation } from './use-text-truncation';
export { useZeroValueDisplay } from './use-zero-value-display';
export { useInteractiveLegendData } from './use-interactive-legend-data';
export { useLegendLayout } from './use-legend-layout';
Loading
Loading