Skip to content

Conversation

bernardobelchior
Copy link
Member

@bernardobelchior bernardobelchior commented Aug 13, 2025

Related to #19064.

Add example with a bar gradient that reflects the bar's value:

image

@mui-bot
Copy link

mui-bot commented Aug 13, 2025

Deploy preview: https://deploy-preview-19174--material-ui-x.netlify.app/

Updated pages:

Bundle size report

Bundle Parsed size Gzip size
@mui/x-data-grid 0B(0.00%) 0B(0.00%)
@mui/x-data-grid-pro 0B(0.00%) ▼-1B(0.00%)
@mui/x-data-grid-premium 0B(0.00%) ▼-1B(0.00%)
@mui/x-charts 0B(0.00%) 0B(0.00%)
@mui/x-charts-pro 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers 0B(0.00%) 0B(0.00%)
@mui/x-date-pickers-pro 0B(0.00%) ▼-1B(0.00%)
@mui/x-tree-view 0B(0.00%) 0B(0.00%)
@mui/x-tree-view-pro 0B(0.00%) ▼-2B(-0.01%)

Details of bundle changes

Generated by 🚫 dangerJS against 067a6e0

By default, a gradient's units are set to `objectBoundingBox`.
When applied to a bar, the gradient will stretch to fill the entire size of the bar, regardless of the bar's value.

Alternatively, you can set `gradientUnits` to `userSpaceOnUse`, which stretches the gradient to fill the entire size of the chart.
Copy link
Member

Choose a reason for hiding this comment

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

Would probably be nice to have a few words on how to achieve that and how to account for the legend/tooltip

Copy link
Member Author

@bernardobelchior bernardobelchior Aug 13, 2025

Choose a reason for hiding this comment

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

how to achieve that

Sure, I'll expand on this.

how to account for the legend/tooltip

No need to account for those since they are outside the SVG. We only need to account for the axes sizes + margins. I'll explain that

Copy link
Member Author

@bernardobelchior bernardobelchior Aug 14, 2025

Choose a reason for hiding this comment

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

Done, let me know if it's clearer or if I should rewrite it. I found it a bit hard to explain succinctly

@alexfauquette
Copy link
Member

The Demo looks good except for the tooltip which looks like a flag (red yellow green is the Guinea)

image

Would it be possible to mix that with a continuouse colormap.

image

While thinking about that edge case, I'm wondering if it should not just be an additional case we support. Because for a continuous color scale we already have a gradient computed (usefull for line and scatter charts https://deploy-preview-19174--material-ui-x.netlify.app/x/react-charts/scatter/#color-scale)
An additional property to allow bar chart to fill its rectangle with the gradient instead of the computed value could do the job in a less hacky way

@bernardobelchior
Copy link
Member Author

The Demo looks good except for the tooltip which looks like a flag (red yellow green is the Guinea)

image Would it be possible to mix that with a continuouse colormap.

You mean so that the tooltip color is the same as the color in the gradient that corresponds to the bar's value?

image While thinking about that edge case, I'm wondering if it should not just be an additional case we support. Because for a continuous color scale we already have a gradient computed (usefull for line and scatter charts [deploy-preview-19174--material-ui-x.netlify.app/x/react-charts/scatter#color-scale](https://deploy-preview-19174--material-ui-x.netlify.app/x/react-charts/scatter/#color-scale)) An additional property to allow bar chart to fill its rectangle with the gradient instead of the computed value could do the job in a less hacky way

So basically allowing a gradient in a continuous color map instead?
Yeah, I think that could work. I'll investigate how to do it

@alexfauquette
Copy link
Member

You mean so that the tooltip color is the same as the color in the gradient that corresponds to the bar's value?

Yes, that would be more interesting to see in the tooltip that values are green or red instead of the gradient

So basically allowing a gradient in a continuous color map instead?
Yeah, I think that could work. I'll investigate how to do it

I investigated a bit. The issue is that we have two places for color management

  • The fixed colors are obtained from getColor() wich is defined in each seriesConfig
  • The gradients are rendered in a single place and we have a hook to get their ids

