Merge pull request #296 from adobe/popoverSize

feat: add size props to chartPopover
adobe · May 9, 2024 · 3b30cca · 3b30cca
2 parents 9acb64a + dd42b32
commit 3b30cca
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 18 deletions.
diff --git a/src/RscChart.tsx b/src/RscChart.tsx
@@ -307,7 +307,8 @@ const ChartDialog = ({ datum, itemName, targetElement, setPopoverState, popovers
 	}
 	const popoverDetail = popovers.find((p) => p.name === itemName);
 	const popover = popoverDetail?.callback;
-	const width = popoverDetail?.width;
+	const dialogProps = popoverDetail?.dialogProps;
+	const minWidth = dialogProps?.minWidth ?? 0;
 	return (
 		<DialogTrigger
 			type="popover"
@@ -319,7 +320,7 @@ const ChartDialog = ({ datum, itemName, targetElement, setPopoverState, popovers
 		>
 			<ActionButton UNSAFE_style={{ display: 'none' }}>launch chart popover</ActionButton>
 			{(close) => (
-				<Dialog data-testid="rsc-popover" UNSAFE_className="rsc-popover" minWidth="size-1000" width={width}>
+				<Dialog data-testid="rsc-popover" UNSAFE_className="rsc-popover" {...dialogProps} minWidth={minWidth}>
 					<SpectrumView gridColumn="1/-1" gridRow="1/-1" margin={12}>
 						{popover && datum && popover(datum, close)}
 					</SpectrumView>

diff --git a/src/hooks/usePopovers.tsx b/src/hooks/usePopovers.tsx
@@ -15,11 +15,15 @@ import { getAllElements } from '@utils';
 
 import { Chart } from '../Chart';
 import { ChartPopover } from '../components/ChartPopover';
-import { ChartChildElement, ChartPopoverElement, PopoverHandler } from '../types';
+import { ChartChildElement, ChartPopoverElement, ChartPopoverProps, PopoverHandler } from '../types';
 
 type MappedPopover = { name: string; element: ChartPopoverElement };
 
-export type PopoverDetail = { name: string; callback: PopoverHandler; width?: number };
+export type PopoverDetail = {
+	name: string;
+	callback: PopoverHandler;
+	dialogProps: Omit<ChartPopoverProps, 'children'>;
+};
 
 export default function usePopovers(children: ChartChildElement[]): PopoverDetail[] {
 	const popoverElements = useMemo(
@@ -31,11 +35,14 @@ export default function usePopovers(children: ChartChildElement[]): PopoverDetai
 		() =>
 			popoverElements
 				.filter((popover) => popover.element.props.children)
-				.map((popover) => ({
-					name: popover.name,
-					callback: popover.element.props.children,
-					width: popover.element.props.width,
-				})) as PopoverDetail[],
+				.map((popover) => {
+					const { children, ...dialogProps } = popover.element.props;
+					return {
+						name: popover.name,
+						callback: children as PopoverHandler,
+						dialogProps,
+					};
+				}),
 		[popoverElements]
 	);
 }
diff --git a/src/stories/components/ChartPopover/ChartPopover.story.tsx b/src/stories/components/ChartPopover/ChartPopover.story.tsx
@@ -109,21 +109,27 @@ const AreaStory: StoryFn<typeof ChartPopover> = (args): ReactElement => {
 };
 
 const Canvas = bindWithProps(ChartPopoverCanvasStory);
-Canvas.args = { children: dialogContent, width: 250 };
+Canvas.args = { children: dialogContent, width: 'auto' };
 
 const Svg = bindWithProps(ChartPopoverSvgStory);
-Svg.args = { children: dialogContent, width: 250 };
+Svg.args = { children: dialogContent, width: 'auto' };
+
+const Size = bindWithProps(ChartPopoverSvgStory);
+Size.args = { children: dialogContent, width: 200, height: 100 };
+
+const MinWidth = bindWithProps(ChartPopoverSvgStory);
+MinWidth.args = { children: dialogContent, width: 'auto', minWidth: 250 };
 
 const AreaChart = bindWithProps(AreaStory);
-AreaChart.args = { children: dialogContent };
+AreaChart.args = { children: dialogContent, width: 'auto' };
 
 const DodgedBarChart = bindWithProps(ChartPopoverDodgedBarStory);
-DodgedBarChart.args = { children: dialogContent, width: 250 };
+DodgedBarChart.args = { children: dialogContent, width: 'auto' };
 
 const LineChart = bindWithProps(LineStory);
-LineChart.args = { children: dialogContent };
+LineChart.args = { children: dialogContent, width: 'auto' };
 
 const StackedBarChart = bindWithProps(ChartPopoverSvgStory);
-StackedBarChart.args = { children: dialogContent, width: 250 };
+StackedBarChart.args = { children: dialogContent, width: 'auto' };
 
-export { Canvas, Svg, AreaChart, DodgedBarChart, LineChart, StackedBarChart };
+export { Canvas, Svg, Size, MinWidth, AreaChart, DodgedBarChart, LineChart, StackedBarChart };
diff --git a/src/stories/components/ChartPopover/ChartPopover.test.tsx b/src/stories/components/ChartPopover/ChartPopover.test.tsx
@@ -28,7 +28,7 @@ import {
 } from '@test-utils';
 import userEvent from '@testing-library/user-event';
 
-import { Canvas, DodgedBarChart, LineChart, StackedBarChart, Svg } from './ChartPopover.story';
+import { Canvas, DodgedBarChart, LineChart, MinWidth, Size, StackedBarChart, Svg } from './ChartPopover.story';
 
 describe('ChartPopover', () => {
 	// ChartPopover is not a real React component. This is test just provides test coverage for sonarqube
@@ -104,6 +104,36 @@ describe('ChartPopover', () => {
 		expect(allElementsHaveAttributeValue(bars.slice(1), 'opacity', 1 / HIGHLIGHT_CONTRAST_RATIO)).toBeTruthy();
 	});
 
+	test('Popover should be corrrect size', async () => {
+		render(<Size {...Size.args} />);
+
+		const chart = await findChart();
+		expect(chart).toBeInTheDocument();
+		const bars = getAllMarksByGroupName(chart, 'bar0');
+
+		// clicking the bar should open the popover
+		await clickNthElement(bars, 0);
+		const popover = await screen.findByTestId('rsc-popover');
+		await waitFor(() => expect(popover).toBeInTheDocument()); // waitFor to give the popover time to make sure it doesn't close
+
+		expect(popover).toHaveStyle('width: 200px; height: 100px; min-width: 0px;');
+	});
+
+	test('should honor minWidth', async () => {
+		render(<MinWidth {...MinWidth.args} />);
+
+		const chart = await findChart();
+		expect(chart).toBeInTheDocument();
+		const bars = getAllMarksByGroupName(chart, 'bar0');
+
+		// clicking the bar should open the popover
+		await clickNthElement(bars, 0);
+		const popover = await screen.findByTestId('rsc-popover');
+		await waitFor(() => expect(popover).toBeInTheDocument()); // waitFor to give the popover time to make sure it doesn't close
+
+		expect(popover).toHaveStyle('width: auto; min-width: 250px;');
+	});
+
 	test('Line popover opens and closes corectly when clicking on the chart', async () => {
 		render(<LineChart {...LineChart.args} />);
 		// validate that the line drew

diff --git a/src/types/Chart.ts b/src/types/Chart.ts
@@ -545,8 +545,20 @@ export interface ChartTooltipProps {
 	highlightBy?: 'series' | 'dimension' | 'item' | string[];
 }
 export interface ChartPopoverProps {
+	/** Callback used to control the content rendered in the popover */
 	children?: PopoverHandler;
-	width?: number;
+	/** Width of the popover */
+	width?: number | 'auto';
+	/** Minimum width of the popover */
+	minWidth?: number;
+	/** Maximum width of the popover */
+	maxWidth?: number;
+	/** Height of the popover */
+	height?: number | 'auto';
+	/** Minimum height of the popover */
+	minHeight?: number;
+	/** Maximum height of the popover */
+	maxHeight?: number;
 }
 
 export interface ReferenceLineProps {