chore: Fixes mixed charts popover behaviour for React 18 (#3851)

Author: pan-kot

src/mixed-line-bar-chart/__integ__/announcements.test.ts

@@ -37,9 +37,9 @@ describe('Popover content is announced as plain text on hover', () => {
     test(
       'with expandable sub-items',
       setupTest(`#/light/mixed-line-bar-chart/drilldown?useLinks=${useLinks}&expandableSubItems=true`, async page => {
-        const coordinateIndex = 3;
         const wrapper = createWrapper().findMixedLineBarChart();
-        const barGroup = wrapper.findBarGroups().get(coordinateIndex).toSelector();
+        const barGroup = wrapper.findBarGroups().get(3).toSelector();
+        const nextBarGroup = wrapper.findBarGroups().get(4).toSelector();
         const getLabel = () => page.getElementAttribute(barGroup, 'aria-label');
         await page.hoverElement(barGroup);
         await page.waitForAssertion(async () => {
@@ -56,11 +56,11 @@ describe('Popover content is announced as plain text on hover', () => {
         await page.click(
           wrapper.findDetailPopover().findContent().findExpandableSection().findExpandButton().toSelector()
         );
-        // Pin and dismiss the poover,
+        // Pin and dismiss the popover,
         // then hover over a different item and come back to hover the initial one.
         await page.clickBarGroup(barGroup);
         await page.click(wrapper.findDetailPopover().findDismissButton().toSelector());
-        await page.hoverElement(wrapper.findBarGroups().get(4).toSelector());
+        await page.hoverElement(nextBarGroup);
         await page.hoverElement(barGroup);
         await page.waitForAssertion(async () => {
           const label = await getLabel();

src/mixed-line-bar-chart/__integ__/common.ts

@@ -29,6 +29,8 @@ export class MixedChartPage extends BasePageObject {
       // If a popover was pinned, we need to unpin it before pinning another one.
       if (this.currentIndex) {
         await this.click(this.wrapper.findDetailPopover().findDismissButton().toSelector());
+        // Wait for popover dismiss reopen delay.
+        await this.pause(50);
       }
       this.currentIndex = index;
       await this.clickBarGroup(barGroup);

src/mixed-line-bar-chart/__tests__/keyboard-navigation.test.tsx

@@ -4,14 +4,33 @@
 import React from 'react';
 import { render } from '@testing-library/react';
 
+import { createWrapper } from '@cloudscape-design/test-utils-core/dom';
+
 import { KeyCode } from '../../../lib/components/internal/keycode';
 import MixedLineBarChart from '../../../lib/components/mixed-line-bar-chart';
-import { MixedLineBarChartWrapper } from '../../../lib/components/test-utils/dom';
 import { barSeries, lineSeries3, thresholdSeries } from './common';
 
 describe('Keyboard navigation', () => {
-  test('opens popover for each series', () => {
-    const { container } = render(
+  function getChart() {
+    return createWrapper().findMixedLineBarChart()!;
+  }
+  function expectValues(a: Array<number>) {
+    for (let i = 0; i < a.length; i++) {
+      const value = a[i];
+      expect(getChart().findDetailPopover()!.findSeries()![i].findValue().getElement()).toHaveTextContent(
+        value.toString()
+      );
+    }
+  }
+  function focusApplication() {
+    getChart().findApplication()!.focus();
+  }
+  function goToNextDataPoint() {
+    getChart().findApplication()!.keydown(KeyCode.right);
+  }
+
+  test('opens popover for each series (mixed)', () => {
+    render(
       <MixedLineBarChart
         height={250}
         xDomain={['Potatoes', 'Chocolate', 'Apples', 'Oranges']}
@@ -20,23 +39,7 @@ describe('Keyboard navigation', () => {
         series={[barSeries, lineSeries3, thresholdSeries]}
       />
     );
-
-    const chart = new MixedLineBarChartWrapper(container);
-    const application = chart.findApplication()!;
-
-    const expectValues = (a: Array<number>) => {
-      for (let i = 0; i < a.length; i++) {
-        const value = a[i];
-        expect(chart.findDetailPopover()!.findSeries()![i].findValue().getElement()).toHaveTextContent(
-          value.toString()
-        );
-      }
-    };
-
-    const goToNextDataPoint = () => application.keydown(KeyCode.right);
-
-    application.focus(); // Focusing the application opens the popover
-
+    focusApplication(); // Focusing the application opens the popover
     expectValues([77, 7, 8]);
     goToNextDataPoint();
     expectValues([546, 5, 8]);
@@ -45,4 +48,24 @@ describe('Keyboard navigation', () => {
     goToNextDataPoint();
     expectValues([47, 7, 8]);
   });
+
+  test('opens popover for each series (line)', () => {
+    render(
+      <MixedLineBarChart
+        height={250}
+        xDomain={['Potatoes', 'Chocolate', 'Apples', 'Oranges']}
+        yDomain={[0, 10]}
+        xScaleType="categorical"
+        series={[lineSeries3]}
+      />
+    );
+    focusApplication(); // Focusing the application opens the popover
+    expectValues([7]);
+    goToNextDataPoint();
+    expectValues([5]);
+    goToNextDataPoint();
+    expectValues([9]);
+    goToNextDataPoint();
+    expectValues([7]);
+  });
 });

src/mixed-line-bar-chart/chart-container.tsx

@@ -1,6 +1,6 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
-import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react';
+import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react';
 
 import { getIsRtl, useMergeRefs } from '@cloudscape-design/component-toolkit/internal';
 
@@ -223,7 +223,7 @@ export default function ChartContainer<T extends ChartDataTypes>({
   const scaledSeries = makeScaledSeries(visibleSeries, xAxisProps.scale, yAxisProps.scale);
   const barGroups: ScaledBarGroup<T>[] = makeScaledBarGroups(visibleSeries, xAxisProps.scale, plotWidth, plotHeight, y);
 
-  const { isPopoverOpen, isPopoverPinned, showPopover, pinPopover, dismissPopover } = usePopover();
+  const { isPopoverOpen, isPopoverPinned, showPopover, pinPopover, hidePopover, dismissPopover } = usePopover();
 
   // Allows to add a delay between popover is dismissed and handlers are enabled to prevent immediate popover reopening.
   const [isHandlersDisabled, setHandlersDisabled] = useState(false);
@@ -251,13 +251,11 @@ export default function ChartContainer<T extends ChartDataTypes>({
       setHighlightedPoint(point);
       if (point) {
         highlightSeries(point.series);
-        setVerticalMarkerX({
-          scaledX: point.x,
-          label: point.datum?.x ?? null,
-        });
+        setVerticalMarkerX({ scaledX: point.x, label: point.datum?.x ?? null });
+        showPopover();
       }
     },
-    [setHighlightedGroupIndex, setHighlightedPoint, highlightSeries]
+    [setHighlightedGroupIndex, setHighlightedPoint, highlightSeries, showPopover]
   );
 
   const clearAllHighlights = useCallback(() => {
@@ -273,8 +271,9 @@ export default function ChartContainer<T extends ChartDataTypes>({
         clearAllHighlights();
       }
       setVerticalMarkerX(marker);
+      showPopover();
     },
-    [clearAllHighlights]
+    [clearAllHighlights, showPopover]
   );
 
   // Highlight all points and bars at a given X index in a mixed line and bar chart
@@ -283,14 +282,15 @@ export default function ChartContainer<T extends ChartDataTypes>({
       highlightSeries(null);
       setHighlightedPoint(null);
       setHighlightedGroupIndex(groupIndex);
+      showPopover();
     },
-    [highlightSeries, setHighlightedPoint, setHighlightedGroupIndex]
+    [highlightSeries, setHighlightedPoint, setHighlightedGroupIndex, showPopover]
   );
 
   const clearHighlightedSeries = useCallback(() => {
     clearAllHighlights();
-    dismissPopover();
-  }, [dismissPopover, clearAllHighlights]);
+    hidePopover();
+  }, [hidePopover, clearAllHighlights]);
 
   const { isGroupNavigation, ...handlers } = useNavigation({
     series,
@@ -349,12 +349,6 @@ export default function ChartContainer<T extends ChartDataTypes>({
     return () => document.removeEventListener('keydown', onKeyDown);
   }, [dismissPopover]);
 
-  useLayoutEffect(() => {
-    if (highlightedX !== null || highlightedPoint !== null) {
-      showPopover();
-    }
-  }, [highlightedX, highlightedPoint, showPopover]);
-
   const onPopoverDismiss = (outsideClick?: boolean) => {
     dismissPopover();
 

src/mixed-line-bar-chart/hooks/use-popover.ts

@@ -1,16 +1,31 @@
 // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
-import { useCallback, useState } from 'react';
+
+import { useCallback, useRef, useState } from 'react';
+
+// The delay prevents re-opening popover immediately upon dismissing,
+// so that the popover actually closes. It can be reopened with the next
+// hover or focus event that occurs after the delay.
+const REOPEN_DELAY_MS = 50;
 
 export function usePopover() {
+  const dismissedTimeRef = useRef(Date.now() - REOPEN_DELAY_MS);
   const [state, setState] = useState<'open' | 'closed' | 'pinned'>('closed');
 
   const isPopoverOpen = state !== 'closed';
   const isPopoverPinned = state === 'pinned';
 
-  const showPopover = useCallback(() => setState('open'), []);
+  const showPopover = useCallback(() => {
+    if (Date.now() - dismissedTimeRef.current > REOPEN_DELAY_MS) {
+      setState('open');
+    }
+  }, []);
   const pinPopover = useCallback(() => setState('pinned'), []);
-  const dismissPopover = useCallback(() => setState('closed'), []);
+  const hidePopover = useCallback(() => setState('closed'), []);
+  const dismissPopover = useCallback(() => {
+    setState('closed');
+    dismissedTimeRef.current = Date.now();
+  }, []);
 
-  return { isPopoverOpen, isPopoverPinned, showPopover, pinPopover, dismissPopover };
+  return { isPopoverOpen, isPopoverPinned, showPopover, pinPopover, hidePopover, dismissPopover };
 }