[charts] Remove intrinsic size requirement #15471
Conversation
JCQuintas
added
enhancement
|
Deploy preview: https://deploy-preview-15471--material-ui-x.netlify.app/ Updated pages: |
CodSpeed Performance ReportMerging #15471 will degrade performances by 14.18%Comparing Summary
Benchmarks breakdown
|
Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
This reverts commit 13a6ce8.
|
|
||
| return ( | ||
| <ResizableContainerRoot | ||
| {...props} | ||
| ownerState={{ width: inWidth, height: inHeight }} | ||
| ref={containerRef} | ||
| > | ||
| {hasIntrinsicSize && props.children} | ||
| {props.children} |
There was a problem hiding this comment.
That let me think that the initial idea of this early return was to skip providers on first render.
If you have no width/height, you will have default width/height used to compute scales, coordinate, ... And when it renders, the width/height get their real size and everything run a second time. The idea was to skip the useless step.
There was a problem hiding this comment.
Providers are now on top of this component, since the datasource is the <ChartDataProvider> now. So I moved the check to the ChartsSurface. This means we can render the <svg/> TAG on the first render, but everything else is only rendered if we have an intrinsic size
packages/x-charts/src/context/SizeProvider/useChartContainerDimensions.ts
Outdated
Show resolved
Hide resolved
| hasInSize || | ||
| !resolveSizeBeforeRender || | ||
| !stateRef.current.initialCompute || |
There was a problem hiding this comment.
If you don't need the resolveSizeBeforeRender you can remove it from the props. The PR change migth impact much more files with the scripts ;)
There was a problem hiding this comment.
yep, had the commit ready, but there would be a lot of file changes
Do you think we need a codemod? I don't think a lot of people would be using it 🙃
There was a problem hiding this comment.
Added a BC to changelog/migration docs
This reverts commit 38e2229.
…mensions.ts Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com> Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
| hasInSize || | ||
| !resolveSizeBeforeRender || | ||
| !stateRef.current.initialCompute || |
Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com> Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com> Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
By removing the height/width attributes and making sure a SVG with some size renders if the size of the parent is unknown, we should be able to fill the entire width and height of the parent container even if it doesn't contain a defined height.
Changelog
resolveSizeBeforeRenderprop from all charts component — Learn more