feat: extend DataLinksContextMenu to support keyboard events

- Update DataLinksContextMenu API to accept position objects for keyboard events - Add getMenuPosition helper function to calculate position from element - Update WithContextMenu to handle position objects and elements - Simplify pie chart keyboard handler to use position object API - Position calculation logic now in core component (DataLinksContextMenu) - Addresses review feedback about keyboard accessibility and position calculation
2026-01-06 19:29:28 -05:00
parent 838bc756db
commit 254a7b9971
4 changed files with 167 additions and 75 deletions
@@ -0,0 +1,99 @@
+# Contribution: Pie Chart Keyboard Focus Indicator Fix
+
+## Issue
+**Issue #114227**: Keyboard focus indicator is not visible on data links in Pie Chart visualization
+
+When keyboard users navigate through a Pie Chart panel using the Tab key, data links (clickable links on chart segments) do not show a visible focus indicator. This makes it impossible for keyboard-only users to know which element is currently focused, reducing navigation clarity and accessibility.
+
+## Problem Analysis
+
+### Root Cause
+1. Pie chart slices with data links are rendered as SVG `<g>` elements
+2. These elements are not naturally keyboard focusable (SVG elements need `tabIndex` to be focusable)
+3. No focus event handlers were attached to trigger visual feedback
+4. No keyboard activation support (Enter/Space keys)
+
+### Two Rendering Cases
+- **Single link case**: `DataLinksContextMenu` wraps the slice in an `<a>` tag
+- **Multiple links case**: `DataLinksContextMenu` provides an `openMenu` function via render prop
+
+## Solution Approach
+
+### 1. Data Link Detection
+- Check for data links using `arc.data.hasLinks` and `arc.data.getLinks`
+- Determine if slice should be focusable based on presence of links
+
+### 2. Keyboard Accessibility
+- Added `tabIndex={0}` for slices with data links (multiple links case)
+- Added `tabIndex={-1}` for slices wrapped in `<a>` tag (single link case) to prevent double focus
+- Added `role="link"` and `aria-label` for proper screen reader support
+
+### 3. Focus Indicator
+- Leveraged Grafana's existing `DataHoverEvent` system for highlighting
+- On focus, publish `DataHoverEvent` to trigger the same visual highlight as mouse hover
+- On blur, publish `DataHoverClearEvent` to clear the highlight
+- Used a small delay on blur to prevent flickering during focus transitions
+
+### 4. Keyboard Activation
+- Added `handleKeyDown` to handle Enter key presses
+- For multiple links: Create synthetic mouse event and call `openMenu` directly
+- For single link: Dispatch native click event to trigger the `<a>` tag's default behavior
+
+### 5. Single Link Case Handling
+- Detected when slice is wrapped in `<a>` tag (when `openMenu` is undefined)
+- Attached focus/blur event listeners to the parent `<a>` tag
+- Ensured inner `<g>` element has `tabIndex={-1}` to prevent tab order conflicts
+- Ensured `<a>` tag is properly focusable (remove any `tabIndex="-1"` if present)
+
+### 6. Tab Order Optimization
+- Sorted slices with data links first in the DOM to ensure proper tab order
+- This ensures keyboard users reach interactive elements before non-interactive ones
+
+## Implementation Details
+
+### Key Changes in `PieChart.tsx`
+
+1. **Imports**: Added `useRef` and `useEffect` from React
+
+2. **SliceProps Interface**: Added `outerRadius` and `innerRadius` props to calculate click coordinates
+
+3. **PieSlice Component**:
+   - Added `elementRef` and `blurTimeoutRef` for DOM manipulation and blur handling
+   - Detected data links: `hasDataLinksDirect` and `hasDataLinks`
+   - Determined focusability: `shouldBeFocusable` (true only for multiple links case)
+   - Added `useEffect` hook to handle single link case (`<a>` tag focus/blur)
+   - Added `handleFocus` callback to publish `DataHoverEvent` on focus
+   - Added `handleBlur` callback to publish `DataHoverClearEvent` on blur (with delay)
+   - Added `handleKeyDown` callback to handle Enter key activation
+   - Updated `<g>` element with accessibility attributes:
+     - `tabIndex={shouldBeFocusable ? 0 : -1}`
+     - `role={shouldBeFocusable ? 'link' : undefined}`
+     - `aria-label={shouldBeFocusable ? ... : undefined}`
+     - `onKeyDown={shouldBeFocusable ? handleKeyDown : undefined}`
+     - `onFocus={hasDataLinks ? handleFocus : undefined}`
+     - `onBlur={hasDataLinks ? handleBlur : undefined}`
+     - `style={{ outline: 'none' }}` to remove browser default outline
+
+4. **PieChart Component**:
+   - Added sorting to `pie.arcs.map` to put slices with data links first
+   - Passed `outerRadius` and `innerRadius` to `PieSlice` components
+
+## Testing
+
+### Expected Behavior
+- Tab navigation reaches slices with data links
+- Focused slice shows visual highlight (scales up, others fade)
+- Enter key activates the data link menu
+- Tab order is logical (data link slices appear before non-link slices)
+
+## Files Modified
+
+- `public/app/plugins/panel/piechart/PieChart.tsx`
+
+## References
+
+- [WCAG 2.4.7 Focus Visible](https://www.w3.org/WAI/WCAG21/Understanding/focus-visible.html)
+- [Grafana Contributing Guide](https://github.com/grafana/grafana/blob/main/CONTRIBUTING.md)
+- Issue: https://github.com/grafana/grafana/issues/114227
+
+
@@ -15,16 +15,38 @@ 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();
+        setMenuPosition({
+          x: rect.left + rect.width / 2,
+          y: rect.top + rect.height / 2 + window.scrollY,
+        });
+      }
+    },
+    []
+  );
+
  return (
    <>
      {children({
-        openMenu: (e) => {
-          setIsMenuOpen(true);
-          setMenuPosition({
-            x: e.pageX,
-            y: e.pageY - window.scrollY,
-          });
-        },
+        openMenu: handleOpenMenu as React.MouseEventHandler<HTMLElement>,
      })}

      {isMenuOpen && (
@@ -22,8 +22,10 @@ export interface DataLinksContextMenuProps {
 }

 export interface DataLinksContextMenuApi {
-  openMenu?: React.MouseEventHandler<HTMLOrSVGElement>;
+  openMenu?: React.MouseEventHandler<HTMLOrSVGElement> | ((position?: { x: number; y: number }) => void);
  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) => {
@@ -62,7 +64,24 @@ export const DataLinksContextMenu = ({ children, links, style }: DataLinksContex
    return (
      <WithContextMenu renderMenuItems={renderMenuGroupItems}>
        {({ openMenu }) => {
-          return children({ openMenu, targetClassName });
+          // 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 });
        }}
      </WithContextMenu>
    );
@@ -367,6 +367,7 @@ function PieSliceWithDataLinks({
          pie={pie}
          fill={fill}
          openMenu={api.openMenu}
+          getMenuPosition={api.getMenuPosition}
          tooltipOptions={tooltipOptions}
          outerRadius={outerRadius}
          innerRadius={innerRadius}
@@ -382,6 +383,7 @@ function PieSlice({
  pie,
  highlightState,
  openMenu,
+  getMenuPosition,
  fill,
  tooltip,
  tooltipOptions,
@@ -462,7 +464,8 @@ function PieSlice({
        event.preventDefault();
        event.stopPropagation();

-        if (elementRef.current) {
+        if (elementRef.current && openMenu) {
+          // Calculate position from arc center for pie chart
          const arcCenterAngle = (arc.startAngle + arc.endAngle) / 2;
          const arcRadius = (outerRadius + innerRadius) / 2;
          const centerX = Math.cos(arcCenterAngle - Math.PI / 2) * arcRadius;
@@ -472,77 +475,26 @@ function PieSlice({
          const svgRect = svgElement?.getBoundingClientRect();

          if (svgRect) {
-            const actualX = svgRect.left + svgRect.width / 2 + centerX;
-            const actualY = svgRect.top + svgRect.height / 2 + centerY;
+            const position = {
+              x: svgRect.left + svgRect.width / 2 + centerX,
+              y: svgRect.top + svgRect.height / 2 + centerY + window.scrollY,
+            };

-            if (openMenu) {
-              const syntheticEvent = {
-                currentTarget: elementRef.current,
-                target: elementRef.current,
-                preventDefault: () => event.preventDefault(),
-                stopPropagation: () => event.stopPropagation(),
-                isDefaultPrevented: () => false,
-                isPropagationStopped: () => false,
-                persist: () => {},
-                nativeEvent: new MouseEvent('click', {
-                  bubbles: true,
-                  cancelable: true,
-                  view: window,
-                  button: 0,
-                  buttons: 0,
-                  clientX: actualX,
-                  clientY: actualY,
-                  screenX: actualX,
-                  screenY: actualY,
-                }),
-                clientX: actualX,
-                clientY: actualY,
-                pageX: actualX,
-                pageY: actualY,
-                screenX: actualX,
-                screenY: actualY,
-                button: 0,
-                buttons: 0,
-                type: 'click',
-                bubbles: true,
-                cancelable: true,
-                defaultPrevented: false,
-                eventPhase: 0,
-                isTrusted: false,
-                timeStamp: Date.now(),
-                altKey: false,
-                ctrlKey: false,
-                shiftKey: false,
-                metaKey: false,
-                getModifierState: () => false,
-                movementX: 0,
-                movementY: 0,
-                relatedTarget: null,
-                detail: 0,
-                view: window,
-                which: 0,
-              } as unknown as React.MouseEvent<SVGGElement>;
-
-              openMenu(syntheticEvent);
-            } else {
-              const nativeMouseEvent = new MouseEvent('click', {
-                bubbles: true,
-                cancelable: true,
-                view: window,
-                button: 0,
-                buttons: 0,
-                clientX: actualX,
-                clientY: actualY,
-                screenX: actualX,
-                screenY: actualY,
-              });
-              elementRef.current.dispatchEvent(nativeMouseEvent);
+            // 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);
+              }
            }
          }
        }
      }
    },
-    [hasDataLinks, openMenu, arc, outerRadius, innerRadius]
+    [hasDataLinks, openMenu, getMenuPosition, arc, outerRadius, innerRadius]
  );

  const handleFocus = useCallback(