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] Support single color area chart #13907

Closed
milotoor opened this issue Jul 19, 2024 · 9 comments
Closed

[charts] Support single color area chart #13907

milotoor opened this issue Jul 19, 2024 · 9 comments
Assignees
Labels
component: charts This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/

Comments

@milotoor
Copy link

milotoor commented Jul 19, 2024

Summary

A line chart with a filled area currently uses a lightened version of the provided color. The given color is used as the stroke value for the line itself, while the area underneath it uses a color derived with:

fill:
  (ownerState.gradientId && `url(#${ownerState.gradientId})`) ||
  (ownerState.isHighlighted && d3Color(ownerState.color)!.brighter(1).formatHex()) ||
  d3Color(ownerState.color)!.brighter(0.5).formatHex()

This makes the area chart a two-tone chart. This may be aesthetically pleasing but is not always desirable. I would like to be able to build a chart in which the stroke and fill colors are the same, effectively removing the line that divides the area elements. I can imagine several ways to achieve this:

  • Provide an additional string field in the LineSeriesType to support specifying the area color. In my case this would be the same as the color field's value. This solution doesn't make a great deal of sense in general, as I am hard pressed to think of a use case where I'd need/want to specify an arbitrary color for the area regardless of the stroke color. In practice it makes sense that the two would be (closely) related.
  • Provide an additional boolean field in the LineSeriesType to support specifying that the fill color should be the same as the stroke color. This would address my use case directly.
  • Provide an additional numeric field in the LineSeriesType to support specifying the fill color's "brightness" factor, i.e. the value passed to the brighter method of the object returned by d3Color(ownerState.color). My use case would be solved by setting this parameter to 0. This seems like the best solution, as it is the most flexible without violating the principle that there should be a relationship between the stroke and fill colors.

Examples

image

Motivation

I am currently developing off of designs that call for a stacked area chart in which the stroke and fill colors are the same (or, alternatively, in which there is no stroke at all), and this is currently impossible (or at least impractical) to achieve. I can hypothetically do so with the gradientId value, but this is cumbersome and feels like a feature the library ought to support.

Search keywords: area chart color fill
Order ID: 87847

@milotoor milotoor added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 19, 2024
@milotoor
Copy link
Author

I would be happy to put together a pull request for any of the possible solutions mentioned above, but would like to first assess if this is something the core team is interested in at all and, if so, align on the path forward.

@milotoor
Copy link
Author

For reference, here is a sample implementation of the third solution outlined above. Tests and documentation updates to follow if maintainers approve.

@michelengelen michelengelen added enhancement This is not a bug, nor a new feature component: charts This is the name of the generic UI component, not the React module! support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/ labels Jul 22, 2024
@michelengelen
Copy link
Member

Hey @milotoor and thanks for raising this.
Supporting the behavior you described makes sense to me and the solutions provided seem very valid. To provide maximal support for this we could go on about it like this:

  1. By default the fill color is the same as the stroke color, but with 0 opacity so that the default behavior stays the same
  2. provide a new prop fillOpacity/areaOpacity to manually increase that if you need it
  3. provide a new prop fillColor/areaColor to override the default color (derived from stroke

WDYT @alexfauquette @JCQuintas ?

@alexfauquette
Copy link
Member

You have a last option, already existing which is to use the slotProps to override the styling

slotProps={{
  area: (ownerState: AreaElementOwnerState) => ({
    style: { fill: ownerState.color }
  })
}}

https://codesandbox.io/s/nervous-glade-9vtq4j?file=/src/Demo.tsx

I'm also wondering, why not just remove the line for such a case:

slots={{line:() =>  null}}

About future direction, I'm considering removing the usage of d3-color in favor of the CSS filter attribute. Such that in the future if a user want to remove the color distinction between line and area, they can overide the CSS property

@michelengelen michelengelen added status: waiting for author Issue with insufficient information and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 22, 2024
@milotoor
Copy link
Author

milotoor commented Jul 23, 2024

Thanks for the ideas!

  1. The slotProps idea yields precisely what I'm looking for, though there's a TypeScript error:

    image
    Type '(ownerState: AreaElementOwnerState) => { style: { fill: string; }; }' is not assignable to type 
    'AnimatedAreaProps'.ts(2322)
    

    I can typecast my way around this so I'm not concerned about it. It seems that the slotProps.area object is expected to be assignable to AnimatedAreaProps, which the ownerState function is not. The useSlotProps function knows how to handle it regardless, so the type mismatch isn't a problem, just a nuisance.

  2. The slots={{line:() => null}} suggestion has some shortcomings for my use case (and in general, IMO). My use case calls for not only the stacked area chart but also a single line (colored red in the graphic in the issue description), so removing all lines from the chart negates that possibility. However, even if that was an acceptable compromise we're still left with the issue of color: the area fill is still a lightened version of the provided color. I could work around that by taking my desired color palette and darkening it such that the AnimatedArea would brighten in back to my color, though it feels kludgy.

I'll plan to use the slotProps suggestion with some typecasts. Is there any plan to adopt the SlotComponentProps from the @mui/utils package? Seems that the x-date-pickers and x-tree-view packages are making use of it. That would help to resolve the typing issue.

@github-actions github-actions bot added status: waiting for maintainer These issues haven't been looked at yet by a maintainer and removed status: waiting for author Issue with insufficient information labels Jul 23, 2024
@alexfauquette
Copy link
Member

Thanks for the detailed feedback. I will give a look to the TS issue.

@alexfauquette
Copy link
Member

I'm closing this issue since the TS asspect has been fixed by #13965. THe propper fix with CSS will be for next major.

Feel free to reopen if something is missing

Copy link

⚠️ This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

@milotoor: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

@milotoor
Copy link
Author

Works great, thanks @alexfauquette!

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! enhancement This is not a bug, nor a new feature support: premium standard Support request from a Premium standard plan user. https://mui.com/legal/technical-support-sla/
Projects
None yet
Development

No branches or pull requests

3 participants