Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[charts] Remove intrinsic size requirement #15471

Merged
Merged
23 changes: 21 additions & 2 deletions packages/x-charts/src/ChartContainer/ResizableContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import PropTypes from 'prop-types';
import { styled } from '@mui/material/styles';
import { useSize } from '../context/SizeProvider';
import type { SizeContextState } from '../context/SizeProvider';
import { useDrawingArea } from '../hooks';

/**
* Wrapping div that take the shape of its parent.
Expand All @@ -22,12 +23,30 @@ export const ResizableContainerRoot = styled('div', {
alignItems: 'center',
justifyContent: 'center',
overflow: 'hidden',
'&>svg': {
'& svg': {
JCQuintas marked this conversation as resolved.
Show resolved Hide resolved
width: '100%',
height: '100%',
},
}));

/**
* This svg will fill the parent container. And prevent an SVG rendered with size 0.
*
* @ignore - internal component.
*/
function SvgSize() {
const { width, height, left, right, top, bottom } = useDrawingArea();

const svgView = {
width: width + left + right,
height: height + top + bottom,
x: 0,
y: 0,
};

return <svg viewBox={`${svgView.x} ${svgView.y} ${svgView.width} ${svgView.height}`} />;
}

/**
* Wrapping div that take the shape of its parent.
*
Expand All @@ -42,7 +61,7 @@ function ResizableContainer(props: { children: React.ReactNode }) {
ownerState={{ width: inWidth, height: inHeight }}
ref={containerRef}
>
{hasIntrinsicSize && props.children}
{hasIntrinsicSize ? props.children : <SvgSize />}
</ResizableContainerRoot>
);
}
Expand Down
2 changes: 0 additions & 2 deletions packages/x-charts/src/ChartsSurface/ChartsSurface.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ const ChartsSurface = React.forwardRef<SVGSVGElement, ChartsSurfaceProps>(functi

return (
<ChartsSurfaceStyles
width={svgWidth}
height={svgHeight}
viewBox={`${svgView.x} ${svgView.y} ${svgView.width} ${svgView.height}`}
className={className}
{...other}
Expand Down
6 changes: 1 addition & 5 deletions packages/x-charts/src/context/SizeProvider/SizeProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,7 @@ import { useChartContainerDimensions } from './useChartContainerDimensions';
* This provider is also responsible for resolving the size of the container before rendering and if the parent size changes.
*/
function SizeProvider(props: SizeProviderProps) {
const dimensions = useChartContainerDimensions(
props.width,
props.height,
props.resolveSizeBeforeRender,
);
const dimensions = useChartContainerDimensions(props.width, props.height);

const value = React.useMemo(
() => ({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@ import * as React from 'react';
import useEnhancedEffect from '@mui/utils/useEnhancedEffect';
import ownerWindow from '@mui/utils/ownerWindow';

export const useChartContainerDimensions = (
inWidth?: number,
inHeight?: number,
resolveSizeBeforeRender?: boolean,
) => {
const MAX_COMPUTE_RUN = 10;

export const useChartContainerDimensions = (inWidth?: number, inHeight?: number) => {
const hasInSize = inWidth !== undefined && inHeight !== undefined;
const stateRef = React.useRef({ displayError: false, initialCompute: true, computeRun: 0 });
const rootRef = React.useRef<HTMLDivElement>(null);
Expand Down Expand Up @@ -48,9 +46,8 @@ export const useChartContainerDimensions = (
// computeRun is used to avoid infinite loops.
if (
hasInSize ||
!resolveSizeBeforeRender ||
!stateRef.current.initialCompute ||
Comment on lines 48 to 49
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🙃

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a BC to changelog/migration docs

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be enough

stateRef.current.computeRun > 20
stateRef.current.computeRun > MAX_COMPUTE_RUN
) {
return;
}
Expand All @@ -61,7 +58,7 @@ export const useChartContainerDimensions = (
} else if (stateRef.current.initialCompute) {
stateRef.current.initialCompute = false;
}
}, [width, height, computeSize, resolveSizeBeforeRender, hasInSize]);
}, [width, height, computeSize, hasInSize]);

useEnhancedEffect(() => {
if (hasInSize) {
Expand Down Expand Up @@ -111,15 +108,14 @@ export const useChartContainerDimensions = (
stateRef.current.displayError = false;
}
}

const finalWidth = inWidth ?? width;
const finalHeight = inHeight ?? height;

return {
containerRef: rootRef,
width: finalWidth,
height: finalHeight,
hasIntrinsicSize: Boolean(finalWidth && finalHeight),
hasIntrinsicSize: Boolean(finalWidth > 0 && finalHeight > 0),
JCQuintas marked this conversation as resolved.
Show resolved Hide resolved
inWidth,
inHeight,
};
Expand Down