[charts] Extract keyboard focus navigation to the series config #20693

alexfauquette · 2025-12-17T15:08:47Z

This PR extract the keyboard behavior from the useKeboardNavigation to each series config

I replaced the custom FocusedItem by a new type derived from item identifier

mui-bot · 2025-12-17T15:10:26Z

Deploy preview: https://deploy-preview-20693--material-ui-x.netlify.app/

Bundle size report

Bundle	Parsed size	Gzip size
@mui/x-data-grid	0B^(0.00%)	0B^(0.00%)
@mui/x-data-grid-pro	0B^(0.00%)	0B^(0.00%)
@mui/x-data-grid-premium	0B^(0.00%)	0B^(0.00%)
@mui/x-charts	🔺+298B^(+0.09%)	🔺+121B^(+0.12%)
@mui/x-charts-pro	🔺+298B^(+0.07%)	🔺+94B^(+0.07%)
@mui/x-charts-premium	🔺+298B^(+0.07%)	🔺+92B^(+0.07%)
@mui/x-date-pickers	0B^(0.00%)	0B^(0.00%)
@mui/x-date-pickers-pro	0B^(0.00%)	0B^(0.00%)
@mui/x-tree-view	0B^(0.00%)	0B^(0.00%)
@mui/x-tree-view-pro	0B^(0.00%)	0B^(0.00%)

Details of bundle changes

Generated by 🚫 dangerJS against 8ba94cb

packages/x-charts/src/internals/plugins/models/seriesConfig/getNextFocusedItem.types.ts

JCQuintas · 2025-12-18T15:51:43Z

packages/x-charts/src/BarChart/seriesConfig/bar/getNextFocusedItem.ts

+const getNextFocusedItem: GetNextFocusedItem<'bar'> = (currentItem, event, state) => {
+  switch (event.key) {
+    case 'ArrowRight':
+      return getNextIndexFocusedItem<'bar', 'bar' | 'line' | 'scatter'>(
+        currentItem,
+        outSeriesTypes,
+        state,
+      );
+    case 'ArrowLeft':
+      return getPreviousIndexFocusedItem<'bar', 'bar' | 'line' | 'scatter'>(
+        currentItem,
+        outSeriesTypes,
+        state,
+      );
+    case 'ArrowDown':
+      return getPreviousSeriesFocusedItem<'bar', 'bar' | 'line' | 'scatter'>(
+        currentItem,
+        outSeriesTypes,
+        state,
+      );
+    case 'ArrowUp':
+      return getNextSeriesFocusedItem<'bar', 'bar' | 'line' | 'scatter'>(
+        currentItem,
+        outSeriesTypes,
+        state,
+      );
+    default:
+      return null;
+  }
+};


if the api is the same for every item we could move the abstraction up so we don't need to split it here. We just care about the returned function

const getNextFocusedItem: GetNextFocusedItem<'bar'> = (event) => { switch (event.key) { case 'ArrowRight': return getNextIndexFocusedItem case 'ArrowLeft': return getPreviousIndexFocusedItem case 'ArrowDown': return getPreviousSeriesFocusedItem case 'ArrowUp': return getNextSeriesFocusedItem default: return null; } };

then outSeriesTypes is just a const, can be part of the series config as well.

packages/x-charts/src/internals/plugins/models/seriesConfig/seriesConfig.types.ts

codspeed-hq · 2025-12-19T16:13:28Z

CodSpeed Performance Report

Merging #20693 will not alter performance

_{Comparing alexfauquette:move-keyboard-to-series-config (8ba94cb) with master (dcbbc64)¹}

Summary

✅ 14 untouched

No successful run was found on master (098da77) during the generation of this report, so dcbbc64 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

…series-config

packages/x-charts/src/internals/commonNextFocusItem.ts

packages/x-charts/src/LineChart/seriesConfig/keyboardFocusHandler.ts

packages/x-charts/src/internals/seriesHasData.ts

...rc/internals/plugins/featurePlugins/useChartKeyboardNavigation/useChartKeyboardNavigation.ts

...rc/internals/plugins/featurePlugins/useChartKeyboardNavigation/keyboardFocusHandler.types.ts

...ternals/plugins/featurePlugins/useChartKeyboardNavigation/utils/getPreviousSeriesWithData.ts

bernardobelchior · 2025-12-22T13:57:53Z

...c/internals/plugins/featurePlugins/useChartKeyboardNavigation/utils/getNextNonEmptySeries.ts

