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] Allow zoom on Y axis and add zoom options to configure zooming behaviour #13726

Merged
merged 40 commits into from
Jul 11, 2024

Conversation

JCQuintas
Copy link
Member

@JCQuintas JCQuintas commented Jul 4, 2024

Add zoom options

The main idea is to have ZoomOptions inside the ChartsAxisProps so we can set an option for each axis.

Zoom is then controlled by a different prop when it is implemented in a future PR. The goal is to have it as a root prop called zoom and onZoomChange

export type ZoomOptions = {
  min?: number;
  max?: number;
  step?: number;
  minSpan?: number;
  maxSpan?: number;
  panning?: boolean;
};

export type ZoomData = {
  min: number;
  max: number;
  axisId: AxisId;
};

export type ZoomProps = {
  zoom: ZoomData[];
  onZoomChange: (zoom: ZoomData[]) => void;
};

<Chart
  xAxis={[
    { 
      id: 'my-axis'
      zoom: boolean | ZoomOptions
    }
  ]}
  zoom: [{ axisId: 'my-axis', start: 10, end: 90 }]
  onZoomChange: (data: ZoomData[]): void
/>
  • min/max controls the possible range, it works like the zoom when set at first render.
  • step changes the sensitivity of the zoom
  • minSpan/maxSpan determines the min/max span size. span = (currentMaxZoom-currentMinZoon)
  • panning enable/disable

Allow Y Axis zooming

Simply enable it on a Y axis.

  • Scatter: works as expected
  • Line/Bar: works but is a bit odd, will require "filterMode" to work nicely

Controllable

Not controllable yet. Will be in a future PR.

@JCQuintas JCQuintas added new feature New feature or request component: charts This is the name of the generic UI component, not the React module! labels Jul 4, 2024
@JCQuintas JCQuintas self-assigned this Jul 4, 2024
@mui-bot
Copy link

mui-bot commented Jul 4, 2024

@alexfauquette
Copy link
Member

The xAxis={{ zoom: ZoomOptions }} seems effectively easier from a user point of view.

With the props zoomOptions={{ }} I don't see how you could easily target some specific axes. You will need to rely on the axes ids, and then deal with errors such that the axes id is not the correct one.

Side note: At first I was concerned by the approach because I thought "If we do a mapping 1 axis to 2 zoom config, how can we handle zoom interaction across axes such that zooming in the x axis impacts the y axis" But I realized the notion of filterMode in Echarts would be ok with this DX.

https://echarts.apache.org/en/option.html#dataZoom-inside.filterMode

About the We then have zoom: ZoomState do you know how this state would look like?

I thought about something like this, but did not thaught deep about it.

{
	xZoom: {
		id1: [min, max],
	},
	yZoom: {
		id1: [min, max],
	}
}

My main ideas were:

  • It should be designed such that onZoomChange={setZoomState} could work
  • All axes should be available such that if the user want to have custom interaction between x/y axes it's doable in the onZoomChange

@JCQuintas
Copy link
Member Author

@alexfauquette my current line of thinking is to have the options be a configuration, while the zoom state only contains an array {min|max|id}[]. So that the main pieces that change are very small. I like the idea of having it as {id:{min|max}} too though

export type ZoomOptions = {
  min?: number;
  max?: number;
  step?: number;
  minSpan?: number;
  maxSpan?: number;
  panning?: boolean;
};

export type ZoomData = {
  min: number;
  max: number;
  axisId: AxisId;
};

export type ZoomProps = {
  zoom: ZoomData[];
  onZoomChange: (zoom: ZoomData[]) => void;
};

@JCQuintas
Copy link
Member Author

Regarding the filterMode yeah we will need it. I was going to add it to the readme eventually. Yesterday I was messing with the different configurations, and right now it breaks when we have different configurations for each axis 😓

@JCQuintas
Copy link
Member Author

Btw the code is currently working if you want to pull it and test it out.

Playground Code
import * as React from 'react';
import { Box } from '@mui/material';
import { DEFAULT_X_AXIS_KEY } from '@mui/x-charts/constants';
import { ScatterChartPro } from '@mui/x-charts-pro/ScatterChartPro';
import { LineChartPro } from '@mui/x-charts-pro/LineChartPro';
import { BarChartPro } from '@mui/x-charts-pro/BarChartPro';
import { Chance } from 'chance';
import Layout from './components/Layout';
import '@mui/x-charts-pro/typeOverloads';

const chance = new Chance(42);

const v = 25;
const min = -v;
const max = v;
const data = [
  ...Array.from({ length: 200 }, () => ({
    x: chance.floating({ min: min, max: max }),
    y: chance.floating({ min: min, max: max }),
  })).map((d, index) => ({ ...d, id: index })),
  { x: 0, y: 0, id: 200 },
  { x: min, y: min, id: 201 },
  { x: max, y: max, id: 202 },
  { x: max, y: min, id: 203 },
  { x: min, y: max, id: 204 },
  { x: 0, y: max, id: 205 },
  { x: 0, y: min, id: 206 },
  { x: min, y: 0, id: 207 },
  { x: max, y: 0, id: 208 },
];

const chartSetting = {
  height: 300,
  tooltip: { trigger: 'none' as const },
  slotProps: { legend: {} },
};

