[charts-pro] Add keyboard navigation to funnel by alexfauquette · Pull Request #20766 · mui/mui-x

alexfauquette · 2025-12-24T11:05:02Z

For now the keyboard navigation behavior is similar to the other ones:

right/left => modify item
top/bottom => modify series

Since by default funnel are vertical we could do the oposit

We could also adapt to the layout.

The code might look weird, but not a big deal :)

const isHorizontal = (currentItem, state) => state.series.funnel.series[currentItem.seriesId].layout === 'horizontal'

const keyboardFocusHandler: KeyboardFocusHandler<'funnel', 'funnel'> = (event) => {
  switch (event.key) {
    case 'ArrowRight':
      return (currentItem, state) => isHorizontal(currentItem, state) ? createGetNextIndexFocusedItem(outSeriesTypes)(currentItem, state) : createGetNextSeriesFocusedItem(outSeriesTypes)(currentItem, state);

You might also notice that arrow right/left remove the focus. We have the same issue with pie chart.
I will fix it after merging #20757 since it will probably impact exactly the same files

mui-bot · 2025-12-24T11:06:29Z

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

Updated pages:

docs/data/charts/funnel/funnel.md

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	0B^(0.00%)	0B^(0.00%)
@mui/x-charts-pro	🔺+1.07KB^(+0.23%)	🔺+453B^(+0.32%)
@mui/x-charts-premium	🔺+1.07KB^(+0.23%)	🔺+420B^(+0.30%)
@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 f5fb267

codspeed-hq · 2025-12-24T11:26:44Z

CodSpeed Performance Report

Merging this PR will not alter performance

_{Comparing alexfauquette:keyboard-funnel (f5fb267) with master (699784f)}

Summary

✅ 14 untouched benchmarks

bernardobelchior · 2025-12-24T11:41:04Z

We could also adapt to the layout.

If I recall correctly, we had already discussed this before. I also initially thought that it would make sense, but there were good points for keeping it consistent, i.e., not change with the layout.

packages/x-charts-pro/src/FunnelChart/FocusedFunnelSection.tsx

scripts/x-charts.exports.json

github-actions · 2025-12-24T17:16:25Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

alexfauquette · 2025-12-29T09:39:07Z

If I recall correctly, we had already discussed this before. I also initially thought that it would make sense, but there were good points for keeping it consistent, i.e., not change with the layout.

Yes, we got this discussion for the bar chart. Since it was oral I write down a brief resume:

The point was: Arrow are better if you consider them as a sementic navigation, like if you were navigating inside a table

up/down: moves series focus
left/right: moves item focus

If we want to match the visible aspect, we open a pandora box with 50 shades of complexity 😈

level 1: up and down should matches stacked lines
level 2: up and down should matches stacked bares with multiple stacking groups
level 3: up and down should matches lines that crosses each other (sometime series A is on top of B. Somtime it's the oposit)
level 4: defines what arrow down odes on a scatter chart

Doing the layout adaptation is the level 0. So I mention the tradeoff to be sure we all agree on this aspect and not just approve the PR because we missed this particular aspect :)

JCQuintas · 2025-12-29T12:06:31Z

packages/x-charts-pro/src/FunnelChart/FocusedFunnelSection.tsx

+
+  // FIXME gap should be obtained from the store.
+  // Maybe moving it to the series would be a good idea similar to what we do with bar charts and their stackingGroups
+  const gap = 0;


Why can't we use store.use(selectorFunnelGap) here?

Good point, that came from the tooltip position getter but no reason to keep it here 👍

JCQuintas

The behaviour seems mostly correct apart from the left/right arrow issue

JCQuintas · 2025-12-29T12:12:25Z

the keyboard navigation behavior is similar to the other ones

It's probably better to be consistent which arrows affect which domains, so I'm good with the current solution

github-actions · 2025-12-30T11:34:03Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions · 2026-01-07T11:31:49Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

packages/x-charts-pro/src/FunnelChart/FocusedFunnelSection.tsx

bernardobelchior · 2026-01-12T12:26:15Z

packages/x-charts-pro/src/FunnelChart/FocusedFunnelSection.tsx

+    y: -Infinity,
+  };
+
+  funnelSeries.dataPoints.forEach((section) => {


This logic is basically a duplicate of FunnelPlot's useAggregatedData. Could we find a way to reuse some of the logic? Otherwise we might forget to update this component when we update the main funnel logic.

Can be done as a follow-up

Done in 5ef0b85

JCQuintas · 2026-01-12T15:22:09Z

It seems like the gap is not aligned properly.

bernardobelchior

Minor things, code looks good 👍

bernardobelchior · 2026-01-12T15:25:24Z

packages/x-charts-pro/src/FunnelChart/coordinateMapper.ts

  scale: D3Scale,
  isCategoryDirection: boolean,
  gap: number,
+  baseScaleData?: readonly any[],


Why is baseScaleData optional? Should the caller default to [] instead?

If not, I'd suggest using | undefined instead of ? so we don't forget to pass the data. | undefined and ? have slightly different semantics.

packages/x-charts-pro/src/FunnelChart/seriesConfig/tooltipPosition.ts

bernardobelchior · 2026-01-12T15:27:32Z

packages/x-charts-pro/src/FunnelChart/getCornerPoints.ts

+import type { Point, PositionGetter } from './curves';
+import type { FunnelDataPoints } from './funnel.types';
+
+export const getCornerPoints = (


This function returns the extrema (min and max) in every direction, so an alternative name could be get2DExtrema or computeExtrema2D, if you think that's clearer

alexfauquette · 2026-01-13T09:49:11Z

It seems like the gap is not aligned properly.

Tricky to debug. That's because I used the useXAxis() instead of the funnel selectors. Only the second one takes into account the gap when computing the scale

github-actions · 2026-01-13T09:59:28Z

This pull request has conflicts, please resolve those before we can evaluate the pull request.

alexfauquette added 2 commits December 24, 2025 11:34

[charts-pro] Add keyboard navigation to funnel

4ffd344

scripts

c8fa969

alexfauquette requested review from a team and prakhargupta1 as code owners December 24, 2025 11:05

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 24, 2025

bernardobelchior reviewed Dec 24, 2025

View reviewed changes

packages/x-charts-pro/src/FunnelChart/FocusedFunnelSection.tsx Outdated Show resolved Hide resolved

packages/x-charts-pro/src/FunnelChart/FocusedFunnelSection.tsx Outdated Show resolved Hide resolved

scripts/x-charts.exports.json Show resolved Hide resolved

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Dec 24, 2025

alexfauquette requested a review from JCQuintas December 29, 2025 09:26

alexfauquette added 4 commits December 29, 2025 10:45

Merge remote-tracking branch 'upstream/master' into keyboard-funnel

a3f4b70

avoid array creation

127e364

more robust early return

1bd8dee

prettier

0520b8a

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Dec 29, 2025

alexfauquette requested a review from bernardobelchior December 29, 2025 10:00

fix

7c7ea57

JCQuintas reviewed Dec 29, 2025

View reviewed changes

JCQuintas approved these changes Dec 29, 2025

View reviewed changes

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Dec 30, 2025

Merge remote-tracking branch 'upstream/master' into keyboard-funnel

f7bcb13

github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged. and removed PR: out-of-date The pull request has merge conflicts and can't be merged. labels Jan 6, 2026

Merge remote-tracking branch 'upstream/master' into keyboard-funnel

44fb0f5

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Jan 9, 2026

bernardobelchior reviewed Jan 12, 2026

View reviewed changes

alexfauquette added 3 commits January 12, 2026 13:57

Merge remote-tracking branch 'upstream/master' into keyboard-funnel

59ca62c

fix-funnel-gap

db6180a

share common code between plot and focused elements

5ef0b85

bernardobelchior approved these changes Jan 12, 2026

View reviewed changes

alexfauquette added 3 commits January 13, 2026 10:14

renaming

30ae117

refine types

d6ef9c5

fix gap error in scales

22ba79c

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Jan 13, 2026

Merge remote-tracking branch 'upstream/master' into keyboard-funnel

aa7e8de

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Jan 13, 2026

scripts

f5fb267

alexfauquette enabled auto-merge (squash) January 13, 2026 10:30

alexfauquette merged commit d1ccc97 into mui:master Jan 13, 2026
21 checks passed

Uh oh!

Conversation

alexfauquette commented Dec 24, 2025

Uh oh!

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

Uh oh!

Bundle size report

Uh oh!

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

Uh oh!

Merging this PR will not alter performance

Summary

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Dec 24, 2025

Uh oh!

alexfauquette commented Dec 29, 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!

JCQuintas left a comment

Choose a reason for hiding this comment

Uh oh!

JCQuintas commented Dec 29, 2025

Uh oh!

github-actions bot commented Dec 30, 2025

Uh oh!

github-actions bot commented Jan 7, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JCQuintas commented Jan 12, 2026

Uh oh!

bernardobelchior left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexfauquette commented Jan 13, 2026

Uh oh!

github-actions bot commented Jan 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mui-bot commented Dec 24, 2025 •

edited

Loading

codspeed-hq bot commented Dec 24, 2025 •

edited

Loading

bernardobelchior commented Dec 24, 2025 •

edited

Loading