+export function getNextSeriesWithData<
+  OutSeriesType extends Exclude<ChartSeriesType, 'sankey'> = Exclude<ChartSeriesType, 'sankey'>,
+>(
+  series: ProcessedSeries<ChartSeriesType>,


Nit: can be done in a follow-up.

This type is broader than it needs to be. We only need the type to be { series: Array<{ data: unknown[] }>; seriesOrder: SeriesId[] }.

Should remove boilerplate from the tests (e.g., no need to specify the stackedData or minBarSize.

That would require some extra processing. If I'm transforming the series look up into an array, I could go with a complitely different solution

create the array

find the index of the current series

pick the one before or after

Looks simpler. I will do that in a follow up

That would require some extra processing

You're right, I wrote the wrong type. I meant something like this:

export type SeriesProcessorResult<TSeriesType extends ChartSeriesType> = { series: Record<SeriesId, { data: unknown[] }>; seriesOrder: SeriesId[]; }; type ProcessedSeries<TSeriesTypes extends ChartSeriesType = ChartSeriesType> = { [type in TSeriesTypes]?: SeriesProcessorResult<type>; };

The only goal would be to avoid having to create boilerplate in tests. We only care about the length of the data of a series, so no need to set minBarSize, etc.

JCQuintas

It seems there is an issue with the bar/line charts, possibly others as well, but it is harder to verify, where pressing the down arrow doesn't do the opposite of the up arrow.

Expected
Press up arrow: Select next series
Press down arrow: Select prev series

Actual
Press down arrow: Select prev ITEM

bernardobelchior · 2025-12-22T15:07:29Z

...c/internals/plugins/featurePlugins/useChartKeyboardNavigation/utils/getNextNonEmptySeries.ts

+      seriesIndex < seriesOfType.seriesOrder.length;
+      seriesIndex += 1
+    ) {
+      if (seriesOfType.series[seriesOfType.seriesOrder[seriesIndex]].data.length > 0) {


Is a series where all data points are null considered an empty series?

bernardobelchior · 2025-12-22T15:08:15Z

packages/x-charts/src/internals/commonNextFocusItem.ts

+    return {
+      type,
+      seriesId,
+      dataIndex: Math.min(dataLength - 1, currentItem?.dataIndex ?? 0),


Should it be possible to focus items whose value is null?

null values are not considered yet. For now it renders a focus object at 0

I created #20746 to discuss this aspect

alexfauquette · 2025-12-22T15:56:07Z

Press down arrow: Select prev ITEM

Yes a wrong copy past fixed by bernardo's review 👍

bernardobelchior · 2025-12-22T16:58:41Z

...rc/internals/plugins/featurePlugins/useChartKeyboardNavigation/keyboardFocusHandler.types.ts

+> = (
+  currentItem: (TSeriesType extends any ? FocusedItemIdentifier<TSeriesType> : never) | null,
+  state: ChartState<[UseChartKeyboardNavigationSignature], []>,
+) => FocusedItemIdentifier<OutputSeriesType> | null;


ID is short for identifier. Should we call it FocusedItemId instead? It's bit shorter

We (charts) use identifier in general for object identifiers, cause ID could be misleading

bernardobelchior · 2025-12-22T17:00:03Z

packages/x-charts/src/internals/seriesHasData.ts

+) {
+  // @ts-ignore sankey is not in MIT version
+  if (type === 'sankey') {
+    return false;


Why do we want this to return false? Is it because we aren't implementing it for Sankey at the moment?

Because sankey is for now the only series type without a data property.

Technically I could also throw an error sinc this shoudl never occure (you're not supposed to mix a bar chart with a sankey)

Sounds good 👍

packages/x-charts/src/internals/seriesHasData.ts

packages/x-charts/src/internals/commonNextFocusItem.ts

Co-authored-by: Bernardo Belchior <[email protected]> Signed-off-by: Alexandre Fauquette <[email protected]>

alexfauquette requested a review from a team as a code owner December 17, 2025 15:08

alexfauquette added type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. scope: charts Changes related to the charts. labels Dec 18, 2025

JCQuintas reviewed Dec 18, 2025

View reviewed changes

alexfauquette added 5 commits December 19, 2025 17:55

Move files to utils folder

608fd53

extract the logic in series config

55d2816

scripts

99839e2

fix

a587017

fix-ts

1226e1c

alexfauquette force-pushed the move-keyboard-to-series-config branch from 711a5d7 to aaee660 Compare December 19, 2025 16:56

reanming

b78384f

alexfauquette force-pushed the move-keyboard-to-series-config branch from aaee660 to b78384f Compare December 19, 2025 17:02

alexfauquette added 3 commits December 22, 2025 10:43

fogontten-update

3b9cb57

Merge remote-tracking branch 'upstream/master' into move-keyboard-to-…

4fe807e

…series-config

prettier

bc52c0c

alexfauquette requested review from JCQuintas and bernardobelchior December 22, 2025 12:34

bernardobelchior reviewed Dec 22, 2025

View reviewed changes

alexfauquette added 3 commits December 22, 2025 15:54

renaming1

923fe18

other wrong function

124cbf1

rename2

74caa0c

alexfauquette requested a review from bernardobelchior December 22, 2025 15:02

JCQuintas reviewed Dec 22, 2025

View reviewed changes

bernardobelchior reviewed Dec 22, 2025

View reviewed changes

bernardobelchior approved these changes Dec 22, 2025

View reviewed changes

Apply suggestions from code review

8ba94cb

Co-authored-by: Bernardo Belchior <[email protected]> Signed-off-by: Alexandre Fauquette <[email protected]>

Uh oh!

[charts] Extract keyboard focus navigation to the series config #20693

Are you sure you want to change the base?

[charts] Extract keyboard focus navigation to the series config #20693

Conversation

alexfauquette commented Dec 17, 2025

Uh oh!

mui-bot commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bundle size report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codspeed-hq bot commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #20693 will not alter performance

Summary

Footnotes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexfauquette Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JCQuintas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexfauquette commented Dec 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mui-bot commented Dec 17, 2025 •

edited

Loading

codspeed-hq bot commented Dec 19, 2025 •

edited

Loading

alexfauquette Dec 22, 2025 •

edited

Loading