[docs] Reproduce npm sparkline #19089

alexfauquette · 2025-08-07T12:46:34Z

I memoised a few stuff in the sparkline bcause it was slow. Might be better to follow the same patern as other charts with a dedicated hook to transform props into props per sub-components

mui-bot · 2025-08-07T12:48:15Z

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

Updated pages:

docs/data/charts/sparkline/sparkline.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	🔺+491B^(+0.16%)	🔺+136B^(+0.14%)
@mui/x-charts-pro	🔺+491B^(+0.12%)	🔺+130B^(+0.11%)
@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%)	🔺+1B^(0.00%)

Details of bundle changes

Generated by 🚫 dangerJS against a38c281

codspeed-hq · 2025-08-07T12:59:30Z

CodSpeed Performance Report

Merging #19089 will improve performances by 9.24%

_{Comparing alexfauquette:npm-sparkline (a38c281) with master (cdea241)¹}

Summary

⚡ 1 improvements
✅ 9 untouched benchmarks

Benchmarks breakdown

	Benchmark	`BASE`	`HEAD`	Change
⚡	`ScatterChartPro with big data amount`	438.6 ms	401.5 ms	+9.24%

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

bernardobelchior

I memoised a few stuff in the sparkline bcause it was slow.

Do you know what part was slow? What caused the slowness?

bernardobelchior · 2025-08-07T12:59:25Z

packages/x-charts/src/SparkLineChart/SparkLineChart.tsx

+        data: Array.from({ length: data.length }, (_, index) => index),
+        hideTooltip: xAxisProps === undefined,
+        ...xAxisProps,


No need to create an array just to not use it after

Suggested change

data: Array.from({ length: data.length }, (_, index) => index),

hideTooltip: xAxisProps === undefined,

...xAxisProps,

hideTooltip: xAxisProps === undefined,

...xAxisProps,

data: xAxisProps.data ?? Array.from({ length: data.length }, (_, index) => index),

bernardobelchior · 2025-08-07T13:02:35Z

docs/data/charts/sparkline/sparkline.md

+## Demo
+
+As an example the sparkline from npm.
+Notice that the accessibility is not down in the chart itself but in a parent context.


What do you mean by this? I couldn't get it

bernardobelchior · 2025-08-07T13:02:48Z

docs/data/charts/sparkline/sparkline.md

+## Demo
+
+As an example the sparkline from npm.
+Notice that the accessibility is not down in the chart itself but in a parent context.


What do you mean by this? I couldn't get it

bernardobelchior · 2025-08-07T13:04:57Z

docs/data/charts/sparkline/sparkline.md

@@ -90,3 +90,10 @@ You can disable clipping by setting `disableClipping` to `true`.
 The example below shows how the line's stroke width, `disableClipping` and `clipAreaOffset` affect the sparkline rendering.

 {{"demo": "SparklineLineWidth.js"}}
+
+## Demo


Should we have a more specific name? E.g., "Weekly downloads example"

I was not sure about where to place it. It does not fit well in a docs page, And I don't want to creat e a demo page for a single sparkline.

Maybe moving it to the to p og the page with an introduction.

Something like

+ A sparkline is a small chart drawn without axes or coordinates, that presents the general shape of a variation in a simplified way. + For example npm downloads display a sparkline of the weekly downloads during the last year. + {{"demo": "NpmDownloadsDemo.js}} ## Basics - A sparkline is a small chart drawn without axes or coordinates, that presents the general shape of a variation in a simplified way. The <SparkLineChart /> requires only the data props which is an array of numbers. You can also switch from line to a bar plot with plotType="bar". {{"demo": "BasicSparkLine.js}}

Something a bit like that

The idea being to add nie looking example on top to say "hey you can do great stuff with that component"

That sounds good to me 👍

alexfauquette · 2025-08-07T13:23:35Z

Do you know what part was slow? What caused the slowness?

Not 100% sure, but the series, xAxis, and yAxis were not memoized. So:

each state update due to our pointer move leads to
a rerender, of that sparkline
which create new series/axes objects
trigger update of the stroe and the entire data pipeline

bernardobelchior · 2025-08-07T13:53:36Z

Compared to the real thing:

Here's how it looks now:

A couple of things I think we can improve:

Reduce font weight in the "Weekly downloads" title
Increase space between the title/download count and the chart
It seems the bottom horizontal line stretches to the right of the end of the chart. It seems npm's as well, but I think it doesn't look as good in our specific example

alexfauquette · 2025-08-08T08:06:03Z

The remaining differences are due to different font-size but it looks fairly similar to my non-designer eyes :)

docs/data/charts/sparkline/NpmSparkLine.js

docs/data/charts/sparkline/NpmSparkLine.tsx

Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>

bernardobelchior · 2025-08-08T08:29:49Z

Should we center the icon vertically with the text?

Screen.Recording.2025-08-08.at.10.28.27.mov

Before:

After:

docs/data/charts/sparkline/sparkline.md

alexfauquette · 2025-08-08T11:43:30Z

Should we center the icon vertically with the text?

I prefer the current solution which align the horizontal bar with the text baseline

…pm-sparkline

JCQuintas · 2025-08-08T11:56:46Z

docs/data/charts/sparkline/generate-sparkline.js

@@ -0,0 +1,64 @@
+// Run `node generate-sparkline.js > weekly-downloads.json` To update the npm chart demo.


Why not make this dynamic? Eg on page load? I fear this will be forgotten here 😆

Why not make this dynamic? Eg on page load? I fear this will be forgotten here 😆

Because data does not need to be up to date. I just made this script to get a realistic look and feel (nothing is more realistic than real data O:) )

I'm not fan of a browser API call. first to avoid bothering npm server, and mostly to avoid having a broken page if they modify their API/their server are for download stats are down

JCQuintas · 2025-08-08T12:03:17Z

packages/x-charts/src/SparkLineChart/SparkLineChart.tsx

-    bottom: clipAreaOffset?.bottom ?? 1,
-    left: clipAreaOffset?.left ?? 1,
-  };
+  const clipPathOffset = React.useMemo(


While I don't have an issue with having them here, shouldn't these memoization be moved to the plugins or series config? 🤔

Those default are chart specific, so I don't see how to put them in pluggins, and putting them in series config seems a bit over engineering the issue

Those default are chart specific

Ah true, because the sparkline uses the line/bar series config 😢
I've been a bit too focused on the sankey lately which is the chart/series 😆

[charts] Provide an accessible demo of sparkline

6b29d00

alexfauquette requested review from bernardobelchior and JCQuintas as code owners August 7, 2025 12:46

alexfauquette added docs Improvements or additions to the documentation. scope: charts Changes related to the charts. labels Aug 7, 2025

docs:typescript

ee73689

bernardobelchior reviewed Aug 7, 2025

View reviewed changes

feedback

7dcf0c0

alexfauquette commented Aug 8, 2025

View reviewed changes

docs/data/charts/sparkline/NpmSparkLine.js Outdated Show resolved Hide resolved

docs/data/charts/sparkline/NpmSparkLine.tsx Outdated Show resolved Hide resolved

Apply suggestions from code review

54547c7

Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>

bernardobelchior approved these changes Aug 8, 2025

View reviewed changes

docs/data/charts/sparkline/sparkline.md Outdated Show resolved Hide resolved

docs/data/charts/sparkline/sparkline.md Outdated Show resolved Hide resolved

text wording

b634a2c

Merge branch 'npm-sparkline' of github.com:alexfauquette/mui-x into n…

a38c281

…pm-sparkline

JCQuintas reviewed Aug 8, 2025

View reviewed changes

JCQuintas approved these changes Aug 8, 2025

View reviewed changes

alexfauquette merged commit 17f3f75 into mui:master Aug 8, 2025
21 checks passed

		@@ -0,0 +1,64 @@
		// Run `node generate-sparkline.js > weekly-downloads.json` To update the npm chart demo.

Uh oh!

[docs] Reproduce npm sparkline #19089

[docs] Reproduce npm sparkline #19089

Uh oh!

Conversation

alexfauquette commented Aug 7, 2025

Uh oh!

mui-bot commented Aug 7, 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 Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging #19089 will improve performances by 9.24%

Summary

Benchmarks breakdown

Footnotes

Uh oh!

bernardobelchior 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 Aug 7, 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!

alexfauquette commented Aug 7, 2025

Uh oh!

bernardobelchior commented Aug 7, 2025

Uh oh!

alexfauquette commented Aug 8, 2025

Uh oh!

Uh oh!

Uh oh!

bernardobelchior commented Aug 8, 2025

Uh oh!

Uh oh!

Uh oh!

alexfauquette commented Aug 8, 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!

Uh oh!

Uh oh!

mui-bot commented Aug 7, 2025 •

edited

Loading

codspeed-hq bot commented Aug 7, 2025 •

edited

Loading

alexfauquette Aug 7, 2025 •

edited

Loading