[charts] Focus the clicked item on radar charts - #23287
Conversation
…ible `isFocused` conflated "the chart subtree owns the DOM focus" with "the focus indicator is rendered". Split the second meaning into `isFocusVisible` so an upcoming pointer-driven focus can set the item without revealing it. The focus indicator, highlight and tooltip selectors now require both flags. `selectorChartsFocusedOrToFocusedItem` stays ungated so the accessibility description keeps working. No behavior change: the focusin handler sets both flags.
Adds `chartsAccessibilityProxyRef` to `useChartElementRef` and a `focusAccessibilityProxy` helper, so the keyboard navigation plugin can move the DOM focus into the chart without querying the layer container, whose subtree also holds the zoom slider and brush focusable elements.
…ance `focusItem` makes any item the one keyboard navigation resumes from and moves the DOM focus into the chart. A focus-visible intent ref, fed by pointerdown, keydown and focusout on the layer container, decides whether the focus indicator shows: pointer driven focus stays hidden until the user presses a key, unless `focusItemOnClick` is set. The highlight and tooltip only switch to keyboard mode when the focus is visible, so clicking an item no longer detaches the tooltip from the pointer. Navigation keys now always reveal the focus, even at the edges of a series that does not cycle, where the focused item does not move.
Threads the new keyboard navigation param through the container, radar and heatmap prop hooks, and regenerates the propTypes and API documentation.
Clicking an item now makes it the item keyboard navigation resumes from, whether or not an `onItemClick` callback is set. `useInteractionItemProps` gained a merged `onClick` covering pie, line marks, scatter markers, radar marks, funnel sections, sankey nodes and links, and map shapes. The paths that have to compute the index themselves call `focusItem` directly: bars and heatmap cells through their container hit test, line and area paths through the axis index, radar areas through the click angle, and scatter through the closest point, so clicking next to a point focuses it. Radial bar and radial line charts register `useChartItemClick`, which also gives them the `onItemClick` callback they were missing. A click that resolves no item still moves the DOM focus into the chart, keeping the previously focused item. Known gap: the WebGL range bar plot has no click path, so range bar click to focus only works with the SVG plot.
Covers the two modes on bar charts, the closest point resolution on scatter, line marks and paths, pie arcs and radar marks, and asserts the highlight stays pointer driven and `onItemClick` still fires exactly once. The accessibility proxy root keeps a `tabIndex` of -1 once an announcer child owns the tab stop. Without it the browser blurred the root during the internal hand-off, which read as the focus leaving the chart and revealed a focus set by a click. Keyboard modality is tracked on the document so tabbing back in from an outside element shows the focus again.
Adds a section to the accessibility page explaining the two focus modes, and a toggle to the keyboard navigation demo. Also lists the map, radial bar and radial line charts among the ones supporting keyboard navigation.
Clicking the empty space of a cartesian chart only gave the chart focus, which felt unresponsive. It now resolves the axis under the pointer and focuses the item it points at, taken from the series being navigated, or from the first series when none is focused yet. Clicks outside of any axis, and charts without a cartesian axis, keep the previous behavior of only focusing the chart. A pointer interaction that already resolved an item skips the fallback, so the closest point of a scatter chart is not overwritten by an axis index.
…t name The accessibility page documented the charts theme defaults under `MuiChartDataProvider`, which matches no component, so the snippet silently did nothing. The name is `MuiChartsDataProvider`. Adds a test covering `focusItemOnClick` set through the theme, which is what caught it.
RadarSeriesPlot, the composition component, spread the interaction props after its own `onClick`, so the click handler added for the keyboard focus replaced `onAreaClick` instead of running alongside it. It now merges both and focuses the clicked item, like the split RadarSeriesArea and RadarSeriesMarks already did. Also strengthens the radar click to focus tests, which only asserted that some indicator appeared and so passed even when the click resolved nothing.
The fallback read the data index off the first x or y axis, so on a chart with several axes a click could resolve against a scale the target series is not bound to and focus the wrong point. The target series is now resolved first, and its own `xAxisId` / `yAxisId` picks the axis, mirroring useLineItemClickHandler. Also keeps the scatter marker click alive during a zoom interaction. Moving the callback into the interaction props had put it behind the `isInteracting` guard, which silently stopped `onItemClick` from firing mid zoom. Only the pointer handlers are dropped there, as before.
`updateFocus` hardcoded `isFocused: true`, which assumes the accessibility proxy is present to receive the focus. A composition that does not render it would get an indicator that no blur can clear, since the focusout listener lives on the same missing container. The flag is now read from the DOM, and the item is still stored so it is restored on the next focus. Also documents that the interaction props own the click, so consumers pass their callback through the options instead of setting `onClick` next to the spread.
Scatter markers and range bars derived their cursor from the presence of a click handler. Now that the handler is always attached to drive the keyboard focus, they showed a pointer cursor on charts that are not clickable. Both take the cursor from their plot instead, which derives it from `onItemClick` like the other series do. `ScatterMarkerProps` gained a `cursor` prop so custom markers can do the same, since `onClick` no longer tells them whether the chart is interactive.
Custom slot components have to forward the click handler for click to focus to work, and can no longer read `onClick` to tell whether the chart is interactive. Documents both, and how to reach the same behavior from a plot built from scratch.
`focusChart` and `focusItem` carried the same enabled check, visibility resolution, proxy focus and no-op comparison. Both now delegate to a single `applyFocus`, where an undefined item means focusing the chart alone, and `focusItem` only adds the identifier validation on top.
Registering `useChartItemClick` on the radial charts gave them click to focus, but it also added a public `onItemClick` they never had. That is a separate API addition, so it moves to its own pull request.
- The click fallback listens on the document, so it runs after the React handlers instead of before them. It was resolving an item on every click, only to be corrected by the series handler a moment later. - `restoreFocus` was `applyFocus` without the focusing step, which is already a no-op once the focus is inside the chart. It now delegates. - `focusChart` was only used by the fallback, so it is no longer part of the instance API. - Inlines a single use helper and folds the two symmetric visible index lookups into one.
The clicked item is resolved by the series that owns the click, not read from the highlight. Controlling the highlight stops the store from tracking the pointer, so a focus derived from it would follow the consumer instead. Pins that down.
The fallback focused the chart whenever a click hit nothing. The drawing area is a rectangle, but the data rarely fills it, so clicking next to a pie, outside its circle, took the focus for a click that was not meant for the chart. Such clicks are now left alone. Cartesian charts are unaffected: a click in the plot still resolves through the axis.
Every series already reports the item the pointer is over, for the tooltip and the highlight, so the click needs no per series wiring of its own. That state was not usable directly because `highlightedItem` and `tooltipItem` are controlled props: once controlled, the store stops following the pointer. Adds `interaction.hoveredItem`, written by the same hover paths but never controlled, and focuses it on tap. This replaces the merged click handler in `useInteractionItemProps` and the click wiring of the pie, line, scatter, radar, funnel, sankey, map and range bar series, along with the cursor adjustments those needed. Touch has no hover phase, so the container hit test, the closest point and the radar area also resolve the item on `pointerdown`: a stationary tap does not produce a `pointermove`. The highlight and tooltip selectors are untouched, and can be derived from the hovered item separately.
It was registered inside the effect that binds the DOM listeners on the layer container, which it has nothing to do with. Also drops the radar wiring, which needs the pointer item to be resolved from the click angle, so it moves to its own pull request.
Deploy previewBundle size
PerformanceTotal duration: 1,531.03 ms +30.22 ms(+2.0%) | Renders: 63 (+0) | Paint: 2,072.98 ms +74.40 ms(+3.7%)
…and 5 more (+16 within noise) — details Metric alarms
…and 2 more metric alarms — details Check out the code infra dashboard for more information about this PR. |
The radial charts ship `getItemAtPosition`, so they report the item under the pointer through the shared hit test and need no wiring of their own.
The radar area only knows its series, and the marks carry no interaction props, so neither reports an item the click can be resolved from. The area now reports the item under the pointer using the rotation index, and the marks report their own index when a click callback makes them pointer active. Both are also bound to `pointerdown`, since a touch tap may never produce a `pointermove`.
7f5d959 to
9a38a29
Compare
Why the radar wires this by hand instead of using
|
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
The click to focus PR this was stacked on has landed, so master already carries its final version. The merge takes master's tree wholesale and keeps only the change that belongs here: the radar area and marks reporting the item under the pointer.
The click to focus section had no demo. The radar is the chart where the behavior is least obvious, since which element takes the click depends on whether a click callback makes the marks interactive. The onMarkClick toggle shows both paths resolve to the same item, and focusItemOnClick shows the indicator without waiting for a key press.
This reverts commit 974fd66.
The radar area path only covers the polygon the data draws, so a click between it and the outer edge lands on no element and reports no hovered item. The cartesian axis fallback cannot help, since a radar has no cartesian axis, so the click resolved nothing. That left the focus dropped rather than moved: the mousedown blurs the accessibility proxy, and the click is what puts the focus back. A click that resolved nothing never restored it, so the indicator disappeared while the stored item stayed, and the next arrow key resumed from the old item. The rotation axis has an index for the click angle, which is the radar counterpart of resolving a click from the cartesian axis under the pointer.
The first test only clicked straight down a metric axis, which resolves the same index whether the angle picks the nearest axis or the wedge it falls in. The cases either side of a wedge midpoint are what tell the two apart.
A click inside the radar but outside the series polygon reached the cartesian fallback, which has no axis to read on a radar, so it resolved nothing and the focus stayed where it was. The axis highlight resolves that same pointer to a spoke, picking the nearest one and switching at the midline between two of them. The focus now reads the index that highlight already computed, rather than repeating the geometry, so the two can not drift apart.
…with The radar reports a per-point item on pointer move, but useInteractionAllItemProps only knows the series, since dataIndex sits on the points rather than on the series entry. Its onPointerLeave therefore cleared an index-less identifier, which never matched the one reported, so clearHoveredItem read it as another item taking over and left the stale one in the store. A later click on empty space resolved that stale item instead of the one under the pointer. The leave now clears exactly what was reported, and the area keeps the pointer-capture release it used to drop by overriding onPointerDown.
Unit tests on the resolver itself: the pointer path it sits behind is driven by the gesture manager, which synthetic pointer events do not reproduce reliably.
The area and mark handlers were written out three times, in RadarSeriesArea, RadarSeriesMarks and the RadarSeriesPlot copy of both. They now come from useRadarHoveredItem, which chains whatever handlers the caller already has so the reporting survives a spread, and takes the mark index on leave so it only clears what it reported. RadarSeriesPlot marks also gain the pointerEvents guard the standalone marks had, so both paths resolve the same item for a given pointer position.
Gives the radar charts click-to-focus.
Note
#23247 has landed, and master was merged in. Files changed now shows only the change that belongs to this PR. The commit list still carries the pre-merge history, so read the diff rather than the commits.
Why it is separate
#23247 resolves a clicked item from
interaction.hoveredItem, the item each series reports as the pointer moves over it. That works for every series that reports a complete identifier, so no per-series click wiring is needed there.Radar is the exception, and needed enough of its own wiring to be worth splitting out:
useRadarRotationIndex, so it has to compute the hovered item itself.onMarkClickis set, and when it is set they cover the area underneath, so the area stops reporting anything. They have to report their own index.Both are bound to
pointerdownas well aspointermove, matching the rest of #23247: touch has no hover phase, so a stationary tap may never produce apointermove.Note this is hover wiring, not click wiring. It makes the radar report the item under the pointer like every other series already does, which is a gap worth closing on its own terms: until now the radar had no per-item pointer state at all.
Testing
RadarChart.clickToFocus.test.tsxcovers both paths, because which one runs depends ononMarkClick:Both assert which mark ends up focused, rather than that some indicator appeared — an indicator shows up either way once an arrow key is pressed, so the weaker assertion passes even when the click resolved nothing.