search

mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.55k stars 1.33k forks source link

[charts] Sparkline margin have unexpected effect #15221

Open bourquep opened 3 weeks ago

bourquep commented 3 weeks ago

Steps to reproduce

https://codesandbox.io/p/sandbox/muix-issue-15221-r5jd9s?file=%2Fsrc%2Frepro.tsx

Current behavior

SparkLine "bleeds" at the bottom if it's bounding box when it's yAxis min value is not set to zero:

image

Expected behavior

Sparklines should occupy the same area regardless of their min value.

Context

No response

Your environment

npx @mui/envinfo ``` System: OS: macOS 15.1 Binaries: Node: 20.9.0 - ~/.nvm/versions/node/v20.9.0/bin/node npm: 10.1.0 - ~/.nvm/versions/node/v20.9.0/bin/npm pnpm: Not Found Browsers: Chrome: 130.0.6723.91 Edge: Not Found Safari: 18.1 npmPackages: @emotion/react: 11.13.3 => 11.13.3 @emotion/styled: 11.13.0 => 11.13.0 @mui/core-downloads-tracker: 6.1.5 @mui/icons-material: 6.1.6 => 6.1.5 @mui/material: 6.1.5 => 6.1.5 @mui/material-nextjs: 6.1.5 => 6.1.5 @mui/system: 6.1.5 @mui/types: 7.2.18 @mui/x-charts: 7.22.0 => 7.22.0 @mui/x-charts-vendor: 7.20.0 @types/react: ^18 => 18.3.12 react: 19.0.0-rc.0 => 19.0.0-rc-69d4b800-20241021 react-dom: 19.0.0-rc.0 => 19.0.0-rc-69d4b800-20241021 typescript: ^5 => 5.6.3 ```

Search keywords: sparkline min

michelengelen commented 2 weeks ago

Hey @bourquep ... without a reproduction checking this will take a lot longer. Could you help us out and provide an example using the codesandbox templates from the docs: https://mui.com/x/react-charts/sparkline/#basics ?

Thanks!

bourquep commented 2 weeks ago

@michelengelen I keep getting this error in the code sandbox after editing one of the samples from the doc:

Could not find module in path: '@mui/x-charts-vendor/lib-vendor/d3-array/src/count.js' relative to '/node_modules/@mui/x-charts-vendor/lib-vendor/d3-array/src/index.js'

But I'll see what I can do!

bourquep commented 2 weeks ago

I have updated the issue description with a code sandbox link: https://codesandbox.io/p/sandbox/muix-issue-15221-r5jd9s?file=%2Fsrc%2Frepro.tsx

michelengelen commented 2 weeks ago

Thanks @bourquep ... @alexfauquette already moved it to the board! 👍🏼

alexfauquette commented 2 weeks ago

The space you see comes from the margin which add 5px margin on all sides. You can remove it with margin={{ bottom: 0, left: 5, right: 5, top: 5 }}.

Does that solves your issue?

Additional notes

By setting min: 1 you translate vertically your area, and it fill the space because the sparkling allows plot to overflow the drawing area.

image

To fix

The default margin of the sparkling get removed if we starter specifying the props. We should allow user to provide margin={{ bottom: 0 }} and only modify the bottom margin

We could add a clip-path to the sparkling. Time would be better invested by re-writing the component on a canvas

bourquep commented 2 weeks ago

@alexfauquette Actually my intended design would be to always have a space below the chart, but I can't do that when I set a min value, not even when playing with the margin.

But for now, I could work with the bottom: 0 workaround so that all my spark lines would extend to the bottom border. They at least would all look the same.

Thanks!

alexfauquette commented 2 weeks ago

Effectively without the clip-path this notion of drawing area lose a lot of sens, we might finally need to add it 🤔