Skip to content
Merged
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
13 changes: 7 additions & 6 deletions packages/react-spectrum-charts/src/RscChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
*/
import { MutableRefObject, forwardRef, useEffect, useMemo } from 'react';

import { SymbolShape } from 'vega';

import { ActionButton, Dialog, DialogTrigger, View as SpectrumView } from '@adobe/react-spectrum';
import { COMPONENT_NAME, DEFAULT_SYMBOL_SHAPES, DEFAULT_SYMBOL_SIZES } from '@spectrum-charts/constants';
import { getChartConfig } from '@spectrum-charts/themes';
Expand Down Expand Up @@ -56,7 +54,7 @@ export const RscChart = forwardRef<ChartHandle, RscChartProps>((props, forwarded
opacities,
padding,
renderer,
symbolShapes = DEFAULT_SYMBOL_SHAPES as SymbolShape[],
symbolShapes = DEFAULT_SYMBOL_SHAPES,
symbolSizes = DEFAULT_SYMBOL_SIZES as [SymbolSize, SymbolSize],
title,
UNSAFE_vegaSpec,
Expand Down Expand Up @@ -146,7 +144,7 @@ RscChart.displayName = 'RscChart';

const ChartDialog = ({ datum, popover, setIsPopoverOpen, targetElement }: ChartDialogProps) => {
const { chartPopoverProps, name } = popover;
const { children, onOpenChange, containerPadding, ...dialogProps } = chartPopoverProps;
const { children, onOpenChange, containerPadding, rightClick, ...dialogProps } = chartPopoverProps;
const minWidth = dialogProps.minWidth ?? 0;

return (
Expand All @@ -162,8 +160,11 @@ const ChartDialog = ({ datum, popover, setIsPopoverOpen, targetElement }: ChartD
hideArrow
containerPadding={containerPadding}
>
<ActionButton id={`${name}-button`} UNSAFE_style={{ display: 'none' }}>
launch chart popover
<ActionButton
id={`${name}-${rightClick ? 'contextmenu' : 'popover'}-button`}
UNSAFE_style={{ display: 'none' }}
>
{rightClick ? 'launch chart context menu' : 'launch chart popover'}
</ActionButton>
{(close) => (
<Dialog data-testid="rsc-popover" UNSAFE_className="rsc-popover" {...dialogProps} minWidth={minWidth}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const ChartPopover: FC<ChartPopoverProps> = ({
maxHeight,
containerPadding = 12,
onOpenChange,
rightClick = false,
UNSAFE_highlightBy = 'item',
}) => {
return null;
Expand Down
10 changes: 1 addition & 9 deletions packages/react-spectrum-charts/src/context/RscChartContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,6 @@ interface ChartContextValue {
setIsPopoverOpen: (isOpen: boolean) => void;
popoverAnchorRef: MutableRefObject<HTMLDivElement | null>;

// Legend state
legendHiddenSeries: string[];
setLegendHiddenSeries: (series: string[]) => void;
// legendIsToggleable: boolean;

// Interaction handlers
onLegendClick?: (seriesName: string) => void;
onLegendMouseOver?: (seriesName: string) => void;
Expand Down Expand Up @@ -62,7 +57,6 @@ export const ChartProvider = ({ children, chartId, chartView }: ChartProviderPro
const selectedDataBounds = useRef<MarkBounds>({ x1: 0, x2: 0, y1: 0, y2: 0 });

const [isPopoverOpen, setIsPopoverOpen] = useState(false);
const [legendHiddenSeries, setLegendHiddenSeries] = useState<string[]>([]);

const value: ChartContextValue = useMemo(
() => ({
Expand All @@ -76,10 +70,8 @@ export const ChartProvider = ({ children, chartId, chartView }: ChartProviderPro
isPopoverOpen,
setIsPopoverOpen,
popoverAnchorRef,
legendHiddenSeries,
setLegendHiddenSeries,
}),
[chartId, chartView, isPopoverOpen, legendHiddenSeries]
[chartId, chartView, isPopoverOpen]
);

return <ChartContext.Provider value={value}>{children}</ChartContext.Provider>;
Expand Down
1 change: 1 addition & 0 deletions packages/react-spectrum-charts/src/hooks/useLegend.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export default function useLegend(children: ChartChildElement[]): UseLegendProps
return getElement(createElement(ChartContainer, undefined, children), Legend);
}, [children]) as LegendElement;
const [legendHiddenSeries, setLegendHiddenSeries] = useState<string[]>(legend?.props?.defaultHiddenSeries ?? []);

if (!legend) return { legendHiddenSeries, setLegendHiddenSeries };
const { descriptions, isToggleable, onClick, onMouseOut, onMouseOver } = legend.props;
return { legendHiddenSeries, setLegendHiddenSeries, descriptions, isToggleable, onClick, onMouseOut, onMouseOver };
Expand Down
24 changes: 23 additions & 1 deletion packages/react-spectrum-charts/src/hooks/useNewChartView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,30 @@ const useNewChartView = (
setHiddenSeries: setLegendHiddenSeries,
legendIsToggleable,
onLegendClick,
trigger: 'click',
})
);
if (popovers.some((p) => p.chartPopoverProps.rightClick)) {
const chartContainer = document.querySelector(`#${chartId}`);
if (chartContainer) {
chartContainer.addEventListener('contextmenu', (e) => e.preventDefault());
}
view.addEventListener(
'contextmenu',
getOnMarkClickCallback({
chartView,
hiddenSeries: legendHiddenSeries,
chartId,
selectedData,
selectedDataBounds,
selectedDataName,
setHiddenSeries: setLegendHiddenSeries,
legendIsToggleable,
onLegendClick,
trigger: 'contextmenu',
})
);
}
}
view.addEventListener('click', getOnChartMarkClickCallback(chartView, markClickDetails));
view.addEventListener('mouseover', getOnMouseInputCallback(onLegendMouseOver));
Expand All @@ -104,7 +126,7 @@ const useNewChartView = (
onLegendClick,
onLegendMouseOut,
onLegendMouseOver,
popovers.length,
popovers,
selectedData,
selectedDataBounds,
selectedDataName,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,14 @@ describe('rscPropsToSpecBuilderOptions()', () => {
colors: 'categorical12',
hiddenSeries: [],
idKey: 'rscMarkId',
legends: [{ hasMouseInteraction: false, hasOnClick: false, lineWidth: { value: 0 } }],
legends: [
{
chartPopovers: [],
hasMouseInteraction: false,
hasOnClick: false,
lineWidth: { value: 0 },
},
],
lineTypes: ['solid', 'dashed', 'dotted', 'dotDash', 'longDash', 'twoDash'],
lineWidths: ['M'],
marks: [
Expand Down Expand Up @@ -285,7 +292,15 @@ describe('rscPropsToSpecBuilderOptions()', () => {
colors: 'categorical12',
hiddenSeries: [],
idKey: 'rscMarkId',
legends: [{ hasMouseInteraction: false, hasOnClick: false, highlight: true, position: 'bottom' }],
legends: [
{
chartPopovers: [],
hasMouseInteraction: false,
hasOnClick: false,
highlight: true,
position: 'bottom',
},
],
lineTypes: ['dotted', 'solid'],
lineWidths: ['M'],
marks: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/
import { createElement } from 'react';

import { DEFAULT_COLOR } from '@spectrum-charts/constants';

import { ChartPopover } from '../components/ChartPopover';
import { getLegendOptions } from './legendAdapter';

describe('getLegendOptions()', () => {
Expand All @@ -37,6 +40,10 @@ describe('getLegendOptions()', () => {
false
);
});
it('should convert popover children to chartPopovers array', () => {
const options = getLegendOptions({ children: [createElement(ChartPopover)] });
expect(options.chartPopovers).toHaveLength(1);
});
it('should pass through included props', () => {
const options = getLegendOptions({ color: DEFAULT_COLOR });
expect(options).toHaveProperty('color', DEFAULT_COLOR);
Expand Down
22 changes: 17 additions & 5 deletions packages/react-spectrum-charts/src/rscToSbAdapter/legendAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,21 @@
import { LegendOptions } from '@spectrum-charts/vega-spec-builder';

import { LegendProps } from '../types';
import { childrenToOptions } from './childrenAdapter';

export const getLegendOptions = ({ onClick, onMouseOut, onMouseOver, ...legendProps }: LegendProps): LegendOptions => ({
...legendProps,
hasOnClick: Boolean(onClick),
hasMouseInteraction: Boolean(onMouseOut || onMouseOver),
});
export const getLegendOptions = ({
children,
onClick,
onMouseOut,
onMouseOver,
...legendProps
}: LegendProps): LegendOptions => {
const { chartPopovers } = childrenToOptions(children);

return {
...legendProps,
hasOnClick: Boolean(onClick),
hasMouseInteraction: Boolean(onMouseOut || onMouseOver),
chartPopovers,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -137,34 +137,49 @@ const DonutStory: StoryFn<typeof ChartPopover> = (args): ReactElement => {
);
};

const AreaChart = bindWithProps(AreaStory);
AreaChart.args = { children: dialogContent, width: 'auto' };

const Canvas = bindWithProps(ChartPopoverCanvasStory);
Canvas.args = { children: dialogContent, width: 'auto' };

const Svg = bindWithProps(ChartPopoverSvgStory);
Svg.args = { children: dialogContent, width: 'auto' };
const DodgedBarChart = bindWithProps(ChartPopoverDodgedBarStory);
DodgedBarChart.args = { children: dialogContent, width: 'auto' };

const Size = bindWithProps(ChartPopoverSvgStory);
Size.args = { children: dialogContent, width: 200, height: 100 };
const DonutChart = bindWithProps(DonutStory);
DonutChart.args = { children: donutDialogContent, width: 'auto' };

const LineChart = bindWithProps(LineStory);
LineChart.args = { children: dialogContent, width: 'auto' };

const MinWidth = bindWithProps(ChartPopoverSvgStory);
MinWidth.args = { children: dialogContent, width: 'auto', minWidth: 250 };

const OnOpenChange = bindWithProps(ChartPopoverSvgStory);
OnOpenChange.args = { children: dialogContent, width: 'auto' };

const AreaChart = bindWithProps(AreaStory);
AreaChart.args = { children: dialogContent, width: 'auto' };
const RightClick = bindWithProps(ChartPopoverSvgStory);
RightClick.args = { children: dialogContent, width: 'auto', rightClick: true };

const DodgedBarChart = bindWithProps(ChartPopoverDodgedBarStory);
DodgedBarChart.args = { children: dialogContent, width: 'auto' };

const LineChart = bindWithProps(LineStory);
LineChart.args = { children: dialogContent, width: 'auto' };
const Size = bindWithProps(ChartPopoverSvgStory);
Size.args = { children: dialogContent, width: 200, height: 100 };

const StackedBarChart = bindWithProps(ChartPopoverSvgStory);
StackedBarChart.args = { children: dialogContent, width: 'auto' };

const DonutChart = bindWithProps(DonutStory);
DonutChart.args = { children: donutDialogContent, width: 'auto' };
const Svg = bindWithProps(ChartPopoverSvgStory);
Svg.args = { children: dialogContent, width: 'auto' };

export { AreaChart, Canvas, DodgedBarChart, DonutChart, LineChart, MinWidth, OnOpenChange, Size, StackedBarChart, Svg };
export {
AreaChart,
Canvas,
DodgedBarChart,
DonutChart,
LineChart,
MinWidth,
OnOpenChange,
RightClick,
Size,
StackedBarChart,
Svg,
};
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
findMarksByGroupName,
getAllMarksByGroupName,
render,
rightClickNthElement,
screen,
waitFor,
within,
Expand All @@ -37,6 +38,7 @@ import {
LineChart,
MinWidth,
OnOpenChange,
RightClick,
Size,
StackedBarChart,
Svg,
Expand Down Expand Up @@ -146,6 +148,32 @@ describe('ChartPopover', () => {
expect(popover).toHaveStyle('width: auto; min-width: 250px;');
});

test('Popover opens on right click, not left click', async () => {
render(<RightClick {...RightClick.args} />);

const chart = await findChart();
expect(chart).toBeInTheDocument();
const bars = getAllMarksByGroupName(chart, 'bar0');

// left clicking the bar should not open the popover
await clickNthElement(bars, 0);
let popover = screen.queryByTestId('rsc-popover');
await waitFor(() => expect(popover).not.toBeInTheDocument());

// clicking the bar should open the popover
await rightClickNthElement(bars, 0);
popover = await screen.findByTestId('rsc-popover');
await waitFor(() => expect(popover).toBeInTheDocument()); // waitFor to give the popover time to make sure it doesn't close

// shouldn't close the popover
await userEvent.click(popover);
expect(popover).toBeInTheDocument();

// should close the popover
await userEvent.click(chart);
await waitFor(() => expect(popover).not.toBeInTheDocument());
});

test('Line popover opens and closes corectly when clicking on the chart', async () => {
render(<LineChart {...LineChart.args} />);
// validate that the line drew
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.
*/
import { Legend } from '../../../components';
import { ChartPopover, Legend } from '../../../components';
import { LegendBarStory, LegendDisconnectedStory, defaultProps } from './LegendStoryUtils';

export default {
Expand Down Expand Up @@ -56,6 +56,12 @@ LabelLimit.args = { legendLabels: truncatedLegendLabels, ...defaultProps };
const OnClick = LegendBarStory.bind({});
OnClick.args = {};

const Popover = LegendBarStory.bind({});
Popover.args = {
children: <ChartPopover width="auto">{(datum) => <div>{datum.value}</div>}</ChartPopover>,
...defaultProps,
};

const Position = LegendBarStory.bind({});
Position.args = { position: 'right', ...defaultProps };

Expand All @@ -71,4 +77,4 @@ Supreme.args = {
title: 'Operating system',
};

export { Basic, Descriptions, Disconnected, Labels, LabelLimit, OnClick, Position, Title, Supreme };
export { Basic, Descriptions, Disconnected, Labels, LabelLimit, OnClick, Popover, Position, Title, Supreme };
Loading