Charts with a mix of fix colors make their choice by picking the gradient if it exist and the getColor() result otherwise.
Here we would need to define a DX for the bar chart that let it swich to gradient when needed

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Aug 14, 2025
Copy link

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

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Aug 14, 2025
@bernardobelchior
Copy link
Member Author

You mean so that the tooltip color is the same as the color in the gradient that corresponds to the bar's value?

Yes, that would be more interesting to see in the tooltip that values are green or red instead of the gradient

Yeah, makes sense. I'm not sure if it's doable in this demo, though. That would probably require our library to support it first-class.

I investigated a bit. The issue is that we have two places for color management

  • The fixed colors are obtained from getColor() wich is defined in each seriesConfig
  • The gradients are rendered in a single place and we have a hook to get their ids

Charts with a mix of fix colors make their choice by picking the gradient if it exist and the getColor() result otherwise. Here we would need to define a DX for the bar chart that let it swich to gradient when needed

Yeah, we'll need to make some changes. Do you think it still makes sense to merge this PR or should we skip it and develop gradient support?

@oliviertassinari oliviertassinari added the scope: charts Changes related to the charts. label Aug 16, 2025
Copy link

codspeed-hq bot commented Aug 16, 2025

CodSpeed Performance Report

Merging #19174 will improve performances by 8.16%

Comparing bernardobelchior:bar-gradient (067a6e0) with master (0e3b886)

Summary

⚡ 1 improvements
✅ 9 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
ScatterChartPro with big data amount 435 ms 402.1 ms +8.16%

@oliviertassinari oliviertassinari added the type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. label Aug 16, 2025
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.

Let's go with that one


### Gradients

By default, a gradient's units are set to `objectBoundingBox`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
By default, a gradient's units are set to `objectBoundingBox`.
By default, a gradient's units are set to [`objectBoundingBox`](https://developer.mozilla.org/en-US/docs/Web/SVG/Reference/Attribute/gradientUnits#objectboundingbox).


{{"demo": "BarOECDHouseholdSavings.js"}}

Note that, in the example above, we're using two separate gradients:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Note that, in the example above, we're using two separate gradients:
Note that, in the example above, there are two distinct gradients:


Note that, in the example above, we're using two separate gradients:

- the series `color` property references the gradient with `gradientUnits="objectBoundingBox"`, so this will be applied to the tooltip, legend, and other elements that reference the series color.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- the series `color` property references the gradient with `gradientUnits="objectBoundingBox"`, so this will be applied to the tooltip, legend, and other elements that reference the series color.
- The series `color` property references the gradient with `gradientUnits="objectBoundingBox"`. This one is applied to the tooltip, legend, and other elements that reference the series color.

Note that, in the example above, we're using two separate gradients:

- the series `color` property references the gradient with `gradientUnits="objectBoundingBox"`, so this will be applied to the tooltip, legend, and other elements that reference the series color.
- the bar's `fill` property is overridden using CSS to reference the gradient with `gradientUnits="userSpaceOnUse"`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- the bar's `fill` property is overridden using CSS to reference the gradient with `gradientUnits="userSpaceOnUse"`.
- The bar's `fill` property is overridden using CSS to reference the gradient with `gradientUnits="userSpaceOnUse"`.

- the series `color` property references the gradient with `gradientUnits="objectBoundingBox"`, so this will be applied to the tooltip, legend, and other elements that reference the series color.
- the bar's `fill` property is overridden using CSS to reference the gradient with `gradientUnits="userSpaceOnUse"`.

We do this because we want all elements to show the whole gradient, except the bars themselves, which should only show the part of the gradient that corresponds to their value.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
We do this because we want all elements to show the whole gradient, except the bars themselves, which should only show the part of the gradient that corresponds to their value.
The first gradient is use for elements showing the whole gradient.
And the second one is for the bars themselves that show the part of the gradient that corresponds to their value.

@bernardobelchior bernardobelchior merged commit 3b644ce into mui:master Sep 11, 2025
22 checks passed
@bernardobelchior bernardobelchior deleted the bar-gradient branch September 11, 2025 09:15
JCQuintas added a commit to JCQuintas/mui-x that referenced this pull request Sep 15, 2025
Signed-off-by: Bernardo Belchior <[email protected]>
Co-authored-by: Jose C Quintas Jr <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: charts Changes related to the charts. type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants