unify with recent changes (pre rebase tidyup)

Automattic · Jan 15, 2025 · 33ea4fc · 33ea4fc
1 parent 936fd9a
commit 33ea4fc
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 34 deletions.
diff --git a/projects/js-packages/charts/src/components/pie-chart/pie-chart.tsx b/projects/js-packages/charts/src/components/pie-chart/pie-chart.tsx
@@ -58,29 +58,29 @@ const PieChart = ( {
 			withTooltips,
 		} );
 
-	// Add padding to ensure chart isn't cut off
-	const totalSize = size + padding * 2;
-	const radius = size / 2;
-	const centerX = totalSize / 2;
-	const centerY = totalSize / 2;
+	// Unified dimensions using the `size` property
+	const width = size;
+	const height = size;
+
+	const radius = Math.min( width, height ) / 2;
+	const centerX = width / 2;
+	const centerY = height / 2;
+
+	// Calculate radii based on thickness and padding
+	const outerRadius = radius - padding;
+	const innerRadius = outerRadius * ( 1 - thickness );
 
-	// Map the data to include index for color assignment
 	const dataWithIndex = data.map( ( d, index ) => ( {
 		...d,
 		index,
 	} ) );
 
-	const outerRadius = radius - padding;
-	const innerRadius = outerRadius * ( 1 - thickness );
-
 	const accessors = {
 		value: ( d: DataPointPercentage ) => d.value,
-		// Use the color property from the data object as a last resort. The theme provides colours by default.
 		fill: ( d: DataPointPercentage & { index: number } ) =>
-			d?.color || providerTheme.colors[ d.index ],
+			d?.color || providerTheme.colors[ d.index % providerTheme.colors.length ],
 	};
 
