[charts] Introduce the radar chart #16406

alexfauquette · 2025-01-30T16:25:36Z

This PR is a rough first version of the Radar chart #7925. Lots of features are missing.

The idea is to validate the data structure, and it's mapping to a rotation/radius scale system. If ok, we can merge this one and add features in distinct PR such that we keep them small enough to be relevant (I don't expect someone to read 200 files with 10 different features and spot any bug or room for improvement)

Main idea

The polar plugin introduce the support of rotatio/radius axes that work similarly to the x/y axes

The radar don't expose directly those axies. It uses an intermediate metrics to make sure the radar has only one rotation axis with scale point, and one radius axis per metric.

Then we have one component per item rendered (grid, area, and marks)

Features

Preview: https://deploy-preview-16406--material-ui-x.netlify.app/x/react-charts/radar/

mui-bot · 2025-01-30T16:29:57Z

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

Updated pages:

docs/data/charts/radar/radar.md

Generated by 🚫 dangerJS against 8cf53e3

github-actions · 2025-01-31T11:09:03Z

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

codspeed-hq · 2025-01-31T15:21:45Z

CodSpeed Performance Report

Merging #16406 will not alter performance

_{Comparing alexfauquette:radar-chart (8cf53e3) with master (b252b9a)}

Summary

✅ 6 untouched benchmarks

github-actions · 2025-02-04T08:14:29Z

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

bernardobelchior · 2025-02-05T10:49:08Z

Is it just me or are the circles not being affected by the area above them?

For example, I think it's clear the blue area is below the yellow area because its color is affected by the yellow area's. However, this doesn't seem to be the case for the circles in blue, which seem unaffected.

bernardobelchior · 2025-02-05T10:50:41Z

docs/data/charts/radar/radar.md

+
+The radar chart displays a grid behind the series that can be configured with
+
+- `startAngle` The rotation angle of the entire chart


Would it make sense to mention the angle should be specified in degrees?

Do you think it should be startAngleDeg?

Sorry, I should have been more specific. I was suggesting something along the lines of:

- `startAngle` The rotation angle of the entire chart in degrees

bernardobelchior · 2025-02-05T10:52:18Z

docs/data/charts/radar/radar.md

+The radar chart displays a grid behind the series that can be configured with
+
+- `startAngle` The rotation angle of the entire chart
+- `divisionNumber` The nb of division of the grid


Not sure if it would make more sense to write "number" here. It might not be clear "nb" means number.

bernardobelchior · 2025-02-05T11:02:57Z

packages/x-charts/src/internals/plugins/featurePlugins/useChartPolarAxis/computeAxisValue.ts

+        degreeToRad((axis as RotationConfig).endAngle, 2 * Math.PI),
+      ];
+      const diff = angles[1] - angles[0];
+      if (diff > Math.PI * 2 - 0.1) {


Maybe I'm missing some context here. What is the reason for subtracting specifically by 0.1?

To take into account the risk of computation errors that could lead to diff === 2*Math.PI being false for less that 0.1

We could move that to 0.001. But I assume polar axes are either displayed on 360° in which case we need to avoid the overlap of first and last point. Or 180° in which case we have nothing to do

bernardobelchior · 2025-02-05T11:08:55Z

...s/x-charts/src/internals/plugins/featurePlugins/useChartPolarAxis/useChartPolarAxis.types.ts

+    params: UseChartPolarAxisParameters;
+    defaultizedParams: UseChartPolarAxisDefaultizedParameters;
+    state: UseChartPolarAxisState;
+    // instance: UseChartPolarAxisInstance;


Just double-checking: is this commented on purpose?

bernardobelchior · 2025-02-05T11:11:48Z

packages/x-charts/src/models/seriesType/config.ts

    valueType: DefaultizedPieValueType;
  };
+  radar: {
+    seriesInput: DefaultizedProps<RadarSeriesType, 'id'> & { color: string };
+    series: DefaultizedRadarSeriesType;
+    seriesProp: RadarSeriesType;
+    itemIdentifier: RadarItemIdentifier;
+    valueType: number;
+    polar: true;
+  };


Just curious: why do we need a polar: true prop here, but not for the pie chart?

As I see it, the pie chart is also polar where the radius is constant and only the rotation axis changes, isn't it?

I assume it is because pie chart doesn't really use polar coordinates, since it can be much simpler.

JCQuintas · 2025-02-05T12:54:25Z

Is it just me or are the circles not being affected by the area above them?

I assume this got carried over because of how we handle the Line/Area charts as well. Should we start rendering the series fully one on top of eachother and only then adding "click/markhandlers" without styling on top of the charts? 🤔

