Skip to content

[charts-pro] Add size for zoom slider #17736

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
5 changes: 5 additions & 0 deletions docs/data/charts/zoom-and-pan/zoom-and-pan.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ You can provide an overview and allow the manipulation of the zoomed area by set

{{"demo": "ZoomSlider.js"}}

You can set the `zoom.slider.size` property to customize the size reserved for the zoom slider's.
Since the zoom slider has a fixed size of 20 px, updating `zoom.slider.size` effectively changes the margin around the slider.

The size is the height on an x-axis and the width on a y-axis.
Comment on lines +65 to +68
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure if it's worth documenting this. I still feel a bit uneasy that we're calling it size, but it only affects the margin.

Should we maybe call it margin instead?

Another drawback of size is that if a user sets a size small than 20, we'll see the slider bleeding onto other elements.

Copy link
Member

Choose a reason for hiding this comment

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

I got confused by this, but now I noticed the slider is an svg element.

I was assuming it to be HTML 🤯

What was the rationale for SVG vs HTML? 🤔
I assumed this interactive part would be easier to handle by having a HTML, since when the preview is added, the scales/drawingarea will have to be recalculated anyways.

This would have the benefit of not having to calculate sizes, but I haven't thought about it too deeply.

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 made it SVG because we can have stacked axis on one side and the slider should be next to the axis it represents, so to use HTML meant we'd need to use a foreignObject, which I wanted to avoid.

Even if we use HTML, we'd need to calculate size because if we have an axis below a zoom slider, we'd need to be observing the sliders size to update the axes' offsets so they wouldn't overlap.

Are you seeing any approach with HTML that makes it simpler to handle this case?

Copy link
Member

Choose a reason for hiding this comment

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

I made it SVG because we can have stacked axis on one side and the slider should be next to the axis it represents, so to use HTML meant we'd need to use a foreignObject, which I wanted to avoid.

I hadn't thought of it, though I suspect this to be such and edge-case that it might not be worth optimising for it.

Even if we use HTML, we'd need to calculate size because if we have an axis below a zoom slider, we'd need to be observing the sliders size to update the axes' offsets so they wouldn't overlap.

WDYM?

If the HTML is in the HTML part of the charts it would behave just like the legend does right now, which is automatic layout.

Copy link
Member Author

Choose a reason for hiding this comment

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

If the HTML is in the HTML part of the charts it would behave just like the legend does right now, which is automatic layout.

If we want to have this edge case working properly:

image

Then we need to offset the second axis by the size of the slider so that they don't overlap.

How could we fix this with HTML?

Copy link
Member

@JCQuintas JCQuintas May 8, 2025

Choose a reason for hiding this comment

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

I just wouldn't solve it. Like, this is already an edge case, it is visually complex for the users to understand too.

It is probably easier both on our and the user side to couple visually similar elements together, and let the users figure out which controls which by using it. We can help by keeping them in order though 😆

Frame 5

If a dev really wants to have them in different order they can play with the axis size and absolute positions. But again, that would be a super edge-case

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 is already an edge case

Yeah, I agree with this.

If a dev really wants to have them in different order they can play with the axis size and absolute positions.

Yeah, I can agree with this.

Is it worth converting everything to HTML now, though? I can do it if needed.

@alexfauquette what's your take on this?


### Composition

When using composition, you can render the axes' sliders by rendering the `ChartZoomSlider` component.
Expand Down
4 changes: 2 additions & 2 deletions docs/pages/x/api/charts/bar-chart-pro.json

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions docs/pages/x/api/charts/line-chart-pro.json

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions docs/pages/x/api/charts/scatter-chart-pro.json

Large diffs are not rendered by default.

16 changes: 16 additions & 0 deletions packages/x-charts-pro/src/BarChartPro/BarChartPro.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ BarChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -465,6 +466,7 @@ BarChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -545,6 +547,7 @@ BarChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -625,6 +628,7 @@ BarChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -705,6 +709,7 @@ BarChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -785,6 +790,7 @@ BarChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -865,6 +871,7 @@ BarChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -945,6 +952,7 @@ BarChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1044,6 +1052,7 @@ BarChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1132,6 +1141,7 @@ BarChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1211,6 +1221,7 @@ BarChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1290,6 +1301,7 @@ BarChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1369,6 +1381,7 @@ BarChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1448,6 +1461,7 @@ BarChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1527,6 +1541,7 @@ BarChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1606,6 +1621,7 @@ BarChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down
16 changes: 16 additions & 0 deletions packages/x-charts-pro/src/LineChartPro/LineChartPro.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ LineChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -470,6 +471,7 @@ LineChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -550,6 +552,7 @@ LineChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -630,6 +633,7 @@ LineChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -710,6 +714,7 @@ LineChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -790,6 +795,7 @@ LineChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -870,6 +876,7 @@ LineChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -950,6 +957,7 @@ LineChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1049,6 +1057,7 @@ LineChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1137,6 +1146,7 @@ LineChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1216,6 +1226,7 @@ LineChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1295,6 +1306,7 @@ LineChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1374,6 +1386,7 @@ LineChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1453,6 +1466,7 @@ LineChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1532,6 +1546,7 @@ LineChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1611,6 +1626,7 @@ LineChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down
16 changes: 16 additions & 0 deletions packages/x-charts-pro/src/ScatterChartPro/ScatterChartPro.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ ScatterChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -455,6 +456,7 @@ ScatterChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -535,6 +537,7 @@ ScatterChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -615,6 +618,7 @@ ScatterChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -695,6 +699,7 @@ ScatterChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -775,6 +780,7 @@ ScatterChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -855,6 +861,7 @@ ScatterChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -935,6 +942,7 @@ ScatterChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1034,6 +1042,7 @@ ScatterChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1122,6 +1131,7 @@ ScatterChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1201,6 +1211,7 @@ ScatterChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1280,6 +1291,7 @@ ScatterChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1359,6 +1371,7 @@ ScatterChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1438,6 +1451,7 @@ ScatterChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1517,6 +1531,7 @@ ScatterChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down Expand Up @@ -1596,6 +1611,7 @@ ScatterChartPro.propTypes = {
panning: PropTypes.bool,
slider: PropTypes.shape({
enabled: PropTypes.bool,
size: PropTypes.number,
}),
step: PropTypes.number,
}),
Expand Down
Loading