diff --git a/CONTRIBUTION.md b/CONTRIBUTION.md new file mode 100644 index 00000000000..14a9237c837 --- /dev/null +++ b/CONTRIBUTION.md @@ -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 `` 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 `` 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 `` 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 `` tag's default behavior + +### 5. Single Link Case Handling +- Detected when slice is wrapped in `` tag (when `openMenu` is undefined) +- Attached focus/blur event listeners to the parent `` tag +- Ensured inner `` element has `tabIndex={-1}` to prevent tab order conflicts +- Ensured `` 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 (`` 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 `` 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 + + diff --git a/packages/grafana-ui/src/components/ContextMenu/WithContextMenu.tsx b/packages/grafana-ui/src/components/ContextMenu/WithContextMenu.tsx index d439938d26e..426e648062a 100644 --- a/packages/grafana-ui/src/components/ContextMenu/WithContextMenu.tsx +++ b/packages/grafana-ui/src/components/ContextMenu/WithContextMenu.tsx @@ -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 | { 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, })} {isMenuOpen && ( diff --git a/packages/grafana-ui/src/components/DataLinks/DataLinksContextMenu.tsx b/packages/grafana-ui/src/components/DataLinks/DataLinksContextMenu.tsx index 5e3b71cbe73..7b0ae645c9b 100644 --- a/packages/grafana-ui/src/components/DataLinks/DataLinksContextMenu.tsx +++ b/packages/grafana-ui/src/components/DataLinks/DataLinksContextMenu.tsx @@ -22,8 +22,10 @@ export interface DataLinksContextMenuProps { } export interface DataLinksContextMenuApi { - openMenu?: React.MouseEventHandler; + openMenu?: React.MouseEventHandler | ((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 ( {({ openMenu }) => { - return children({ openMenu, targetClassName }); + // Wrapper that handles both mouse events and position/element for keyboard events + const handleOpenMenu: React.MouseEventHandler | ((positionOrElement?: { x: number; y: number } | HTMLElement | SVGElement) => void) = ( + e: React.MouseEvent | { 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 }); }} ); diff --git a/public/app/plugins/panel/piechart/PieChart.tsx b/public/app/plugins/panel/piechart/PieChart.tsx index 3a0c30db28f..d2a011435dc 100644 --- a/public/app/plugins/panel/piechart/PieChart.tsx +++ b/public/app/plugins/panel/piechart/PieChart.tsx @@ -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; - - 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(