This would make the styles more consistent, but might create other issues 🫠

JCQuintas · 2025-02-05T13:01:48Z

docs/data/charts/radar/radar.md

+The `metrics` property of `radar` takes an array with one item per corner of the radar.
+This item can either be:
+
+- A string used as the axis label. The other properties will be defaulted.


Suggested change

- A string used as the axis label. The other properties will be defaulted.

- A string used as the axis label. The other properties are populated from the data.

I feel that defaulted usually has a negative connotation, we should probably not use it in documents. Eg: "John defaulted on his loans"

Another option is to fully remove the second phrase as well, it doesn't add much information.

JCQuintas · 2025-02-05T13:07:40Z

packages/x-charts/src/ChartContainer/useChartContainerProps.ts

+    // @ts-ignore
    onAxisClick,
    disableAxisListener,
+    // @ts-ignore
    highlightedItem,
+    // @ts-ignore
    onHighlightChange,
    sx,
    title,
+    // @ts-ignore
    xAxis,
+    // @ts-ignore
    yAxis,
+    // @ts-ignore
    zAxis,
+    // @ts-ignore
+    rotationAxis,
+    // @ts-ignore
+    radiusAxis,


Eh, these are bad 😅

Might as well have "any" as type.

WDYT about moving these type changes to a different PR so we can discuss them in isolation? 🤔

JCQuintas · 2025-02-05T13:11:50Z

packages/x-charts/src/internals/plugins/featurePlugins/useChartPolarAxis/useChartPolarAxis.ts

+  // The effect do not track any value defined synchronously during the 1st render by hooks called after `useChartPolarAxis`
+  // As a consequence, the state generated by the 1st run of this useEffect will always be equal to the initialization one


I didn't understand this comment

JCQuintas · 2025-02-05T13:13:01Z

...s/x-charts/src/internals/plugins/featurePlugins/useChartPolarAxis/useChartPolarAxis.test.tsx

+  testSkipIf(!isJSDOM)('should throw an error when axis have duplicate ids', () => {
+    const expectedError = [
+      'MUI X: The following axis ids are duplicated: qwerty.',
+      'Please make sure that each axis has a unique id.',
+    ].join('\n');
+
+    expect(() =>
+      render(
+        <BarChart
+          xAxis={[
+            { scaleType: 'band', id: 'qwerty', data: ['a', 'b', 'c'] },
+            { scaleType: 'band', id: 'qwerty', data: ['a', 'b', 'c'] },
+          ]}
+          series={[{ data: [1, 2, 3] }]}
+          height={100}
+          width={100}
+        />,
+      ),
+    ).toErrorDev(expectedError);
+  });
+
+  // can't catch render errors in the browser for unknown reason
+  // tried try-catch + error boundary + window onError preventDefault
+  testSkipIf(!isJSDOM)(
+    'should throw an error when axis have duplicate ids across different directions (x,y)',
+    () => {
+      const expectedError = [
+        'MUI X: The following axis ids are duplicated: qwerty.',
+        'Please make sure that each axis has a unique id.',
+      ].join('\n');
+
+      expect(() =>
+        render(
+          <BarChart
+            xAxis={[{ scaleType: 'band', id: 'qwerty', data: ['a', 'b', 'c'] }]}
+            yAxis={[{ id: 'qwerty' }]}
+            series={[{ data: [1, 2, 3] }]}
+            height={100}
+            width={100}
+          />,
+        ),
+      ).toErrorDev(expectedError);
+    },
+  );


I guess you missed the tests here 😆

JCQuintas · 2025-02-05T13:18:22Z

packages/x-charts/src/RadarChart/RadarDataProvider/RadarDataProvider.tsx

+export interface RadarDataProviderProps
+  extends Omit<
+    ChartContainerProps<'radar', RadarPluginSignatures>,
+    'series' | 'plugins' | 'rotationAxis' | 'radiusAxis' | 'dataset'
+  > {
+  /**
+   * The series to display in the bar chart.
+   * An array of [[RadarSeriesType]] objects.
+   */
+  series: MakeOptional<RadarSeriesType, 'type'>[];
+  /**
+   * The configuration of the radar scales.
+   */
+  radar: RadarConfig;
+}


I like the API, sounds reasonable. series handles the series, radar is the chart config 👍

bernardobelchior · 2025-02-05T13:36:44Z

Is it just me or are the circles not being affected by the area above them?

I assume this got carried over because of how we handle the Line/Area charts as well. Should we start rendering the series fully one on top of eachother and only then adding "click/markhandlers" without styling on top of the charts? 🤔

This would make the styles more consistent, but might create other issues 🫠

Hm, good points. I don't have any suggestions at the moment, but I'll let you know if I can think of something.

github-actions · 2025-02-07T08:44:34Z

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

distinguish polar from cartesian

d60a12a

alexfauquette added new feature New feature or request component: charts This is the name of the generic UI component, not the React module! labels Jan 30, 2025

alexfauquette added 6 commits January 31, 2025 12:02

create a polar plugin

21d0c35

basic hooks and type definitions

ea3a57b

Update data providers to create a RadarDataProvider

ebf6158

Add basic radar chart

5d5b532

docs

21fa10d

scripts

a95fddc

alexfauquette force-pushed the radar-chart branch from 37ed9fc to a95fddc Compare January 31, 2025 11:08

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

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

alexfauquette marked this pull request as ready for review January 31, 2025 11:11

alexfauquette added 2 commits January 31, 2025 14:03

Merge remote-tracking branch 'upstream/master' into radar-chart

0f09e9f

fix errors due to highlight modification

e1b7842

alexfauquette force-pushed the radar-chart branch from a589b3a to e1b7842 Compare January 31, 2025 13:43

fix

a254ea1

alexfauquette marked this pull request as draft January 31, 2025 15:28

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

alexfauquette added 2 commits February 4, 2025 09:52

Merge remote-tracking branch 'upstream/master' into radar-chart

98f500e

fix luggins signatures

e1b19f5

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

alexfauquette added 2 commits February 4, 2025 11:14

fix

9f68947

fix

8cf53e3

alexfauquette marked this pull request as ready for review February 4, 2025 10:59

alexfauquette requested a review from JCQuintas February 4, 2025 11:00

bernardobelchior reviewed Feb 5, 2025

View reviewed changes

JCQuintas reviewed Feb 5, 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 Feb 7, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[charts] Introduce the radar chart #16406

[charts] Introduce the radar chart #16406

alexfauquette commented Jan 30, 2025 •

edited by oliviertassinari

Loading

mui-bot commented Jan 30, 2025 •

edited

Loading

github-actions bot commented Jan 31, 2025

codspeed-hq bot commented Jan 31, 2025 •

edited

Loading

github-actions bot commented Feb 4, 2025

bernardobelchior commented Feb 5, 2025

bernardobelchior Feb 5, 2025

JCQuintas Feb 5, 2025

bernardobelchior Feb 5, 2025 •

edited

Loading

bernardobelchior Feb 5, 2025

bernardobelchior Feb 5, 2025

alexfauquette Feb 5, 2025

bernardobelchior Feb 5, 2025

bernardobelchior Feb 5, 2025

JCQuintas Feb 5, 2025

JCQuintas commented Feb 5, 2025

JCQuintas Feb 5, 2025

JCQuintas Feb 5, 2025 •

edited

Loading

JCQuintas Feb 5, 2025

JCQuintas Feb 5, 2025

JCQuintas Feb 5, 2025

bernardobelchior commented Feb 5, 2025

github-actions bot commented Feb 7, 2025


		The radar chart displays a grid behind the series that can be configured with

		- `startAngle` The rotation angle of the entire chart

	- A string used as the axis label. The other properties will be defaulted.
	- A string used as the axis label. The other properties are populated from the data.

		// The effect do not track any value defined synchronously during the 1st render by hooks called after `useChartPolarAxis`
		// As a consequence, the state generated by the 1st run of this useEffect will always be equal to the initialization one

[charts] Introduce the radar chart #16406

Are you sure you want to change the base?

[charts] Introduce the radar chart #16406

Conversation

alexfauquette commented Jan 30, 2025 • edited by oliviertassinari Loading

Main idea

Features

mui-bot commented Jan 30, 2025 • edited Loading

github-actions bot commented Jan 31, 2025

codspeed-hq bot commented Jan 31, 2025 • edited Loading

Merging #16406 will not alter performance

Summary

github-actions bot commented Feb 4, 2025

bernardobelchior commented Feb 5, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bernardobelchior Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JCQuintas commented Feb 5, 2025

Choose a reason for hiding this comment

JCQuintas Feb 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bernardobelchior commented Feb 5, 2025

github-actions bot commented Feb 7, 2025

alexfauquette commented Jan 30, 2025 •

edited by oliviertassinari

Loading

mui-bot commented Jan 30, 2025 •

edited

Loading

codspeed-hq bot commented Jan 31, 2025 •

edited

Loading

bernardobelchior Feb 5, 2025 •

edited

Loading

JCQuintas Feb 5, 2025 •

edited

Loading