fix: Merge mouseleave detection logic (#1374)

cloudscape-design · Jul 28, 2023 · 725901c · 725901c
1 parent 4773c00
commit 725901c
Show file tree

Hide file tree

Showing 7 changed files with 68 additions and 37 deletions.
diff --git a/src/internal/components/chart-popover/index.tsx b/src/internal/components/chart-popover/index.tsx
@@ -98,7 +98,7 @@ function ChartPopover(
   return (
     <div
       {...baseProps}
-      className={clsx(popoverStyles.root, styles.root, baseProps.className, { [styles.dismissable]: dismissButton })}
+      className={clsx(popoverStyles.root, styles.root, baseProps.className)}
       ref={popoverRef}
       onMouseEnter={onMouseEnter}
       onMouseLeave={onMouseLeave}

diff --git a/src/internal/utils/__tests__/dom.test.ts b/src/internal/utils/__tests__/dom.test.ts
@@ -100,6 +100,12 @@ describe('nodeContains', () => {
     expect(nodeContains(null, null)).toBe(false);
   });
 
+  test('returns false if descendant is not a valid node', () => {
+    const div = document.createElement('div');
+    expect(nodeContains(div, window)).toBe(false);
+    expect(nodeContains(div, new EventTarget())).toBe(false);
+  });
+
   const testCases: Record<string, string> = {
     regular: `<div id="parent">
                 <div id="inbetween">
@@ -149,6 +155,8 @@ describe('nodeContains', () => {
           const inbetween = div.querySelector('#inbetween');
           const child = div.querySelector('#child');
 
+          expect(nodeContains(parent, parent)).toBe(true);
+          expect(nodeContains(child, child)).toBe(true);
           expect(nodeContains(parent, child)).toBe(true);
           expect(nodeContains(parent, inbetween)).toBe(true);
           expect(nodeContains(inbetween, child)).toBe(true);

diff --git a/src/internal/utils/dom.ts b/src/internal/utils/dom.ts
@@ -103,8 +103,8 @@ export function parseCssVariable(value: string) {
  * @param parent Parent node
  * @param descendant Node that is checked to be a descendant of the parent node
  */
-export function nodeContains(parent: Node | null, descendant: Node | null) {
-  if (!parent || !descendant) {
+export function nodeContains(parent: Node | null, descendant: Node | EventTarget | null) {
+  if (!parent || !descendant || !(descendant instanceof Node)) {
     return false;
   }
 

diff --git a/src/pie-chart/__tests__/pie-chart.test.tsx b/src/pie-chart/__tests__/pie-chart.test.tsx
@@ -528,6 +528,29 @@ describe('Details popover', () => {
     expect(detailPopover?.findHeader()?.getElement()).toHaveTextContent(defaultData[0].title);
   });
 
+  test('allow mouse to be move between segment and popover ', () => {
+    const { wrapper } = renderPieChart(<PieChart data={defaultData} />);
+    fireEvent.mouseOver(wrapper!.findSegments()[0].getElement());
+
+    expect(wrapper.findDetailPopover()).toBeTruthy();
+
+    fireEvent.mouseOut(wrapper.findDetailPopover()!.getElement(), {
+      relatedTarget: wrapper!.findSegments()[0].getElement(),
+    });
+
+    expect(wrapper.findDetailPopover()).toBeTruthy();
+
+    fireEvent.mouseOut(wrapper!.findSegments()[0].getElement(), {
+      relatedTarget: wrapper.findDetailPopover()!.getElement(),
+    });
+
+    expect(wrapper.findDetailPopover()).toBeTruthy();
+
+    fireEvent.mouseOut(wrapper!.findSegments()[0].getElement(), { relatedTarget: window });
+
+    expect(wrapper.findDetailPopover()).toBeFalsy();
+  });
+
   test('close popover when mouse leaves it ', () => {
     const { wrapper } = renderPieChart(<PieChart data={defaultData} />);
     wrapper.findApplication()!.focus();
@@ -565,13 +588,24 @@ describe('Details popover', () => {
 
   test('dismisses when clicking same segment', () => {
     const { wrapper } = renderPieChart(<PieChart data={defaultData} />);
-    wrapper.findSegments()[0].click();
-    expect(wrapper.findDetailPopover()).toBeDefined();
 
-    wrapper.findSegments()[0].click();
+    fireEvent.mouseDown(wrapper.findSegments()[0].getElement());
+    expect(wrapper.findDetailPopover()).toBeTruthy();
+
+    fireEvent.mouseDown(wrapper.findSegments()[0].getElement());
     expect(wrapper.findDetailPopover()).toBeNull();
   });
 
+  test('stays open with mouseleave events when pinned', () => {
+    const { wrapper } = renderPieChart(<PieChart data={defaultData} />);
+
+    fireEvent.mouseDown(wrapper.findSegments()[0].getElement());
+    expect(wrapper.findDetailPopover()).toBeTruthy();
+
+    fireEvent.mouseOut(wrapper.findSegments()[0].getElement());
+    expect(wrapper.findDetailPopover()).toBeTruthy();
+  });
+
   test('can be dismissed with click on the dismiss button', () => {
     const { wrapper } = renderPieChart(<PieChart data={defaultData} />);
     wrapper.findSegments()[1].click();

diff --git a/src/pie-chart/index.tsx b/src/pie-chart/index.tsx
@@ -87,8 +87,6 @@ const PieChart = function PieChart<T extends PieChartProps.Datum = PieChartProps
     }
   );
 
-  const [pinnedSegment, setPinnedSegment] = useState<T | null>(null);
-
   const visibleData = useMemo(
     () => data.filter(d => visibleSegments?.indexOf(d.datum) !== -1),
     [data, visibleSegments]
@@ -212,8 +210,6 @@ const PieChart = function PieChart<T extends PieChartProps.Datum = PieChartProps
             onHighlightChange={onHighlightChange}
             highlightedSegment={highlightedSegment}
             legendSegment={legendSegment}
-            pinnedSegment={pinnedSegment}
-            setPinnedSegment={setPinnedSegment}
             detailPopoverSize={detailPopoverSize}
             pieData={pieData}
             dataSum={dataSum}

diff --git a/src/pie-chart/pie-chart.tsx b/src/pie-chart/pie-chart.tsx
@@ -22,6 +22,7 @@ import { nodeBelongs } from '../internal/utils/node-belongs';
 import clsx from 'clsx';
 import { useVisualRefresh } from '../internal/hooks/use-visual-mode';
 import { useHeightMeasure } from '../internal/hooks/container-queries/use-height-measure';
+import { nodeContains } from '../internal/utils/dom';
 
 export interface InternalChartDatum<T> {
   index: number;
@@ -42,9 +43,6 @@ interface InternalPieChartProps<T extends PieChartProps.Datum>
 
   legendSegment: T | null;
 
-  pinnedSegment: T | null;
-  setPinnedSegment: React.Dispatch<React.SetStateAction<T | null>>;
-
   pieData: PieArcDatum<InternalChartDatum<T>>[];
   dataSum: number;
 }
@@ -76,11 +74,10 @@ export default <T extends PieChartProps.Datum>({
   highlightedSegment,
   onHighlightChange,
   legendSegment,
-  pinnedSegment,
-  setPinnedSegment,
   pieData,
   dataSum,
 }: InternalPieChartProps<T>) => {
+  const [pinnedSegment, setPinnedSegment] = useState<T | null>(null);
   const plotRef = useRef<ChartPlotRef>(null);
   const containerRef = useRef<HTMLDivElement>(null);
   const focusedSegmentRef = useRef<SVGGElement>(null);
@@ -158,6 +155,21 @@ export default <T extends PieChartProps.Datum>({
     onHighlightChange(null);
   }, [onHighlightChange, setTooltipOpen]);
 
+  const checkMouseLeave = (event: React.MouseEvent) => {
+    if (pinnedSegment !== null) {
+      return;
+    }
+
+    if (
+      nodeContains(popoverRef.current, event.relatedTarget) ||
+      nodeContains(focusedSegmentRef.current, event.relatedTarget)
+    ) {
+      return;
+    }
+
+    clearHighlightedSegment();
+  };
+
   useEffect(() => {
     const onKeyDown = (event: KeyboardEvent) => {
       if (event.key === 'Escape') {
@@ -194,16 +206,7 @@ export default <T extends PieChartProps.Datum>({
     },
     [pinnedSegment, highlightSegment]
   );
-  const onMouseOut = useCallback(
-    (event: React.MouseEvent<SVGElement, MouseEvent>) => {
-      if (pinnedSegment !== null || popoverRef.current?.contains(event.relatedTarget as Node)) {
-        return;
-      }
 
-      clearHighlightedSegment();
-    },
-    [pinnedSegment, clearHighlightedSegment]
-  );
   const onKeyDown = useCallback(
     (event: React.KeyboardEvent) => {
       const keyCode = event.keyCode;
@@ -283,13 +286,6 @@ export default <T extends PieChartProps.Datum>({
     }
   };
 
-  const onPopoverLeave = (event: React.MouseEvent) => {
-    if (pinnedSegment !== null || focusedSegmentRef.current!.contains(event.relatedTarget as Node)) {
-      return;
-    }
-    clearHighlightedSegment();
-  };
-
   return (
     <div
       className={clsx(styles['chart-container'], fitHeight && styles['chart-container--fit-height'])}
@@ -319,7 +315,7 @@ export default <T extends PieChartProps.Datum>({
           onFocus={onFocus}
           onBlur={onBlur}
           onKeyDown={onKeyDown}
-          onMouseOut={onMouseOut}
+          onMouseOut={checkMouseLeave}
         >
           <Segments
             pieData={pieData}
@@ -331,7 +327,6 @@ export default <T extends PieChartProps.Datum>({
             segmentAriaRoleDescription={i18nStrings?.segmentAriaRoleDescription}
             onMouseDown={onMouseDown}
             onMouseOver={onMouseOver}
-            onMouseOut={onMouseOut}
           />
           {hasLabels && (
             <Labels
@@ -385,7 +380,7 @@ export default <T extends PieChartProps.Datum>({
           onDismiss={onPopoverDismiss}
           container={plotRef.current?.svg || null}
           size={detailPopoverSize}
-          onMouseLeave={onPopoverLeave}
+          onMouseLeave={checkMouseLeave}
         >
           {tooltipContent}
           {detailPopoverFooterContent && <InternalBox margin={{ top: 's' }}>{detailPopoverFooterContent}</InternalBox>}

diff --git a/src/pie-chart/segments.tsx b/src/pie-chart/segments.tsx
@@ -20,7 +20,6 @@ interface SegmentsProps<T> {
   segmentAriaRoleDescription?: string;
   onMouseDown: (datum: InternalChartDatum<T>) => void;
   onMouseOver: (datum: InternalChartDatum<T>) => void;
-  onMouseOut: (event: React.MouseEvent<SVGElement>) => void;
 }
 
 export default function Segments<T extends PieChartProps.Datum>({
@@ -33,7 +32,6 @@ export default function Segments<T extends PieChartProps.Datum>({
   segmentAriaRoleDescription,
   onMouseDown,
   onMouseOver,
-  onMouseOut,
 }: SegmentsProps<T>) {
   const i18n = useInternalI18n('pie-chart');
 
@@ -68,7 +66,7 @@ export default function Segments<T extends PieChartProps.Datum>({
   }, [highlightedSegment, pieData, arcFactory]);
 
   return (
-    <g onMouseLeave={event => onMouseOut(event)}>
+    <g>
       {pieData.map(datum => {
         const isHighlighted = highlightedSegment === datum.data.datum;
         const isDimmed = highlightedSegment !== null && !isHighlighted;