Merge pull request #648 from adobe/AN-404626

feat: add prop for ChartPopover

Author: tanugarg04

packages/react-spectrum-charts/src/RscChart.tsx

@@ -145,7 +145,7 @@ RscChart.displayName = 'RscChart';
 
 const ChartDialog = ({ datum, popover, setIsPopoverOpen, targetElement }: ChartDialogProps) => {
   const { chartPopoverProps, name } = popover;
-  const { children, onOpenChange, containerPadding, rightClick, ...dialogProps } = chartPopoverProps;
+  const { children, onOpenChange, containerPadding, contentMargin, rightClick, ...dialogProps } = chartPopoverProps;
   const minWidth = dialogProps.minWidth ?? 0;
 
   return (
@@ -166,7 +166,12 @@ const ChartDialog = ({ datum, popover, setIsPopoverOpen, targetElement }: ChartD
       </ActionButton>
       {(close) => (
         <Dialog data-testid="rsc-popover" UNSAFE_className="rsc-popover" {...dialogProps} minWidth={minWidth}>
-          <SpectrumView gridColumn="1/-1" gridRow="1/-1" margin={12}>
+          <SpectrumView
+            data-testid="rsc-popover-content"
+            gridColumn="1/-1"
+            gridRow="1/-1"
+            margin={contentMargin ?? 12}
+          >
             {datum && datum[COMPONENT_NAME] === name && children?.(datum, close)}
           </SpectrumView>
         </Dialog>

packages/react-spectrum-charts/src/rscToSbAdapter/chartPopoverAdapter.test.ts

@@ -24,4 +24,8 @@ describe('getChartPopoverOptions()', () => {
     const options = getChartPopoverOptions({});
     expect(options).not.toHaveProperty('height');
   });
+  it('should pass through contentMargin prop', () => {
+    const options = getChartPopoverOptions({ contentMargin: 24 });
+    expect(options).toHaveProperty('contentMargin', 24);
+  });
 });

packages/react-spectrum-charts/src/stories/components/ChartPopover/ChartPopover.story.tsx

@@ -170,9 +170,13 @@ StackedBarChart.args = { children: dialogContent, width: 'auto' };
 const Svg = bindWithProps(ChartPopoverSvgStory);
 Svg.args = { children: dialogContent, width: 'auto' };
 
+const ContentMargin = bindWithProps(ChartPopoverSvgStory);
+ContentMargin.args = { children: dialogContent, width: 'auto', contentMargin: 24 };
+
 export {
   AreaChart,
   Canvas,
+  ContentMargin,
   DodgedBarChart,
   DonutChart,
   LineChart,

packages/react-spectrum-charts/src/stories/components/ChartPopover/ChartPopover.test.tsx

@@ -32,6 +32,7 @@ import {
 import '../../../test-utils/__mocks__/matchMedia.mock.js';
 import {
   Canvas,
+  ContentMargin,
   DodgedBarChart,
   DonutChart,
   LineChart,
@@ -147,6 +148,52 @@ describe('ChartPopover', () => {
     expect(popover).toHaveStyle('width: auto; min-width: 250px;');
   });
 
+  test('should honor contentMargin', async () => {
+    render(<ContentMargin {...ContentMargin.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
+
+    // Check that the View inside the popover has the correct margin
+    const view = screen.getByTestId('rsc-popover-content');
+    // React Spectrum View applies margin using specific margin-* properties
+    expect(view).toHaveStyle({
+      marginTop: '24px',
+      marginRight: '24px',
+      marginBottom: '24px',
+      marginLeft: '24px',
+    });
+  });
+
+  test('should use default contentMargin when not provided', async () => {
+    render(<StackedBarChart {...StackedBarChart.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());
+
+    // Check that the View inside the popover has the default margin of 12px
+    const view = screen.getByTestId('rsc-popover-content');
+    // React Spectrum View applies margin using specific margin-* properties
+    expect(view).toHaveStyle({
+      marginTop: '12px',
+      marginRight: '12px',
+      marginBottom: '12px',
+      marginLeft: '12px',
+    });
+  });
+
   test('Popover opens on right click, not left click', async () => {
     render(<RightClick {...RightClick.args} />);
 

packages/vega-spec-builder/src/types/dialogs/chartPopoverSpec.types.ts

@@ -32,11 +32,13 @@ export interface ChartPopoverOptions {
   rightClick?: boolean;
   /** Sets which marks should be highlighted when a popover is visible.  This feature is incomplete. */
   UNSAFE_highlightBy?: 'series' | 'dimension' | 'item' | string[];
+  /** The margin that should be applied between the popover and its surrounding container */
+  contentMargin?: number;
 }
 
 type ChartPopoverOptionsWithDefaults = 'UNSAFE_highlightBy';
 
 export interface ChartPopoverSpecOptions
   extends PartiallyRequired<ChartPopoverOptions, ChartPopoverOptionsWithDefaults> {
   markName: string;
-}
+}