[internal] Improve React 19 support#16048
Conversation
Signed-off-by: Armin Mehinovic <4390250+arminmeh@users.noreply.github.com> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Jose Quintas <juniorquintas@gmail.com> Co-authored-by: Lukas <llukas.tyla@gmail.com> Co-authored-by: flavien <flaviendelangle@gmail.com> Co-authored-by: Armin Mehinovic <armin@mui.com> Co-authored-by: Armin Mehinovic <4390250+arminmeh@users.noreply.github.com> Co-authored-by: Bilal Shafi <bilalshafidev@gmail.com>
|
Deploy preview: https://deploy-preview-16048--material-ui-x.netlify.app/ |
| const drawingArea = useDrawingArea(); // Get the dimensions of the chart inside the <svg/>. | ||
|
|
||
| if (!tooltipData || !mousePosition) { | ||
| if (!tooltipData || !mousePosition || !svgRef.current) { |
There was a problem hiding this comment.
This change is unique to v7.
| tooltipData.identifier.dataIndex === undefined || | ||
| tooltipData.value === null | ||
| tooltipData.value === null || | ||
| svgRef.current === null |
There was a problem hiding this comment.
This change is unique to v7.
| if (svgRef.current === null) { | ||
| return undefined; | ||
| } | ||
| const element = svgRef.current; |
There was a problem hiding this comment.
This basically replicated the situation in master to avoid the element being | null below.
| skipAnimation | ||
| slots={slots} | ||
| slotProps={slotProps} | ||
| sx={{ shapeRendering: 'auto' }} |
There was a problem hiding this comment.
This works on v7, but is once again an unknown prop. 🤔 🙈
How do you want to handle it for v7 @mui/xcharts?
Reference for v8/master: https://github.com/mui/mui-x/pull/15769/files#r1892161016
There was a problem hiding this comment.
@JCQuintas WDYT about this case?
For v7 this is a tiny behavior change as sx works. 🤔
Maybe we should just ignore this line on v7 to avoid unwanted changes. 🤔
There was a problem hiding this comment.
just remove is ok, doesn't affect visual it seems
| export interface DrawingProviderProps extends LayoutConfig { | ||
| children: React.ReactNode; | ||
| svgRef: React.RefObject<SVGSVGElement>; | ||
| svgRef: React.RefObject<SVGSVGElement | null>; |
There was a problem hiding this comment.
These changes are unique to v7.
|
@LukasTy charts are not working on the preview. I guess some of the changes broke it. I'll eventually check it this week 😆 |
|
Why argos has a lot of changes and additions? 🤔 |
It might have been a flaky run/diff on Argos side. 🤔 |
@JCQuintas it could have been produced by the run on react 18.3.1. 🙈 |
Cherry-pick #15769.
@mui/internal-test-utils@testing-library/react,sinon,jsdom,mocha