const dataset = [
  [59, 57, 86, 21, 'Jan'],
  [50, 52, 78, 28, 'Fev'],
  [47, 53, 106, 41, 'Mar'],
  [54, 56, 92, 73, 'Apr'],
  // [57, 69, 92, 99, 'May'],
  // [60, 63, 103, 144, 'June'],
  // [59, 60, 105, 319, 'July'],
  // [65, 60, 106, 249, 'Aug'],
  // [51, 51, 95, 131, 'Sept'],
  // [60, 65, 97, 55, 'Oct'],
  // [67, 64, 76, 48, 'Nov'],
  // [61, 70, 103, 25, 'Dec'],
].map(([london, paris, newYork, seoul, month]) => ({ london, paris, newYork, seoul, month }));

function BarsDataset() {
  return (
    <Box>
      <BarChartPro
        dataset={dataset}
        xAxis={[{ scaleType: 'band', dataKey: 'month', position: 'top', zoom: true }]}
        yAxis={[{ label: 'rainfall (mm)', zoom: true }]}
        series={[
          {
            dataKey: 'london',
            label: 'London',
          },
          {
            dataKey: 'paris',
            label: 'Paris',
          },
          {
            dataKey: 'newYork',
            label: 'New York',
          },
          {
            dataKey: 'seoul',
            label: 'Seoul',
          },
        ]}
        {...chartSetting}
      />
      <ScatterChartPro
        series={[
          {
            type: 'scatter',
            id: 'linear',
            data,
          },
        ]}
        topAxis={{
          axisId: DEFAULT_X_AXIS_KEY,
          disableLine: false,
          disableTicks: false,
          label: 'My axis',
          tickSize: 6,
        }}
        xAxis={[
          {
            zoom: {
              min: 25,
              max: 75,
              minSpan: 50,
              maxSpan: 50,
            },
          },
        ]}
        yAxis={[
          {
            zoom: {
              min: 25,
              max: 75,
              minSpan: 50,
              maxSpan: 50,
            },
          },
        ]}
        {...chartSetting}
      />
      <LineChartPro
        {...chartSetting}
        series={[
          {
            data: [4000, 3000, 2000, 2780, 1890, 2390, 3490],
            label: 'pv',
          },
          {
            data: [2400, 1398, 9800, 3908, 4800, 3800, 4300],
            label: 'uv',
            area: true,
          },
        ]}
        xAxis={[
          {
            scaleType: 'point',
            data: ['Page A', 'Page B', 'Page C', 'Page D', 'Page E', 'Page F', 'Page G'],
            zoom: true,
          },
        ]}
        yAxis={[
          {
            zoom: true,
          },
        ]}
      />
      <LineChartPro
        xAxis={[{ data: [1, 10, 30, 50, 70, 90, 100] }]}
        yAxis={[
          { id: 'linearAxis', scaleType: 'linear' },
          { id: 'logAxis', scaleType: 'linear' },
        ]}
        series={[
          { yAxisKey: 'linearAxis', data: [1, 10, 30, 50, 70, 90, 100], label: 'linear' },
          { yAxisKey: 'logAxis', data: [1, 10, 30, 50, 70, 90, 100], label: 'log' },
        ]}
        leftAxis="linearAxis"
        rightAxis="logAxis"
        height={400}
      />
    </Box>
  );
}
export default function Page() {
  return (
    <Layout>
      <BarsDataset />
    </Layout>
  );
}

@JCQuintas JCQuintas marked this pull request as ready for review July 9, 2024 19:23
@alexfauquette
Copy link
Member

Small details to fix latter:

The highlight elements can overflow the drawing areas

image
image

Copy link
Member

@alexfauquette alexfauquette left a comment

Choose a reason for hiding this comment

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

Nice PR 🎉

Most of the comments are about small improvement for code readablity and DX, but the over all structure looks very good

docs/data/charts/zoom-and-pan/zoom-and-pan.md Outdated Show resolved Hide resolved
packages/x-charts-pro/src/LineChartPro/LineChartPro.tsx Outdated Show resolved Hide resolved
packages/x-charts-pro/src/typeOverloads/modules.ts Outdated Show resolved Hide resolved
packages/x-charts/src/ChartsYAxis/ChartsYAxis.tsx Outdated Show resolved Hide resolved
packages/x-charts/src/ChartsYAxis/ChartsYAxis.tsx Outdated Show resolved Hide resolved
JCQuintas and others added 12 commits July 10, 2024 16:00
Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 11, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 11, 2024
@JCQuintas JCQuintas enabled auto-merge (squash) July 11, 2024 14:31
@JCQuintas JCQuintas merged commit 863a168 into mui:master Jul 11, 2024
15 checks passed
@JCQuintas JCQuintas deleted the zoom-props branch July 11, 2024 14:48
DungTiger pushed a commit to DungTiger/mui-x that referenced this pull request Jul 23, 2024
…ng behaviour (mui#13726)

Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
thomasmoon pushed a commit to thomasmoon/mui-x that referenced this pull request Sep 9, 2024
…ng behaviour (mui#13726)

Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: charts This is the name of the generic UI component, not the React module! new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants