Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions docs/data/charts/zoom-and-pan/ZoomSlider.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ export default function ZoomSlider() {
height={400}
margin={{ bottom: 40 }}
initialZoom={[
{ axisId: 'x', start: 10, end: 90 },
{ axisId: 'x', start: 45, end: 55 },
{ axisId: 'x2', start: 30, end: 70 },
{ axisId: 'y', start: 10, end: 90 },
{ axisId: 'y', start: 40, end: 60 },
{ axisId: 'y2', start: 30, end: 70 },
]}
/>
Expand Down
4 changes: 2 additions & 2 deletions docs/data/charts/zoom-and-pan/ZoomSlider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ export default function ZoomSlider() {
height={400}
margin={{ bottom: 40 }}
initialZoom={[
{ axisId: 'x', start: 10, end: 90 },
{ axisId: 'x', start: 45, end: 55 },
{ axisId: 'x2', start: 30, end: 70 },
{ axisId: 'y', start: 10, end: 90 },
{ axisId: 'y', start: 40, end: 60 },
Comment on lines +53 to +55
Copy link
Member Author

Choose a reason for hiding this comment

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

Updated demo so it's easier to test the range selection

{ axisId: 'y2', start: 30, end: 70 },
]}
/>
Expand Down
1 change: 1 addition & 0 deletions packages/x-charts-pro/src/BarChartPro/BarChartPro.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ BarChartPro.propTypes = {
current: PropTypes.shape({
exportAsImage: PropTypes.func.isRequired,
exportAsPrint: PropTypes.func.isRequired,
setAxisZoomData: PropTypes.func.isRequired,
setZoomData: PropTypes.func.isRequired,
}),
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import {
useStore,
ZoomData,
ZOOM_SLIDER_MARGIN,
selectorChartRawAxis,
ChartState,
} from '@mui/x-charts/internals';
import { styled } from '@mui/material/styles';
import { useXAxes, useYAxes } from '@mui/x-charts/hooks';
Expand All @@ -29,6 +31,7 @@ const ZoomSliderTrack = styled('rect')(({ theme }) => ({
theme.palette.mode === 'dark'
? (theme.vars || theme).palette.grey[800]
: (theme.vars || theme).palette.grey[300],
cursor: 'crosshair',
Copy link
Member

Choose a reason for hiding this comment

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

A bit weird but ok 🤷

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I don't love it either. I'm in contact with @noraleonte and @kenanyusuf to discuss what a better cursor would be.

We're having a hard time finding a better solution.

Copy link
Member Author

Choose a reason for hiding this comment

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

There you go 😛
#17977

},
}));

Expand Down Expand Up @@ -122,13 +125,16 @@ export function ChartAxisZoomSlider({ axisDirection, axisId }: ChartZoomSliderPr

return (
<g transform={`translate(${x} ${y})`}>
<ZoomSliderTrack
<ChartAxisZoomSliderTrack
x={axisDirection === 'x' ? 0 : backgroundRectOffset}
y={axisDirection === 'x' ? backgroundRectOffset : 0}
height={axisDirection === 'x' ? ZOOM_SLIDER_TRACK_SIZE : drawingArea.height}
width={axisDirection === 'x' ? drawingArea.width : ZOOM_SLIDER_TRACK_SIZE}
rx={ZOOM_SLIDER_TRACK_SIZE / 2}
ry={ZOOM_SLIDER_TRACK_SIZE / 2}
axisId={axisId}
axisDirection={axisDirection}
reverse={reverse}
/>
<ChartAxisZoomSliderActiveTrack
zoomData={zoomData}
Expand All @@ -141,6 +147,140 @@ export function ChartAxisZoomSlider({ axisDirection, axisId }: ChartZoomSliderPr
);
}

interface ChartAxisZoomSliderTrackProps extends React.ComponentProps<'rect'> {
axisId: AxisId;
axisDirection: 'x' | 'y';
reverse: boolean;
}

function ChartAxisZoomSliderTrack({
Copy link
Member Author

Choose a reason for hiding this comment

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

This file is getting huge. I'll move the components to other files in a follow-up.

Copy link
Member Author

Choose a reason for hiding this comment

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

axisId,
axisDirection,
reverse,
...other
}: ChartAxisZoomSliderTrackProps) {
const ref = React.useRef<SVGRectElement>(null);
const { instance, svgRef } = useChartContext<[UseChartProZoomSignature]>();
const store = useStore<[UseChartProZoomSignature]>();

const onPointerDown = function onPointerDown(event: React.PointerEvent<SVGRectElement>) {
const rect = ref.current;
const element = svgRef.current;

if (!rect || !element) {
return;
}

const pointerDownPoint = getSVGPoint(element, event);
let zoomFromPointerDown = calculateZoomFromPoint(store.getSnapshot(), axisId, pointerDownPoint);
Copy link
Member

Choose a reason for hiding this comment

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

maybe something like firstZoomPoint and currentZoomPoint for the move one?

Copy link
Member

Choose a reason for hiding this comment

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

or initialZoomPoint

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if firstZoomPoint is better because this isn't a point, it's the zoom value (ranging from 0 to 100) of the pointer down event. I could call it initialZoom, but not sure if that's less confusing 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll merge this PR as it's been approved, but we can keep discussing and I'll update the code in a follow-up PR.

Copy link
Member

Choose a reason for hiding this comment

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

My line of thought was that it is a point in the zoom "scale"

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we can look at it that way. I'm afraid it might be confused with the SVGPoint that we're using in the same scope, that's why I initially tried suffixing it with Zoom


if (zoomFromPointerDown === null) {
return;
}

const { minStart, maxEnd } = selectorChartAxisZoomOptionsLookup(store.getSnapshot(), axisId);

// Ensure the zoomFromPointerDown is within the min and max range
zoomFromPointerDown = Math.max(Math.min(zoomFromPointerDown, maxEnd), minStart);

let pointerMoved = false;

const onPointerMove = rafThrottle(function onPointerMove(pointerMoveEvent: PointerEvent) {
Copy link
Member

Choose a reason for hiding this comment

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

It's a bit weird to define the onPointerMove inside the onPointerDown

Does removing the pointerMove/pointerDown from document makes a difference in terms of performances?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a bit weird to define the onPointerMove inside the onPointerDown

Why? I think it makes sense. We only need to listen to the pointer move events after a pointer down happens, so it makes sense that we only add the listener after a pointer down event.

We could add the event listener regardless, but then we're calling a function on every move just to do nothing. Seems a bit useless, IMO.

Does removing the pointerMove/pointerDown from document makes a difference in terms of performances?

It's probably negligible, I didn't test it.

const pointerMovePoint = getSVGPoint(element, pointerMoveEvent);
const zoomFromPointerMove = calculateZoomFromPoint(
store.getSnapshot(),
axisId,
pointerMovePoint,
);

if (zoomFromPointerMove === null) {
return;
}

pointerMoved = true;
const zoomOptions = selectorChartAxisZoomOptionsLookup(store.getSnapshot(), axisId);

instance.setAxisZoomData(axisId, (prevZoomData) => {
if (zoomFromPointerMove > zoomFromPointerDown) {
const end = calculateZoomEnd(
zoomFromPointerMove,
{ ...prevZoomData, start: zoomFromPointerDown },
zoomOptions,
);

/* If the starting point is too close to the end that minSpan wouldn't be respected, we need to update the
* start point. */
const start = calculateZoomStart(
zoomFromPointerDown,
{ ...prevZoomData, start: zoomFromPointerDown, end },
zoomOptions,
);

return { ...prevZoomData, start, end };
}

const start = calculateZoomStart(
zoomFromPointerMove,
{ ...prevZoomData, end: zoomFromPointerDown },
zoomOptions,
);

/* If the starting point is too close to the start that minSpan wouldn't be respected, we need to update the
* start point. */
const end = calculateZoomEnd(
zoomFromPointerDown,
{ ...prevZoomData, start, end: zoomFromPointerDown },
zoomOptions,
);

return { ...prevZoomData, start, end };
});
});

const onPointerUp = function onPointerUp(pointerUpEvent: PointerEvent) {
rect.releasePointerCapture(pointerUpEvent.pointerId);
rect.removeEventListener('pointermove', onPointerMove);
document.removeEventListener('pointerup', onPointerUp);

if (pointerMoved) {
return;
}

// If the pointer didn't move, we still need to respect the zoom constraints (minSpan, etc.)
// In that case, we assume the start to be the pointerZoom and calculate the end.
const pointerUpPoint = getSVGPoint(element, pointerUpEvent);
const zoomFromPointerUp = calculateZoomFromPoint(store.getSnapshot(), axisId, pointerUpPoint);

if (zoomFromPointerUp === null) {
return;
}

const zoomOptions = selectorChartAxisZoomOptionsLookup(store.getSnapshot(), axisId);

instance.setAxisZoomData(axisId, (prev) => ({
...prev,
start: zoomFromPointerUp,
end: calculateZoomEnd(zoomFromPointerUp, prev, zoomOptions),
}));
};

event.preventDefault();
event.stopPropagation();

rect.setPointerCapture(event.pointerId);
document.addEventListener('pointerup', onPointerUp);
Copy link
Member Author

Choose a reason for hiding this comment

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

Couldn't make it work when pointerup listener was set on rect. The event wasn't being triggered in some cases.

Copy link
Member

Choose a reason for hiding this comment

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

this is normal

rect.addEventListener('pointermove', onPointerMove);

instance.setAxisZoomData(axisId, (prev) => ({
...prev,
start: zoomFromPointerDown,
end: zoomFromPointerDown,
}));
Comment on lines +274 to +278
Copy link
Member

Choose a reason for hiding this comment

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

What's the purpose of this setAxisZoomData?

The only effect is to clear the chart between pointer-down and pointer-move

image

On this topic, Echarts has a much better interaction. The pointer down and move only show the future range. And it's on pointer up that the zoom get applied. For this feature, they do not respect the min/max span.

Which seems better than having a jumping selection

Capture.video.du.2025-05-23.09-46-27.mp4

Copy link
Member Author

Choose a reason for hiding this comment

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

What's the purpose of this setAxisZoomData?

It's a higher level function that setZoomData that allows setting the zoom data for a specific axis, instead of having to iterate over all axes to find the one you want and update it.

IMO, our setZoomData allows too much control. For example, you can disrepect a minStart or minSpan with it. I'm not sure we should allow users to do that.

On this topic, Echarts has a much better interaction. The pointer down and move only show the future range. And it's on pointer up that the zoom get applied.

We could do something like that as well. @kenanyusuf @noraleonte what's your take? Should I try to do something like this?

For this feature, they do not respect the min/max span.

Is that a good idea? What's the point of min/max span if you can disrepect it?

Copy link
Member

Choose a reason for hiding this comment

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

Is that a good idea? What's the point of min/max span if you can disrepect it?

It's feasible to respect if with an interaction like: if the preview does not respect min/max span we ignore it, or have a different visualisation

Even if we dont not respect it with this interaction, it's usefull for the other interactions.

For me the minSpan is useful to avoid user zooming infinitely.

IMO, our setZoomData allows too much control. For example, you can disrepect a minStart or minSpan with it. I'm not sure we should allow users to do that.

It seems the setAxisZoomData does not respect minSpan since you can call it with start = end (which means span=0) 🙈

Copy link
Member

Choose a reason for hiding this comment

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

On this topic, Echarts has a much better interaction. The pointer down and move only show the future range. And it's on pointer up that the zoom get applied.

I don't think it is better though 😆
It feels a bit odd that the selection is only applied after pointer up

Copy link
Member Author

Choose a reason for hiding this comment

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

It's feasible to respect if with an interaction like: if the preview does not respect min/max span we ignore it, or have a different visualisation

I'm ok with not respecting min/max span while dragging, but after pointer release we ensure those constraints are met.

Even if we dont not respect it with this interaction, it's usefull for the other interactions.

For me the minSpan is useful to avoid user zooming infinitely.

Yeah, but if the range selection doesn't respect the min/max span, then a user can zoom in too much.

Plus, I think it would be pretty unexpected to for the range selection to bypass min/max span. I think users wouldn't expect it.

It seems the setAxisZoomData does not respect minSpan since you can call it with start = end (which means span=0) 🙈

Yeah, at the moment we aren't respecting it. What I meant is that I think we should provide higher-level functions that respect it. E.g., instead of setAxisZoomData, you could have moveAxisZoomStart, moveAxisZoomEnd, moveAxisZoomRange, zoomIn, zoomOut, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it is better though 😆
It feels a bit odd that the selection is only applied after pointer up

UX-wise I think it's debatable, but for performance I think updating the chart only after pointer up would be much better.

Copy link
Member

Choose a reason for hiding this comment

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

UX-wise, it's fairly similar to the brush zoom
like in rechart Applied on the overview instead of the drawing area

Copy link
Member

Choose a reason for hiding this comment

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

I would argue it is similar, but fairly different.

The brush zoom you are selecting part of your content. It makes sense for it to only apply the selection after the fact, since you would change the value being currently selected as you move the mouse.

The slider is a control, an abstraction over the zoom, which the user already implies has a meaning, and which in turn allows it to directly affect the content.

See that in the first case, the control is static, you are selecting the area you want to see. While in the second case, the control is dynamic, you are adapting the view to what you want to see.

Just like the gap example in funnel chart changes the gap instantly, so should the zoom slider change the zoom.

Copy link
Member

Choose a reason for hiding this comment

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

I say that from a UX point of view. Due to performance reasons we can offer both if the case arises eventually. 😄

};

return <ZoomSliderTrack ref={ref} onPointerDown={onPointerDown} {...other} />;
}

const formatter = Intl.NumberFormat(undefined, { maximumFractionDigits: 0 });
const zoomValueFormatter = (value: number) => formatter.format(value);

Expand Down Expand Up @@ -185,25 +325,17 @@ function ChartAxisZoomSliderActiveTrack({
let prevPointerZoom = 0;

const onPointerMove = rafThrottle((event: PointerEvent) => {
const { left, top, height, width } = selectorChartDrawingArea(store.getSnapshot());
const axisZoomData = selectorChartAxisZoomData(store.getSnapshot(), axisId);
const element = svgRef.current;

if (!axisZoomData || !element) {
if (!element) {
return;
}

const point = getSVGPoint(element, event);
let pointerZoom = calculateZoomFromPoint(store.getSnapshot(), axisId, point);

let pointerZoom: number;
if (axisDirection === 'x') {
pointerZoom = ((point.x - left) / width) * 100;
} else {
pointerZoom = ((top + height - point.y) / height) * 100;
}

if (reverse) {
pointerZoom = 100 - pointerZoom;
if (pointerZoom === null) {
return;
}

pointerZoom = Math.max(pointerZoomMin, Math.min(pointerZoomMax, pointerZoom));
Expand All @@ -216,7 +348,7 @@ function ChartAxisZoomSliderActiveTrack({

const onPointerUp = () => {
activePreviewRect.removeEventListener('pointermove', onPointerMove);
activePreviewRect.removeEventListener('pointerup', onPointerUp);
document.removeEventListener('pointerup', onPointerUp);
setShowTooltip(null);
};

Expand All @@ -225,7 +357,6 @@ function ChartAxisZoomSliderActiveTrack({
event.preventDefault();
activePreviewRect.setPointerCapture(event.pointerId);

const { left, top, height, width } = selectorChartDrawingArea(store.getSnapshot());
const axisZoomData = selectorChartAxisZoomData(store.getSnapshot(), axisId);
const element = svgRef.current;

Expand All @@ -234,25 +365,18 @@ function ChartAxisZoomSliderActiveTrack({
}

const point = getSVGPoint(element, event);
const pointerDownZoom = calculateZoomFromPoint(store.getSnapshot(), axisId, point);

// The corresponding value of zoom where the pointer was pressed
let pointerDownZoom: number;
if (axisDirection === 'x') {
pointerDownZoom = ((point.x - left) / width) * 100;
} else {
pointerDownZoom = ((top + height - point.y) / height) * 100;
}

if (reverse) {
pointerDownZoom = 100 - pointerDownZoom;
if (pointerDownZoom === null) {
return;
}

prevPointerZoom = pointerDownZoom;
pointerZoomMin = pointerDownZoom - axisZoomData.start;
pointerZoomMax = 100 - (axisZoomData.end - pointerDownZoom);

setShowTooltip('both');
activePreviewRect.addEventListener('pointerup', onPointerUp);
document.addEventListener('pointerup', onPointerUp);
activePreviewRect.addEventListener('pointermove', onPointerMove);
};

Expand All @@ -275,22 +399,14 @@ function ChartAxisZoomSliderActiveTrack({
const point = getSVGPoint(element, event);

instance.setZoomData((prevZoomData) => {
const { left, top, width, height } = selectorChartDrawingArea(store.value);

const zoomOptions = selectorChartAxisZoomOptionsLookup(store.value, axisId);
const zoomOptions = selectorChartAxisZoomOptionsLookup(store.getSnapshot(), axisId);

return prevZoomData.map((zoom) => {
if (zoom.axisId === axisId) {
let newStart: number;

if (axisDirection === 'x') {
newStart = ((point.x - left) / width) * 100;
} else {
newStart = ((top + height - point.y) / height) * 100;
}
const newStart = calculateZoomFromPoint(store.getSnapshot(), axisId, point);

if (reverse) {
newStart = 100 - newStart;
if (newStart === null) {
return zoom;
}

return {
Expand All @@ -314,9 +430,8 @@ function ChartAxisZoomSliderActiveTrack({
const point = getSVGPoint(element, event);

instance.setZoomData((prevZoomData) => {
const { left, top, width, height } = selectorChartDrawingArea(store.value);

const zoomOptions = selectorChartAxisZoomOptionsLookup(store.value, axisId);
const { left, top, width, height } = selectorChartDrawingArea(store.getSnapshot());
const zoomOptions = selectorChartAxisZoomOptionsLookup(store.getSnapshot(), axisId);

return prevZoomData.map((zoom) => {
if (zoom.axisId === axisId) {
Expand Down Expand Up @@ -449,6 +564,30 @@ function ChartAxisZoomSliderActiveTrack({
);
}

export function calculateZoomFromPoint(state: ChartState<any>, axisId: AxisId, point: DOMPoint) {
const { left, top, height, width } = selectorChartDrawingArea(state);
const axis = selectorChartRawAxis(state, axisId);

if (!axis) {
return null;
}

const axisDirection = axis.position === 'right' || axis.position === 'left' ? 'y' : 'x';

let pointerZoom: number;
if (axisDirection === 'x') {
pointerZoom = ((point.x - left) / width) * 100;
} else {
pointerZoom = ((top + height - point.y) / height) * 100;
}

if (axis.reverse) {
pointerZoom = 100 - pointerZoom;
}

return pointerZoom;
}

export function calculateZoomStart(
newStart: number,
currentZoom: ZoomData,
Expand Down
1 change: 1 addition & 0 deletions packages/x-charts-pro/src/FunnelChart/FunnelChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ FunnelChart.propTypes = {
current: PropTypes.shape({
exportAsImage: PropTypes.func.isRequired,
exportAsPrint: PropTypes.func.isRequired,
setAxisZoomData: PropTypes.func.isRequired,
setZoomData: PropTypes.func.isRequired,
}),
}),
Expand Down
1 change: 1 addition & 0 deletions packages/x-charts-pro/src/LineChartPro/LineChartPro.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ LineChartPro.propTypes = {
current: PropTypes.shape({
exportAsImage: PropTypes.func.isRequired,
exportAsPrint: PropTypes.func.isRequired,
setAxisZoomData: PropTypes.func.isRequired,
setZoomData: PropTypes.func.isRequired,
}),
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ ScatterChartPro.propTypes = {
current: PropTypes.shape({
exportAsImage: PropTypes.func.isRequired,
exportAsPrint: PropTypes.func.isRequired,
setAxisZoomData: PropTypes.func.isRequired,
setZoomData: PropTypes.func.isRequired,
}),
}),
Expand Down
Loading