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] Decouple margin and axis-size #16418

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
644cd1f
general improvements
JCQuintas Feb 5, 2025
7ee2fe4
multiple axis + axis offset + axis size
JCQuintas Feb 5, 2025
977dd34
Merge branch 'master' into axis-size-poc
JCQuintas Feb 6, 2025
f48e90b
todo
JCQuintas Feb 6, 2025
caab7bb
calculate margins + axis size
JCQuintas Feb 6, 2025
1b961f7
defaultize margin fn
JCQuintas Feb 6, 2025
74f320f
remove "calculatelegendmargin" behavior
JCQuintas Feb 6, 2025
45d9e94
fix missing filechange
JCQuintas Feb 6, 2025
a73e755
try document
JCQuintas Feb 6, 2025
acc67bd
Merge commit 'd8f269155f3ca08f72cb3f40a142005db0e1f6e1' into axis-siz…
JCQuintas Feb 7, 2025
0747b9e
use params not state
JCQuintas Feb 7, 2025
ec16952
add comment
JCQuintas Feb 7, 2025
6424b51
types
JCQuintas Feb 7, 2025
5bd7329
Merge commit '798b73c92c614b042514050c85edbdfccfce0584' into axis-siz…
JCQuintas Feb 7, 2025
1cf7de6
Revert "[charts] Document plugins for internal use"
JCQuintas Feb 7, 2025
75916db
empty sides
JCQuintas Feb 7, 2025
f1eb5dd
render based on state rather than props
JCQuintas Feb 7, 2025
0daad2b
Merge commit '587f7c5757e0d66222ea1f3d37c923a0814e9d86' into axis-siz…
JCQuintas Feb 7, 2025
f1c99ba
Merge commit 'e71e83180666ae867c95b2dccad6653619aa7e2c' into axis-siz…
JCQuintas Feb 11, 2025
e21062d
fix cycle
JCQuintas Feb 11, 2025
ec5f0a7
Merge axisprops
JCQuintas Feb 11, 2025
50d753c
none
JCQuintas Feb 11, 2025
419a688
fix pie charts
JCQuintas Feb 11, 2025
cd1b875
react to svg width/height changes
JCQuintas Feb 11, 2025
7b14f1f
Remove left/right/top/bottomAxis props
JCQuintas Feb 12, 2025
7ae9799
define offset on defaultize axis
JCQuintas Feb 12, 2025
3d228db
move params to axisconfig instead of axisprops
JCQuintas Feb 12, 2025
90cc7f1
move selectors
JCQuintas Feb 12, 2025
818a4ed
Merge commit '9b36487c9bf35b9166b7c83909164893cb92b4b4' into axis-siz…
JCQuintas Feb 12, 2025
5a51ef7
fix flicker
JCQuintas Feb 12, 2025
9e30455
add docs
JCQuintas Feb 13, 2025
e9863f5
Merge branch 'master' into axis-size-poc
JCQuintas Feb 13, 2025
e4beced
fix types
JCQuintas Feb 13, 2025
f3b4c70
fix types cycle
JCQuintas Feb 13, 2025
2b8a0f3
fix failing tests
JCQuintas Feb 13, 2025
d8617cf
Apply suggestions from code review
JCQuintas Feb 13, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 129 additions & 53 deletions packages/x-charts/src/ChartsAxis/ChartsAxis.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,32 +11,37 @@ import {
ChartsYAxisProps,
} from '../models/axis';
import { useXAxes, useYAxes } from '../hooks';
import { DEFAULT_AXIS_SIZE } from '../constants';

export interface ChartsAxisProps {
/**
* Indicate which axis to display the top of the charts.
* Can be a string (the id of the axis) or an object `ChartsXAxisProps`.
* @default null
* @deprecated Use `xAxis[].position="top"` instead.
JCQuintas marked this conversation as resolved.
Show resolved Hide resolved
*/
topAxis?: null | string | ChartsXAxisProps;
/**
* Indicate which axis to display the right of the charts.
* Can be a string (the id of the axis) or an object `ChartsYAxisProps`.
* @default null
* @deprecated Use `yAxis[].position="right"` instead.
*/
rightAxis?: null | string | ChartsYAxisProps;
/**
* Indicate which axis to display the bottom of the charts.
* Can be a string (the id of the axis) or an object `ChartsXAxisProps`.
* @default xAxisIds[0] The id of the first provided axis
* @deprecated Use `xAxis[].position="bottom"` instead.
*/
bottomAxis?: null | string | ChartsXAxisProps;
/**
* Indicate which axis to display the left of the charts.
* Can be a string (the id of the axis) or an object `ChartsYAxisProps`.
* @default yAxisIds[0] The id of the first provided axis
* @deprecated Use `yAxis[].position="left"` instead.
*/
leftAxis?: null | string | ChartsYAxisProps;
/**
* Indicate which axis to display the right of the charts.
* Can be a string (the id of the axis) or an object `ChartsYAxisProps`.
* @default null
*/
rightAxis?: null | string | ChartsYAxisProps;
/**
* Overridable component slots.
* @default {}
Expand All @@ -51,13 +56,12 @@ export interface ChartsAxisProps {

const getAxisId = (
propsValue: undefined | null | AxisId | ChartsXAxisProps | ChartsYAxisProps,
defaultAxisId?: AxisId,
): AxisId | null => {
if (propsValue == null) {
return null;
}
if (typeof propsValue === 'object') {
return propsValue.axisId ?? defaultAxisId ?? null;
return propsValue.axisId ?? null;
}
return propsValue;
};
Expand All @@ -69,7 +73,6 @@ const mergeProps = (
) => {
return typeof axisConfig === 'object'
? {
...axisConfig,
slots: { ...slots, ...axisConfig?.slots },
slotProps: { ...slotProps, ...axisConfig?.slotProps },
}
Expand All @@ -86,58 +89,131 @@ const mergeProps = (
* - [ChartsAxis API](https://mui.com/x/api/charts/charts-axis/)
*/
function ChartsAxis(props: ChartsAxisProps) {
const { topAxis, leftAxis, rightAxis, bottomAxis, slots, slotProps } = props;
const {
topAxis: topAxisProp,
rightAxis: rightAxisProp,
bottomAxis: bottomAxisProp,
leftAxis: leftAxisProp,
slots,
slotProps,
} = props;
const { xAxis, xAxisIds } = useXAxes();
const { yAxis, yAxisIds } = useYAxes();

const leftId = getAxisId(leftAxis === undefined ? yAxisIds[0] : leftAxis, yAxisIds[0]);
const bottomId = getAxisId(bottomAxis === undefined ? xAxisIds[0] : bottomAxis, xAxisIds[0]);
const topId = getAxisId(topAxis, xAxisIds[0]);
const rightId = getAxisId(rightAxis, yAxisIds[0]);
const topId = getAxisId(topAxisProp);
const rightId = getAxisId(rightAxisProp);
const bottomId = getAxisId(bottomAxisProp);
const leftId = getAxisId(leftAxisProp);

if (topId !== null && !xAxis[topId]) {
throw new Error(
[
`MUI X: id used for top axis "${topId}" is not defined.`,
`Available ids are: ${xAxisIds.join(', ')}.`,
].join('\n'),
);
}
if (leftId !== null && !yAxis[leftId]) {
throw new Error(
[
`MUI X: id used for left axis "${leftId}" is not defined.`,
`Available ids are: ${yAxisIds.join(', ')}.`,
].join('\n'),
);
if (process.env.NODE_ENV !== 'production') {
if (topId !== null && !xAxis[topId]) {
throw new Error(
[
`MUI X: id used for top axis "${topId}" is not defined.`,
`Available ids are: ${xAxisIds.join(', ')}.`,
].join('\n'),
);
}
if (rightId !== null && !yAxis[rightId]) {
throw new Error(
[
`MUI X: id used for right axis "${rightId}" is not defined.`,
`Available ids are: ${yAxisIds.join(', ')}.`,
].join('\n'),
);
}
if (bottomId !== null && !xAxis[bottomId]) {
throw new Error(
[
`MUI X: id used for bottom axis "${bottomId}" is not defined.`,
`Available ids are: ${xAxisIds.join(', ')}.`,
].join('\n'),
);
}
if (leftId !== null && !yAxis[leftId]) {
throw new Error(
[
`MUI X: id used for left axis "${leftId}" is not defined.`,
`Available ids are: ${yAxisIds.join(', ')}.`,
].join('\n'),
);
}
}
if (rightId !== null && !yAxis[rightId]) {
throw new Error(
[
`MUI X: id used for right axis "${rightId}" is not defined.`,
`Available ids are: ${yAxisIds.join(', ')}.`,
].join('\n'),
);
}
if (bottomId !== null && !xAxis[bottomId]) {
throw new Error(
[
`MUI X: id used for bottom axis "${bottomId}" is not defined.`,
`Available ids are: ${xAxisIds.join(', ')}.`,
].join('\n'),
);
}
const topAxisProps = mergeProps(topAxis, slots, slotProps);
const bottomAxisProps = mergeProps(bottomAxis, slots, slotProps);
const leftAxisProps = mergeProps(leftAxis, slots, slotProps);
const rightAxisProps = mergeProps(rightAxis, slots, slotProps);

const topAxes = topId
? [xAxis[topId]]
: xAxisIds.map((id) => xAxis[id]).filter((axis) => axis.position === 'top');
const rightAxes = rightId
? [yAxis[rightId]]
: yAxisIds.map((id) => yAxis[id]).filter((axis) => axis.position === 'right');
const bottomAxes = bottomId
? [xAxis[bottomId]]
: xAxisIds.map((id) => xAxis[id]).filter((axis) => axis.position === 'bottom');
const leftAxes = leftId
? [yAxis[leftId]]
: yAxisIds.map((id) => yAxis[id]).filter((axis) => axis.position === 'left');

const completeTopAxisProps = topAxes.map((axis) => ({
...axis,
...mergeProps(axis, slots, slotProps),
}));
const completeRightAxisProps = rightAxes.map((axis) => ({
...axis,
...mergeProps(axis, slots, slotProps),
}));
const completeBottomAxisProps = (
bottomAxes.length === 0 && !xAxis[xAxisIds[0]].position ? [xAxis[xAxisIds[0]]] : bottomAxes
).map((axis) => ({ ...axis, ...mergeProps(axis, slots, slotProps) }));
const completeLeftAxisProps = (
leftAxes.length === 0 && !yAxis[yAxisIds[0]].position ? [yAxis[yAxisIds[0]]] : leftAxes
).map((axis) => ({ ...axis, ...mergeProps(axis, slots, slotProps) }));
Copy link
Member

Choose a reason for hiding this comment

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

This could be simplified if you setup by default axis.position === 'bottom' for the first item in axis pluggin


return (
<React.Fragment>
{topId && <ChartsXAxis {...topAxisProps} position="top" axisId={topId} />}
{bottomId && <ChartsXAxis {...bottomAxisProps} position="bottom" axisId={bottomId} />}
{leftId && <ChartsYAxis {...leftAxisProps} position="left" axisId={leftId} />}
{rightId && <ChartsYAxis {...rightAxisProps} position="right" axisId={rightId} />}
{completeTopAxisProps.map((axis, i, arr) => (
<ChartsXAxis
key={axis.id}
{...axis}
position="top"
axisId={axis.id}
offset={arr
.slice(0, i)
.reduce((acc, curr) => acc + (curr.height ?? DEFAULT_AXIS_SIZE), axis.offset ?? 0)}
/>
))}
{completeRightAxisProps.map((axis, i, arr) => (
<ChartsYAxis
key={axis.id}
{...axis}
position="right"
axisId={axis.id}
offset={arr
.slice(0, i)
.reduce((acc, curr) => acc + (curr.width ?? DEFAULT_AXIS_SIZE), axis.offset ?? 0)}
/>
))}
{completeBottomAxisProps.map((axis, i, arr) => (
<ChartsXAxis
key={axis.id}
{...axis}
position="bottom"
axisId={axis.id}
offset={arr
.slice(0, i)
.reduce((acc, curr) => acc + (curr.height ?? DEFAULT_AXIS_SIZE), axis.offset ?? 0)}
/>
))}
{completeLeftAxisProps.map((axis, i, arr) => (
<ChartsYAxis
key={axis.id}
{...axis}
position="left"
axisId={axis.id}
offset={arr
.slice(0, i)
.reduce((acc, curr) => acc + (curr.width ?? DEFAULT_AXIS_SIZE), axis.offset ?? 0)}
/>
))}
</React.Fragment>
);
}
Expand Down
66 changes: 37 additions & 29 deletions packages/x-charts/src/ChartsXAxis/ChartsXAxis.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ const defaultProps = {
disableLine: false,
disableTicks: false,
tickSize: 6,
offset: 0,
} as const;

/**
Expand Down Expand Up @@ -136,6 +137,7 @@ function ChartsXAxis(inProps: ChartsXAxisProps) {
tickPlacement,
tickLabelPlacement,
sx,
offset,
} = defaultizedProps;

const theme = useTheme();
Expand Down Expand Up @@ -212,44 +214,50 @@ function ChartsXAxis(inProps: ChartsXAxisProps) {
}
return (
<XAxisRoot
transform={`translate(0, ${position === 'bottom' ? top + height : top})`}
transform={`translate(0, ${position === 'bottom' ? top + height + offset : top - offset})`}
className={classes.root}
sx={sx}
>
{!disableLine && (
<Line x1={left} x2={left + width} className={classes.line} {...slotProps?.axisLine} />
)}

{xTicksWithDimension.map(({ formattedValue, offset, labelOffset, skipLabel }, index) => {
const xTickLabel = labelOffset ?? 0;
const yTickLabel = positionSign * (tickSize + 3);
{xTicksWithDimension.map(
({ formattedValue, offset: tickOffset, labelOffset, skipLabel }, index) => {
const xTickLabel = labelOffset ?? 0;
const yTickLabel = positionSign * (tickSize + 3);

const showTick = instance.isPointInside({ x: offset, y: -1 }, { direction: 'x' });
const showTickLabel = instance.isPointInside(
{ x: offset + xTickLabel, y: -1 },
{ direction: 'x' },
);
return (
<g key={index} transform={`translate(${offset}, 0)`} className={classes.tickContainer}>
{!disableTicks && showTick && (
<Tick
y2={positionSign * tickSize}
className={classes.tick}
{...slotProps?.axisTick}
/>
)}
const showTick = instance.isPointInside({ x: tickOffset, y: -1 }, { direction: 'x' });
const showTickLabel = instance.isPointInside(
{ x: tickOffset + xTickLabel, y: -1 },
{ direction: 'x' },
);
return (
<g
key={index}
transform={`translate(${tickOffset}, 0)`}
className={classes.tickContainer}
>
{!disableTicks && showTick && (
<Tick
y2={positionSign * tickSize}
className={classes.tick}
{...slotProps?.axisTick}
/>
)}

{formattedValue !== undefined && !skipLabel && showTickLabel && (
<TickLabel
x={xTickLabel}
y={yTickLabel}
{...axisTickLabelProps}
text={formattedValue.toString()}
/>
)}
</g>
);
})}
{formattedValue !== undefined && !skipLabel && showTickLabel && (
<TickLabel
x={xTickLabel}
y={yTickLabel}
{...axisTickLabelProps}
text={formattedValue.toString()}
/>
)}
</g>
);
},
)}

{label && (
<g className={classes.label}>
Expand Down
15 changes: 11 additions & 4 deletions packages/x-charts/src/ChartsYAxis/ChartsYAxis.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const defaultProps = {
disableLine: false,
disableTicks: false,
tickSize: 6,
offset: 0,
} as const;

/**
Expand Down Expand Up @@ -79,6 +80,7 @@ function ChartsYAxis(inProps: ChartsYAxisProps) {
tickInterval,
tickLabelInterval,
sx,
offset,
} = defaultizedProps;

const theme = useTheme();
Expand Down Expand Up @@ -156,6 +158,7 @@ function ChartsYAxis(inProps: ChartsYAxisProps) {

const domain = yScale.domain();
const ordinalAxis = isBandScale(yScale);

// Skip axis rendering if no data is available
// - The domain is an empty array for band/point scales.
// - The domains contains Infinity for continuous scales.
Expand All @@ -165,28 +168,32 @@ function ChartsYAxis(inProps: ChartsYAxisProps) {

return (
<YAxisRoot
transform={`translate(${position === 'right' ? left + width : left}, 0)`}
transform={`translate(${position === 'right' ? left + width + offset : left - offset}, 0)`}
className={classes.root}
sx={sx}
>
{!disableLine && (
<Line y1={top} y2={top + height} className={classes.line} {...lineSlotProps} />
)}

{yTicks.map(({ formattedValue, offset, labelOffset, value }, index) => {
{yTicks.map(({ formattedValue, offset: tickOffset, labelOffset, value }, index) => {
const xTickLabel = positionSign * (tickSize + 2);
const yTickLabel = labelOffset;
const skipLabel =
typeof tickLabelInterval === 'function' && !tickLabelInterval?.(value, index);

const showLabel = instance.isPointInside({ x: -1, y: offset }, { direction: 'y' });
const showLabel = instance.isPointInside({ x: -1, y: tickOffset }, { direction: 'y' });

if (!showLabel) {
return null;
}

return (
<g key={index} transform={`translate(0, ${offset})`} className={classes.tickContainer}>
<g
key={index}
transform={`translate(0, ${tickOffset})`}
className={classes.tickContainer}
>
{!disableTicks && (
<Tick
x2={positionSign * tickSize}
Expand Down
Loading
Loading