wip: improve the working branch

e2e-playwright/various-suite/pie-chart.spec.ts

@@ -17,5 +17,9 @@ test.describe(
       );
       await expect(pieChartSlices).toHaveCount(5);
     });
+
+    describe('keyboard accessibility', () => {
+      // FIXME: DATALINKS ACCESSIBILITY TESTS HERE
+    });
   }
 );

packages/grafana-ui/src/components/ContextMenu/WithContextMenu.test.tsx

@@ -0,0 +1,174 @@
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+
+import { MenuItem, MenuGroup } from '@grafana/ui';
+
+import { WithContextMenu } from './WithContextMenu';
+
+describe('WithContextMenu', () => {
+  it('supports mouse events', async () => {
+    render(
+      <WithContextMenu
+        renderMenuItems={() => (
+          <>
+            <MenuGroup>
+              <MenuItem label="Item 1" />
+              <MenuItem label="Item 2" />
+            </MenuGroup>
+          </>
+        )}
+      >
+        {({ openMenu }) => (
+          <div data-testid="context-menu-target" onClick={openMenu}>
+            Click me
+          </div>
+        )}
+      </WithContextMenu>
+    );
+
+    expect(screen.getByTestId('context-menu-target')).toBeInTheDocument();
+    expect(screen.queryByText('Item 1')).not.toBeInTheDocument();
+    expect(screen.queryByText('Item 2')).not.toBeInTheDocument();
+
+    // Simulate click to open context menu
+    await userEvent.click(screen.getByTestId('context-menu-target'));
+
+    expect(screen.getByText('Item 1')).toBeInTheDocument();
+    expect(screen.getByText('Item 2')).toBeInTheDocument();
+  });
+
+  // FIXME: this test isn't correct yet, probably because of how I've done the user-events wrong
+  it('supports keyboard events', async () => {
+    class DOMRect {
+      public get top(): number {
+        return this.y;
+      }
+      public get left(): number {
+        return this.x;
+      }
+      public get bottom(): number {
+        return this.y + this.height;
+      }
+      public get right(): number {
+        return this.x + this.width;
+      }
+      constructor(
+        public x = 0,
+        public y = 0,
+        public width = 0,
+        public height = 0
+      ) {
+        this.x = x;
+        this.y = y;
+        this.width = width;
+        this.height = height;
+      }
+      static fromRect(other: DOMRect) {
+        return new DOMRect(other.x, other.y, other.width, other.height);
+      }
+      toJSON() {
+        return JSON.stringify(this);
+      }
+    }
+
+    render(
+      <WithContextMenu
+        renderMenuItems={() => (
+          <>
+            <MenuGroup>
+              <MenuItem label="Item 1" />
+              <MenuItem label="Item 2" />
+            </MenuGroup>
+          </>
+        )}
+      >
+        {({ openMenu }) => (
+          <div
+            data-testid="context-menu-target"
+            tabIndex={0}
+            onKeyDown={(ev) => {
+              ev.preventDefault();
+              openMenu(ev);
+            }}
+          >
+            Press enter on me
+          </div>
+        )}
+      </WithContextMenu>
+    );
+
+    expect(screen.getByTestId('context-menu-target')).toBeInTheDocument();
+    expect(screen.queryByText('Item 1')).not.toBeInTheDocument();
+    expect(screen.queryByText('Item 2')).not.toBeInTheDocument();
+
+    const target = screen.getByTestId('context-menu-target');
+
+    // Simulate key down to open context menu
+    await userEvent.type(target, ' ');
+
+    expect(screen.getByText('Item 1')).toBeInTheDocument();
+    expect(screen.getByText('Item 2')).toBeInTheDocument();
+  });
+
+  it('supports explicit positioning', async () => {
+    render(
+      <WithContextMenu
+        renderMenuItems={() => (
+          <>
+            <MenuGroup>
+              <MenuItem label="Item 1" />
+              <MenuItem label="Item 2" />
+            </MenuGroup>
+          </>
+        )}
+      >
+        {({ openMenu }) => (
+          <div data-testid="context-menu-target" onClick={(ev) => openMenu({ x: ev.pageX, y: ev.pageY })}>
+            Click me
+          </div>
+        )}
+      </WithContextMenu>
+    );
+
+    expect(screen.getByTestId('context-menu-target')).toBeInTheDocument();
+    expect(screen.queryByText('Item 1')).not.toBeInTheDocument();
+    expect(screen.queryByText('Item 2')).not.toBeInTheDocument();
+
+    // Simulate key down to open context menu
+    await userEvent.click(screen.getByTestId('context-menu-target'));
+
+    expect(screen.getByText('Item 1')).toBeInTheDocument();
+    expect(screen.getByText('Item 2')).toBeInTheDocument();
+  });
+
+  it('does not open menu when openMenu is called with undefined', async () => {
+    render(
+      <WithContextMenu
+        renderMenuItems={() => (
+          <>
+            <MenuGroup>
+              <MenuItem label="Item 1" />
+              <MenuItem label="Item 2" />
+            </MenuGroup>
+          </>
+        )}
+      >
+        {({ openMenu }) => (
+          <div data-testid="context-menu-target" onClick={() => openMenu(undefined)}>
+            Click me
+          </div>
+        )}
+      </WithContextMenu>
+    );
+
+    expect(screen.getByTestId('context-menu-target')).toBeInTheDocument();
+    expect(screen.queryByText('Item 1')).not.toBeInTheDocument();
+    expect(screen.queryByText('Item 2')).not.toBeInTheDocument();
+
+    // Simulate click to open context menu
+    await userEvent.click(screen.getByTestId('context-menu-target'));
+
+    expect(screen.queryByText('Item 1')).not.toBeInTheDocument();
+    expect(screen.queryByText('Item 2')).not.toBeInTheDocument();
+  });
+});

