[charts] Fix spark line not having clip path

[bernardobelchior]

Fixes #15221.

Before we weren't respecting the margin because the drawing area wasn't being clipped, but it is now. The examples below have a margin of 5px, which wasn't respected before.

Before:

After:

[github-actions]

Thanks for adding a type label to the PR! 👍

[mui-bot]

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

Generated by 🚫 dangerJS against 1ec6be6

[codspeed-hq]

CodSpeed Performance Report

Merging #17501 will not alter performance

_{Comparing bernardobelchior:sparkline-add-clip-path (1ec6be6) with master (33e0b7f)}

Summary

✅ 8 untouched benchmarks

[bernardobelchior]

An unfortunate consequence of this change is the problem we see in this visual test.

Before/after:

Since lines have a stroke width of 2, a line that links two points whose y-coordinate is 0 will only have a stroke width of 1 because it will be clipped.

We can probably figure out some ways to solve this issue, but I'm not sure if it would make sense as the clip path is working as expected.

Would appreciate some feedback on this.

[JCQuintas]

Since lines have a stroke width of 2, a line that links two points whose y-coordinate is 0 will only have a stroke width of 1 because it will be clipped.

Does changing the baseline on the bar affects anything?

We could have the clippath on the AreaPlot only, not the line 🤔, but I'm not sure how it would affect the lines with baseline, would we plot things out of the chart? 🤔

[bernardobelchior]

Does changing the baseline on the bar affects anything?

What do you mean by changing the baseline on the bar?

We could have the clippath on the AreaPlot only, not the line 🤔, but I'm not sure how it would affect the lines with baseline, would we plot things out of the chart? 🤔

Yeah, if we don't apply the clip path to the line plot then this issue won't surface, but then there would still be some bleeding outside the drawing area, which is what this PR is supposed to fix.

This is hard to fix because if we change the clip path based on stroke width, we might have weird edge cases when a user uses their own line component instead of the default one.

An option we could consider is having a prop for clipping the path?

[JCQuintas]

Nvm baseline is not present on bar not sparkline 😆

Yeah it is kinda tricky

[alexfauquette]

Only clipping the area could solve also this issue

{plotType === 'bar' && <g clipPath={`url(#${clipPathId})`}><BarPlot skipAnimation slots={slots} slotProps={slotProps} /></g>}

{plotType === 'line' && (
	<React.Fragment>
		<g clipPath={`url(#${clipPathId})`}>
			<AreaPlot skipAnimation slots={slots} slotProps={slotProps} />
		</g>
		<LinePlot skipAnimation slots={slots} slotProps={slotProps} />
		<LineHighlightPlot slots={slots} slotProps={slotProps} />
	</React.Fragment>
)}

But it opens question about when user set the min or max smaller/larger than the actual min/max of the series

A solution could be to add 1 extra pixels top the clipfor line and area

[bernardobelchior]

Only clipping the area could solve also this issue

But it opens question about when user set the min or max smaller/larger than the actual min/max of the series

The problem of only clipping the area is that min won't be respected for lines if the margin is large enough:

When not clipping LinePlot:

When clipping LinePlot:

A solution could be to add 1 extra pixels top the clipfor line and area

I considered that. It's a bit hacky because as soon as the stroke width changes (or the user uses a custom component), those extra pixels are wrong, but it would the fix the issue for now. I'm ok with this, but it would be nice to add a warning in the docs and a escape hatch in case users aren't using the defaults. Not sure how to do it, though 🤔 should we provide ChartsClipPath as a slot?

It wouldn't fix the issue with the highlight, though.

[alexfauquette]

It wouldn't fix the issue with the highlight, though.

In all cases, the line highlight element should be outside of the clip-path

It's a bit hacky because as soon as the stroke width changes (or the user uses a custom component), those extra pixels are wrong, but it would the fix the issue for now. I'm ok with this, but it would be nice to add a warning in the docs and a escape hatch in case users aren't using the defaults. Not sure how to do it, though 🤔 should we provide ChartsClipPath as a slot?

What about adding those props to the sparlkline specifically

• disableClipping: boolean or disableClipPath
• clipPathEntension: number that extends the clipPath by a given number of pixels around the drawing area (default being 1)