Skip to content
Open
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
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 @@ import {
useZeroValueDisplay,
useChartMargin,
useElementHeight,
useLegendLayout,
} from '../../hooks';
import {
GlobalChartsProvider,
Expand All @@ -18,6 +19,7 @@ import {
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 @@ const BarChartInternal: FC< BarChartProps > = ( {
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 @@ const BarChartInternal: FC< BarChartProps > = ( {
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 @@ const BarChartInternal: FC< BarChartProps > = ( {
<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 }
xScale={ chartOptions.xScale }
yScale={ chartOptions.yScale }
horizontal={ horizontal }
Expand Down Expand Up @@ -376,7 +385,7 @@ const BarChartInternal: FC< BarChartProps > = ( {
{ 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 @@ import {
useChartDataTransform,
useChartMargin,
useElementHeight,
useLegendLayout,
} from '../../hooks';
import {
GlobalChartsProvider,
Expand All @@ -21,6 +22,7 @@ import {
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 LineChartInternal = forwardRef< SingleChartRef, LineChartProps >(

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 @@ const LineChartInternal = forwardRef< SingleChartRef, LineChartProps >(
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 @@ const LineChartInternal = forwardRef< SingleChartRef, LineChartProps >(
<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 }
// 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 @@ const LineChartInternal = forwardRef< SingleChartRef, LineChartProps >(
{ 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
40 changes: 25 additions & 15 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,17 @@ 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,
} );

if ( ! isValid ) {
return (
<div className={ clsx( 'pie-chart', styles[ 'pie-chart' ], className ) }>
Expand All @@ -231,16 +243,12 @@ 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;
const radius = Math.min( width, adjustedChartHeight ) / 2;

// Center the chart in the available space
const centerX = width / 2;
const centerY = adjustedHeight / 2;
const centerY = adjustedChartHeight / 2;

// Calculate the angle between each (use original data length for consistent spacing)
const padAngle = gapScale * ( ( 2 * Math.PI ) / data.length );
Expand Down Expand Up @@ -273,22 +281,24 @@ const PieChartInternal = ( {
value={ {
chartId,
chartWidth: width,
chartHeight: adjustedHeight,
chartHeight: adjustedChartHeight,
} }
>
<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 }` }
viewBox={ `0 0 ${ width } ${ adjustedChartHeight }` }
preserveAspectRatio="xMidYMid meet"
width={ width }
height={ adjustedHeight }
height={ adjustedChartHeight }
>
<Group top={ centerY } left={ centerX }>
{ allSegmentsHidden ? (
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,18 @@ 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,
} );

if ( ! isValid ) {
return (
<div className={ styles[ 'pie-semi-circle-chart' ] }>
Expand All @@ -260,13 +273,7 @@ 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 radius = Math.min( width / 2, adjustedChartHeight );
const innerRadius = radius * ( 1 - thickness );

// Map data with index for color assignment
Expand All @@ -293,21 +300,23 @@ 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 }
height={ radius }
viewBox={ `0 0 ${ width } ${ chartHeight }` }
viewBox={ `0 0 ${ width } ${ adjustedChartHeight }` }
data-testid="pie-chart-svg"
>
{ /* Main chart group centered horizontally and positioned at bottom */ }
<Group top={ chartHeight } left={ width / 2 }>
<Group top={ adjustedChartHeight } left={ width / 2 }>
{ allSegmentsHidden ? (
<text
textAnchor="middle"
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