Merge pull request #573 from adobe/fixHideShowRightCLick

Fix hide show right click

Author: tanugarg04

packages/react-spectrum-charts/src/context/RscChartContext.tsx

@@ -30,11 +30,6 @@ interface ChartContextValue {
 	setIsPopoverOpen: (isOpen: boolean) => void;
 	popoverAnchorRef: MutableRefObject<HTMLDivElement | null>;
 
-	// Interaction handlers
-	onLegendClick?: (seriesName: string) => void;
-	onLegendMouseOver?: (seriesName: string) => void;
-	onLegendMouseOut?: (seriesName: string) => void;
-
 	// Spec state
 	controlledHoveredIdSignal: MutableRefObject<Signal | undefined>;
 	controlledHoveredGroupSignal: MutableRefObject<Signal | undefined>;

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

@@ -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 { LegendBarHiddenSeriesStory, LegendBarStory, defaultProps } from './LegendStoryUtils';
 
 export default {
@@ -18,7 +18,17 @@ export default {
 };
 
 const DefaultHiddenSeries = LegendBarStory.bind({});
-DefaultHiddenSeries.args = { defaultHiddenSeries: ['Other'], isToggleable: true, highlight: true, ...defaultProps };
+DefaultHiddenSeries.args = {
+	defaultHiddenSeries: ['Other'],
+	isToggleable: true,
+	highlight: true,
+	children: (
+		<ChartPopover rightClick width="auto">
+			{(datum) => <div>{datum.value}</div>}
+		</ChartPopover>
+	),
+	...defaultProps,
+};
 DefaultHiddenSeries.storyName = 'Default Hidden Series (uncontrolled)';
 
 const HiddenSeries = LegendBarHiddenSeriesStory.bind({});

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

@@ -22,6 +22,7 @@ import {
 	getAllLegendSymbols,
 	hoverNthElement,
 	render,
+	rightClickNthElement,
 	screen,
 } from '../../../test-utils';
 import '../../../test-utils/__mocks__/matchMedia.mock';
@@ -120,3 +121,22 @@ test('Hidden series should not highlight any marks', async () => {
 	bars = await findAllMarksByGroupName(chart, 'bar0');
 	expect(bars[0]).toHaveAttribute('opacity', '1');
 });
+
+test('Right clicking on a legend entry should not hide the series or trigger onClick', async () => {
+	const onLegendClick = jest.fn();
+	render(<DefaultHiddenSeries {...DefaultHiddenSeries.args} onClick={onLegendClick} />);
+	const chart = await findChart();
+
+	let bars = await findAllMarksByGroupName(chart, 'bar0');
+	// there should only be 6 bars (2 series 3 categories)
+	expect(bars.length).toEqual(6);
+
+	const entries = getAllLegendEntries(chart);
+	await rightClickNthElement(entries, 0);
+
+	bars = await findAllMarksByGroupName(chart, 'bar0');
+	// there should still be 6 bars (2 series 3 categories)
+	expect(bars.length).toEqual(6);
+
+	expect(onLegendClick).not.toHaveBeenCalled();
+});

packages/react-spectrum-charts/src/utils/markClickUtils.test.ts

@@ -41,6 +41,13 @@ describe('getItemBounds()', () => {
 
 describe('handleLegendItemClick()', () => {
 	let setHiddenSeries;
+	const item = {
+		context: null,
+		height: null,
+		width: null,
+		items: [{ role: 'legend-label', bounds: null, clip: null, items: [{ datum: { value: 'test' } }] }],
+	} as unknown as Item;
+
 	beforeEach(() => {
 		setHiddenSeries = jest.fn();
 	});
@@ -49,12 +56,6 @@ describe('handleLegendItemClick()', () => {
 	});
 
 	test('should call setHiddenSeries if legendItemValue is found', () => {
-		const item = {
-			context: null,
-			height: null,
-			width: null,
-			items: [{ role: 'legend-label', bounds: null, clip: null, items: [{ datum: { value: 'test' } }] }],
-		} as unknown as Item;
 		handleLegendItemClick(item, { ...defaultMarkClickArgs, setHiddenSeries, legendIsToggleable: true });
 		expect(setHiddenSeries).toHaveBeenCalled();
 	});
@@ -63,6 +64,11 @@ describe('handleLegendItemClick()', () => {
 		handleLegendItemClick(item, { ...defaultMarkClickArgs, setHiddenSeries, legendIsToggleable: true });
 		expect(setHiddenSeries).not.toHaveBeenCalled();
 	});
+	test('should call onLegendClick if trigger is click', () => {
+		const onLegendClick = jest.fn();
+		handleLegendItemClick(item, { ...defaultMarkClickArgs, onLegendClick, trigger: 'click' });
+		expect(onLegendClick).toHaveBeenCalled();
+	});
 });
 
 describe('getLegendItemValue', () => {

packages/react-spectrum-charts/src/utils/markClickUtils.ts

@@ -204,9 +204,12 @@ export const handleLegendItemClick = (
 		triggerPopover(chartId, itemName, trigger);
 	}
 
-	onLegendClick?.(legendItemValue);
-	if (!legendIsToggleable) return;
-	setHiddenSeries(toggleStringArrayValue(hiddenSeries, legendItemValue));
+	if (trigger === 'click') {
+		onLegendClick?.(legendItemValue);
+		if (legendIsToggleable) {
+			setHiddenSeries(toggleStringArrayValue(hiddenSeries, legendItemValue));
+		}
+	}
 };
 
 /**