-	// Create legend items from data
 	const legendItems = data.map( ( item, index ) => ( {
 		label: item.label,
 		value: item.value.toString(),
@@ -89,28 +89,34 @@ const PieChart = ( {
 
 	return (
 		<div className={ clsx( 'pie-chart', styles[ 'pie-chart' ], className ) }>
-			<svg viewBox={ `0 0 ${ size } ${ size }` }>
+			<svg
+				width="100%"
+				height="100%"
+				viewBox={ `0 0 ${ size } ${ size }` }
+				preserveAspectRatio="xMidYMid meet"
+			>
 				<Group top={ centerY } left={ centerX }>
 					<Pie< DataPointPercentage & { index: number } >
 						data={ dataWithIndex }
 						pieValue={ accessors.value }
 						outerRadius={ outerRadius }
 						innerRadius={ innerRadius }
+						padAngle={ 0.02 }
+						cornerRadius={ 0 }
 					>
-						{ pie => {
-							return pie.arcs.map( ( arc, index ) => {
+						{ pie =>
+							pie.arcs.map( ( arc, index ) => {
 								const [ centroidX, centroidY ] = pie.path.centroid( arc );
 								const hasSpaceForLabel = arc.endAngle - arc.startAngle >= 0.25;
-								const handleMouseMove = ( event: MouseEvent< SVGElement > ) =>
-									onMouseMove( event, arc.data );
 
 								const pathProps: SVGProps< SVGPathElement > = {
 									d: pie.path( arc ) || '',
 									fill: accessors.fill( arc.data ),
 								};
 
 								if ( withTooltips ) {
-									pathProps.onMouseMove = handleMouseMove;
+									pathProps.onMouseMove = ( event: MouseEvent< SVGElement > ) =>
+										onMouseMove( event, arc.data );
 									pathProps.onMouseLeave = onMouseLeave;
 								}
 
@@ -134,8 +140,8 @@ const PieChart = ( {
 										) }
 									</g>
 								);
-							} );
-						} }
+							} )
+						}
 					</Pie>
 				</Group>
 			</svg>
@@ -153,9 +159,7 @@ const PieChart = ( {
 					data={ tooltipData }
 					top={ tooltipTop || 0 }
 					left={ tooltipLeft || 0 }
-					style={ {
-						transform: 'translate(-50%, -100%)',
-					} }
+					style={ { transform: 'translate(-50%, -100%)' } }
 				/>
 			) }
 		</div>

diff --git a/projects/js-packages/charts/src/components/pie-chart/stories/index.stories.tsx b/projects/js-packages/charts/src/components/pie-chart/stories/index.stories.tsx
@@ -52,8 +52,8 @@ const meta = {
 		size: {
 			control: {
 				type: 'range',
-				min: 0,
-				max: 1000,
+				min: 100,
+				max: 800,
 				step: 10,
 			},
 		},
@@ -65,6 +65,14 @@ const meta = {
 				step: 0.01,
 			},
 		},
+		padding: {
+			control: {
+				type: 'range',
+				min: 0,
+				max: 100,
+				step: 5,
+			},
+		},
 	},
 } satisfies Meta< typeof PieChart >;
 
@@ -73,6 +81,7 @@ type Story = StoryObj< typeof PieChart >;
 
 export const Default: Story = {
 	args: {
+		size: 400,
 		thickness: 1,
 		padding: 20,
 		withTooltips: false,

diff --git a/projects/js-packages/charts/src/components/pie-semi-circle-chart/stories/index.stories.tsx b/projects/js-packages/charts/src/components/pie-semi-circle-chart/stories/index.stories.tsx
@@ -30,9 +30,8 @@ const ResponsiveDecorator = Story => (
 			overflow: 'hidden',
 			padding: '2rem',
 			width: '800px',
-			height: '600px',
+			aspectRatio: '2/1',
 			minWidth: '400px',
-			minHeight: '300px',
 			maxWidth: '1200px',
 			border: '1px dashed #ccc',
 		} }
@@ -49,10 +48,10 @@ const meta = {
 	},
 	decorators: [ ResponsiveDecorator ],
 	argTypes: {
-		width: {
+		size: {
 			control: {
 				type: 'range',
-				min: 0,
+				min: 100,
 				max: 1000,
 				step: 10,
 			},
@@ -65,6 +64,14 @@ const meta = {
 				step: 0.01,
 			},
 		},
+		padding: {
+			control: {
+				type: 'range',
+				min: 0,
+				max: 100,
+				step: 5,
+			},
+		},
 	},
 } satisfies Meta< typeof PieSemiCircleChart >;
 
@@ -73,7 +80,9 @@ type Story = StoryObj< typeof PieSemiCircleChart >;
 
 export const Default: Story = {
 	args: {
+		size: 400,
 		thickness: 0.4,
+		padding: 20,
 		data,
 		label: 'OS',
 		note: 'Windows +10%',
@@ -85,11 +94,16 @@ export const Default: Story = {
 
 export const WithTooltips: Story = {
 	args: {
-		data,
-		label: 'OS',
-		note: 'Windows +10%',
+		...Default.args,
 		withTooltips: true,
 	},
+	parameters: {
+		docs: {
+			description: {
+				story: 'Semi-circle pie chart with interactive tooltips that appear on hover.',
+			},
+		},
+	},
 };
 
 export const WithHorizontalLegend: Story = {
@@ -115,8 +129,9 @@ export const FixedDimensions: Story = {
 		</div>
 	),
 	args: {
-		width: 400,
+		size: 400,
 		thickness: 0.4,
+		padding: 20,
 		data,
 		label: 'Fixed Dimensions',
 		note: 'Non-responsive chart',
@@ -127,7 +142,7 @@ export const FixedDimensions: Story = {
 		docs: {
 			description: {
 				story:
-					'Semi-circle pie chart with fixed dimensions that override the responsive behavior. Only width is needed as height is calculated internally.',
+					'Semi-circle pie chart with fixed dimensions that override the responsive behavior. Uses size prop for unified width/height handling.',
 			},
 		},
 	},