packages/grafana-ui/src/components/ContextMenu/WithContextMenu.tsx

@@ -3,9 +3,23 @@ import * as React from 'react';
 
 import { ContextMenu } from '../ContextMenu/ContextMenu';
 
+/**
+ * This callback supports several ways to provide the x/y coordinates to open the context menu:
+ * - MouseEvent, to open the menu at the mouse position
+ * - SyntheticEvent, to open the menu at the location of the currentTarget element for non-mouse events
+ * - An object with x and y coordinates to open the menu at a specific position, for other use-cases
+ */
+export type WithContextMenuOpenMenuCallback = (
+  e:
+    | React.MouseEvent<HTMLElement | SVGElement>
+    | React.SyntheticEvent<HTMLElement | SVGElement>
+    | { x: number; y: number }
+    | undefined
+) => void;
+
 export interface WithContextMenuProps {
   /** Menu item trigger that accepts openMenu prop */
-  children: (props: { openMenu: React.MouseEventHandler<HTMLElement> }) => JSX.Element;
+  children: (props: { openMenu: WithContextMenuOpenMenuCallback }) => JSX.Element;
   /** A function that returns an array of menu items */
   renderMenuItems: () => React.ReactNode;
   /** On menu open focus the first element */
@@ -15,39 +29,43 @@ export interface WithContextMenuProps {
 export const WithContextMenu = ({ children, renderMenuItems, focusOnOpen = true }: WithContextMenuProps) => {
   const [isMenuOpen, setIsMenuOpen] = useState(false);
   const [menuPosition, setMenuPosition] = useState({ x: 0, y: 0 });
-  
-  const handleOpenMenu = React.useCallback(
-    (e: React.MouseEvent<HTMLElement> | { x: number; y: number } | HTMLElement | SVGElement) => {
-      setIsMenuOpen(true);
-      if (e && 'pageX' in e && 'pageY' in e) {
-        // Mouse event
-        setMenuPosition({
-          x: e.pageX,
-          y: e.pageY - window.scrollY,
-        });
-      } else if (e && 'x' in e && 'y' in e && typeof e.x === 'number') {
-        // Position object
-        setMenuPosition({
-          x: e.x,
-          y: e.y,
-        });
-      } else if (e && 'getBoundingClientRect' in e) {
-        // Element - calculate position from element's bounding rect
-        const rect = (e as HTMLElement | SVGElement).getBoundingClientRect();
+
+  const handleOpenMenu: WithContextMenuOpenMenuCallback = React.useCallback((e) => {
+    if (!e) {
+      return;
+    }
+
+    setIsMenuOpen(true);
+
+    if ('pageX' in e && 'pageY' in e) {
+      // Mouse event
+      setMenuPosition({
+        x: e.pageX,
+        y: e.pageY - window.scrollY,
+      });
+    } else if ('currentTarget' in e) {
+      // Element - calculate position from element's bounding rect
+      const rect = e.currentTarget.getBoundingClientRect();
+      if (rect) {
         setMenuPosition({
           x: rect.left + rect.width / 2,
           y: rect.top + rect.height / 2 + window.scrollY,
         });
       }
-    },
-    []
-  );
+    } else if ('x' in e && 'y' in e && typeof e.x === 'number') {
+      // Position object
+      setMenuPosition({
+        x: e.x,
+        y: e.y,
+      });
+    } else if (process.env.NODE_ENV !== 'production') {
+      console.warn('WithContextMenu: Unsupported parameter to openMenu:', e);
+    }
+  }, []);
 
   return (
     <>
-      {children({
-        openMenu: handleOpenMenu as React.MouseEventHandler<HTMLElement>,
-      })}
+      {children({ openMenu: handleOpenMenu })}
 
       {isMenuOpen && (
         <ContextMenu

packages/grafana-ui/src/components/DataLinks/DataLinksContextMenu.tsx

@@ -1,13 +1,12 @@
 import { css } from '@emotion/css';
 import { CSSProperties, type JSX } from 'react';
-import * as React from 'react';
 
 import { ActionModel, GrafanaTheme2, LinkModel } from '@grafana/data';
 import { selectors } from '@grafana/e2e-selectors';
 
 import { useStyles2 } from '../../themes/ThemeContext';
 import { linkModelToContextMenuItems } from '../../utils/dataLinks';
-import { WithContextMenu } from '../ContextMenu/WithContextMenu';
+import { WithContextMenu, WithContextMenuOpenMenuCallback } from '../ContextMenu/WithContextMenu';
 import { MenuGroup, MenuItemsGroup } from '../Menu/MenuGroup';
 import { MenuItem } from '../Menu/MenuItem';
 
@@ -22,10 +21,8 @@ export interface DataLinksContextMenuProps {
 }
 
 export interface DataLinksContextMenuApi {
-  openMenu?: React.MouseEventHandler<HTMLOrSVGElement> | ((position?: { x: number; y: number }) => void);
+  openMenu?: WithContextMenuOpenMenuCallback;
   targetClassName?: string;
-  /** Function to calculate menu position from an element (for keyboard events) */
-  getMenuPosition?: (element: HTMLElement | SVGElement) => { x: number; y: number };
 }
 
 export const DataLinksContextMenu = ({ children, links, style }: DataLinksContextMenuProps) => {
@@ -49,6 +46,7 @@ export const DataLinksContextMenu = ({ children, links, style }: DataLinksContex
             active={item.active}
             onClick={item.onClick}
             className={styles.itemWrapper}
+            tabIndex={0}
           />
         ))}
       </MenuGroup>
@@ -64,24 +62,7 @@ export const DataLinksContextMenu = ({ children, links, style }: DataLinksContex
     return (
       <WithContextMenu renderMenuItems={renderMenuGroupItems}>
         {({ openMenu }) => {
-          // Wrapper that handles both mouse events and position/element for keyboard events
-          const handleOpenMenu: React.MouseEventHandler<HTMLOrSVGElement> | ((positionOrElement?: { x: number; y: number } | HTMLElement | SVGElement) => void) = (
-            e: React.MouseEvent<HTMLOrSVGElement> | { x: number; y: number } | HTMLElement | SVGElement | undefined
-          ) => {
-            if (openMenu) {
-              openMenu(e as any);
-            }
-          };
-
-          const getMenuPosition = (element: HTMLElement | SVGElement) => {
-            const rect = element.getBoundingClientRect();
-            return {
-              x: rect.left + rect.width / 2,
-              y: rect.top + rect.height / 2 + window.scrollY,
-            };
-          };
-
-          return children({ openMenu: handleOpenMenu, targetClassName, getMenuPosition });
+          return children({ openMenu, targetClassName });
         }}
       </WithContextMenu>
     );

public/app/plugins/panel/piechart/PieChart.tsx

@@ -16,6 +16,7 @@ import {
   GrafanaTheme2,
   DataHoverClearEvent,
   DataHoverEvent,
+  LinkModel,
 } from '@grafana/data';
 import { selectors } from '@grafana/e2e-selectors';
 import { t } from '@grafana/i18n';
@@ -28,7 +29,7 @@ import {
   SeriesTable,
   usePanelContext,
 } from '@grafana/ui';
-import { getTooltipContainerStyles, useComponentInstanceId } from '@grafana/ui/internal';
+import { DataLinksContextMenuApi, getTooltipContainerStyles, useComponentInstanceId } from '@grafana/ui/internal';
 
 import { PieChartType, PieChartLabels } from './panelcfg.gen';
 import { filterDisplayItems, sumDisplayItemsReducer } from './utils';
@@ -117,40 +118,40 @@ export const PieChart = ({
             {(pie) => (
               <>
                 {pie.arcs.map((arc) => {
-                    const color = arc.data.display.color ?? FALLBACK_COLOR;
-                    const highlightState = getHighlightState(highlightedTitle, arc);
+                  const color = arc.data.display.color ?? FALLBACK_COLOR;
+                  const highlightState = getHighlightState(highlightedTitle, arc);
 
-                    if (arc.data.hasLinks && arc.data.getLinks) {
-                      return (
-                        <PieSliceWithDataLinks
-                          key={arc.index}
-                          arc={arc}
-                          pie={pie}
-                          highlightState={highlightState}
-                          fill={getGradientColor(color)}
-                          tooltip={tooltip}
-                          tooltipOptions={tooltipOptions}
-                          outerRadius={layout.outerRadius}
-                          innerRadius={layout.innerRadius}
-                          links={arc.data.getLinks}
-                        />
-                      );
-                    } else {
-                      return (
-                        <PieSlice
-                          key={arc.index}
-                          highlightState={highlightState}
-                          tooltip={tooltip}
-                          arc={arc}
-                          pie={pie}
-                          fill={getGradientColor(color)}
-                          tooltipOptions={tooltipOptions}
-                          outerRadius={layout.outerRadius}
-                          innerRadius={layout.innerRadius}
-                        />
-                      );
-                    }
-                  })}
+                  if (arc.data.hasLinks && arc.data.getLinks) {
+                    return (
+                      <PieSliceWithDataLinks
+                        key={arc.index}
+                        arc={arc}
+                        pie={pie}
+                        highlightState={highlightState}
+                        fill={getGradientColor(color)}
+                        tooltip={tooltip}
+                        tooltipOptions={tooltipOptions}
+                        outerRadius={layout.outerRadius}
+                        innerRadius={layout.innerRadius}
+                        links={arc.data.getLinks}
+                      />
+                    );
+                  } else {
+                    return (
+                      <PieSlice
+                        key={arc.index}
+                        highlightState={highlightState}
+                        tooltip={tooltip}
+                        arc={arc}
+                        pie={pie}
+                        fill={getGradientColor(color)}
+                        tooltipOptions={tooltipOptions}
+                        outerRadius={layout.outerRadius}
+                        innerRadius={layout.innerRadius}
+                      />
+                    );
+                  }
+                })}
                 {showLabel &&
                   pie.arcs.map((arc) => {
                     const highlightState = getHighlightState(highlightedTitle, arc);
@@ -195,18 +196,18 @@ interface SliceProps {
   fill: string;
   tooltip: UseTooltipParams<SeriesTableRowProps[]>;
   tooltipOptions: VizTooltipOptions;
-  openMenu?: (event: React.MouseEvent<SVGElement>) => void;
+  openMenu?: DataLinksContextMenuApi['openMenu'];
   outerRadius: number;
   innerRadius: number;
 }
 
 interface PieSliceWithDataLinksProps extends Omit<SliceProps, 'openMenu'> {
-  links: () => any[];
+  links: () => LinkModel[];
 }
 
 interface PieChartDataLinksContextMenuProps {
-  links: () => any[];
-  children: (props: { openMenu?: React.MouseEventHandler<HTMLOrSVGElement> }) => React.ReactElement;
+  links: () => LinkModel[];
+  children: (props: { openMenu?: DataLinksContextMenuApi['openMenu'] }) => React.ReactElement;
   elementRef: React.RefObject<SVGGElement>;
   publishDataHoverEvent: (raw: Event | React.SyntheticEvent) => void;
   publishDataHoverClearEvent: (raw: Event | React.SyntheticEvent) => void;
@@ -253,13 +254,8 @@ function PieChartDataLinksContextMenu({
       setTimeout(ensureNotFocusable, 0);
     }
 
-    const handleAnchorFocus = (e: FocusEvent) => {
-      publishDataHoverEvent(e);
-    };
-
-    const handleAnchorBlur = (e: FocusEvent) => {
-      publishDataHoverClearEvent(e);
-    };
+    const handleAnchorFocus = publishDataHoverEvent;
+    const handleAnchorBlur = publishDataHoverClearEvent;
 
     const handleAnchorKeyDown = (e: KeyboardEvent) => {
       if (e.key === 'Tab' && elementRef.current) {
@@ -353,12 +349,7 @@ function PieSliceWithDataLinks({
   );
 
   return (
-    <PieChartDataLinksContextMenu
-      links={links}
-      elementRef={elementRef}
-      publishDataHoverEvent={publishDataHoverEvent}
-      publishDataHoverClearEvent={publishDataHoverClearEvent}
-    >
+    <DataLinksContextMenu links={links}>
       {(api) => (
         <PieSlice
           tooltip={tooltip}
@@ -367,14 +358,13 @@ function PieSliceWithDataLinks({
           pie={pie}
           fill={fill}
           openMenu={api.openMenu}
-          getMenuPosition={api.getMenuPosition}
           tooltipOptions={tooltipOptions}
           outerRadius={outerRadius}
           innerRadius={innerRadius}
           elementRef={elementRef}
         />
       )}
-    </PieChartDataLinksContextMenu>
+    </DataLinksContextMenu>
   );
 }
 
@@ -383,7 +373,6 @@ function PieSlice({
   pie,
   highlightState,
   openMenu,
-  getMenuPosition,
   fill,
   tooltip,
   tooltipOptions,
@@ -455,12 +444,12 @@ function PieSlice({
         });
       }
     },
-    [publishDataHoverEvent, tooltip, pie, tooltipOptions]
+    [publishDataHoverEvent, tooltip, pie, tooltipOptions, arc]
   );
 
   const handleKeyDown = useCallback(
     (event: React.KeyboardEvent<SVGGElement>) => {
-      if (hasDataLinks && event.key === 'Enter') {
+      if (hasDataLinks && (event.key === 'Enter' || event.key === ' ')) {
         event.preventDefault();
         event.stopPropagation();
 
@@ -480,21 +469,12 @@ function PieSlice({
               y: svgRect.top + svgRect.height / 2 + centerY + window.scrollY,
             };
 
-            // Use the updated API that supports position objects
-            if (typeof openMenu === 'function' && openMenu.length === 1) {
-              openMenu(position);
-            } else if (getMenuPosition && elementRef.current) {
-              // Fallback: use getMenuPosition if available
-              const calculatedPosition = getMenuPosition(elementRef.current);
-              if (typeof openMenu === 'function') {
-                openMenu(calculatedPosition);
-              }
-            }
+            openMenu(position);
           }
         }
       }
     },
-    [hasDataLinks, openMenu, getMenuPosition, arc, outerRadius, innerRadius]
+    [hasDataLinks, openMenu, arc, outerRadius, innerRadius, elementRef]
   );
 
   const handleFocus = useCallback(
@@ -527,7 +507,7 @@ function PieSlice({
         blurTimeoutRef.current = null;
       }, 100);
     },
-    [publishDataHoverClearEvent]
+    [publishDataHoverClearEvent, elementRef]
   );
 
   useEffect(() => {