[charts-pro] Add funnelDirection to control pyramid direction
#18568
Conversation
|
Deploy preview: https://deploy-preview-18568--material-ui-x.netlify.app/ Bundle size reportTotal Size Change: ▼-640KB(-4.81%) - Total Gzip Change: ▼-136KB(-3.36%) Show details for 100 more bundles (22 more not shown)@mui/x-charts-pro/RadarChartPro parsed: ▼-32.2KB(-15.81%) gzip: ▼-6.76KB(-10.76%) |
CodSpeed Performance ReportMerging #18568 will not alter performanceComparing Summary
|
|
The reason we need this is because we're trying to detect the data direction, right? If we don't detect it, then it would just be a matter of calling |
This is to force a direction. Right now if the user uses |
|
|
No opinion on this one
What about
We should probably have a link from the funnel to the pyramid page to indicate further customization are available |
Yeah, I think I might not have explained myself correctly. My point was: if we don't automatically detect direction, wouldn't a user be able to control the direction by the array order? I.e., What I'm trying to understand is if automatically detecting the direction causes unexpected behavior and maybe we should consider reverting it. |
The issue is when the user has |
Pretty much this is used to detect which direction the pointy shape should be |
Forcing the rendering to be independent from the data order would open the option to sorting data. For example you get unsorted data from your API. You just have to put data in the Funnel, and they appear sorted according to the pyramid shape you chose. Like what Echarts is doing
https://echarts.apache.org/en/option.html#series-funnel.sort |
Yeah, and is it that beneficial? The user can easily The only benefit I'd see is if we would be providing a more performant API, but I'm not sure that's the case.
Yeah, makes sense. I just wonder if our effort would be better spent elsewhere since |
|
I don't think we should sort it at all. Pyramid is generally viewed as hierarchical data, it does not necessarily have to be sorted in the pyramid shape. A chart like this wouldn't really make sense: 😆 |
|
We could however offer data transforms, though that is another topic 🙂 |
Isn't that similar to the chart in #18559? |
Yes, it is similar, though your example is in the correct direction |
What do you mean by correct direction? I think I misunderstood why the chart you posted doesn't make sense 😅 |
The one I posted the data is correct, the pyramid shape isn't. When you represent organisational hierarchy execs are at the top, so technically they should be the pointy bit 😆 |
|
Ah, I see what you mean. Yeah, agreed. Still, I'm not entirely sure that internalizing the sorting/data detection behavior is great. It's more code we need to maintain, we'll always be behind what users want (today it's sorting, tomorrow someone will need filtering), and I'm failing to see the benefit. I think it could be beneficial if we were using this to improve performance (which we might need to do in the future), but otherwise I'm not seeing how this can be helpful. |
It would be mostly performance when using a dataset I guess. But it is not high in our prio list indeed |
|
I've renamed |
dataDirection to control pyramid directionfunnelDirection to control pyramid direction
|
Just so it doesn't get lost: I'm still wondering if we should consider removing the direction detection behavior. More info here: #18568 (comment) |
I don't think the comment you linked provides valid points.
No, because we need to automatically detect the direction for the visualisation to change. Simply changing the order of data wouldn't do anything in a pyramid chart, where we don't calculate the sizes, we simply define the middle and boundaries of the pyramid, and plot the lines according to that. The pyramid shape can't be affected by the series data, else it wouldn't be a pyramid.
For the pyramid it wouldn't change the direction at all without it. |
|
@bernardobelchior With {[10,20,30]} you get △, and with {[30,20,10]} you get ▽. Now if I want to represent my data so that the lowest band i.e, base of the pyramid shows lowest income level and the income level rises with pyramid, then I should be able to do that with a prop, irrespective of how my data is ordered. |
Hm, I see! Then my proposal would be as follows: The algorithm maps that the first value in the data to the base of the pyramid (i.e., it's widest part). The pyramid is then built from its base to its top. We then have layout For the horizontal layout, the default is to render from left to right; bottom to top for the vertical. Additionally, there's a I think this might be clearer for the user, as I think it's counter-intuitive that the rendering direction changes with the data: Screen.Recording.2025-07-03.at.12.46.32.mov |
8f3e42c to
5f0bf05
Compare
Tbh I feel this just makes something that is easy harder and the only reason is apparent consistency. When using a linear curve we also automatically detect the direction, it just behaves differently. Screen.Recording.2025-07-03.at.14.58.08.movWhen we want to use a funnel, the data will often be either increasing or decreasing, and we would be right most of the time out-of-the-box. If the user expects a specific direction, we are providing a way to override that in this PR. I've pushed changes to make it similar to your suggestion, you can play with it in the |
| * Toggles the direction the funnel is drawn. | ||
| * | ||
| * - `decreasing`, funnel is drawn with a wide top and a point at the base. | ||
| * - `increasing`, funnel is drawn with a point at the top and a wide base. |
There was a problem hiding this comment.
I think decreasing/increasing isn't very clear. I can't think of a clear name, so maybe a invert: boolean would make more sense.
Nevertheless, I think this should be more explicit. Something like this:
| * Toggles the direction the funnel is drawn. | |
| * | |
| * - `decreasing`, funnel is drawn with a wide top and a point at the base. | |
| * - `increasing`, funnel is drawn with a point at the top and a wide base. | |
| * Defines the direction in which the funnel is drawn. | |
| * | |
| * Depending on the funnel's layout, vertical and horizontal respectively, the widest section is drawn: | |
| * - `decreasing`: top/left first, and the funnel grows towards bottom/right; | |
| * - `increasing`: bottom/right first, and the funnel grows towards top/left. |
There was a problem hiding this comment.
The suggestion adds too much context to the documentation, makes it harder to read.
Using a boolean is a bit odd, cause we are just inverting it in the case of the pyramid, but if we remove the auto mode, then this should work for all curves, which aren't really inverted.
There was a problem hiding this comment.
The suggestion adds too much context to the documentation, makes it harder to read.
We can reword it if you think it's too long.
IMO we should be explicit on how this prop affects the chart, otherwise people will just use trial and error to get what they want, which might be frustrating.
I'm ok with keeping increasing/decreasing, but I really think we should explain how this prop will affect the chart
There was a problem hiding this comment.
My point is more that I don't think this change is good. Regardless of the wording. 😆
There was a problem hiding this comment.
Could you explain why? I'm still trying to understand how the funnel works.
For example, why does setting "direction: 'increasing'" just makes the funnel sections wider?
Screen.Recording.2025-07-03.at.17.55.37.mov
There was a problem hiding this comment.
The basis of the funnel is that each starts at it's own value, and then tappers to the value of the next section.
When the funnel is decreasing, we make the bottom section "static" = Straight sides, every section before that is tapered.
When it is increasing, the top section is "static" instead. To maintain the same shape overall and keep it consistent.
# linear decreasing
\ /
\ /
| |
# linear increasing
| |
/ \
/ \
For the linear-sharp version we just force the points to touch on the end
# linear-sharp
\ /
\ /
\ /
If we don't automatically detect the data-direction. It looks odd for some funnels, as it starts on the value of the previous section. Which for some reason look odd, although it is also technically correct.
# linear decreasing
| |
\ /
\ /
If you reverse the data, then it looks as before.
There was a problem hiding this comment.
Ah, it all makes sense now, thanks for the explanation!
So, basically, 'increasing' makes the widest section a rectangle, while 'decreasing' makes the narrowest section a rectangle, right? This for the funnel.
For the pyramid chart, 'increasing' sets the rendering direction from bottom to top, while 'decreasing' sets it to top to bottom?
Funnel is still in preview, so I'm ok with moving on with this to unblock Prakhar. IMO, we should also do a follow-up PR with documentation since I couldn't find any documentation on how the direction prop works.
Ultimately, I think we should re-evaluate this API because I feel it isn't very clear, but I'm also failing to come up with a better suggestion. Maybe in the docs PR it will become clearer how to proceed?
There was a problem hiding this comment.
Ultimately, I think we should re-evaluate this API because I feel it isn't very clear, but I'm also failing to come up with a better suggestion
The issue is that it creates charts that make no sense. A funnel with increasing value but decreasing shapes is ... uncommon
we should also do a follow-up PR with documentation since I couldn't find any documentation on how the direction prop works.
We could have a section to explain what JC just did. But for most of the users that don't want to read, they should just
- Don't use this prop. the "auto" covers 99% of the usecases
- Know that it correspond to the order they display their value for the 1% remaining
This reverts commit 5f0bf05.
Fixes #18559
funnelDirectionto control pyramid directionThoughts
increasing/decreasingorascending/descendingdirectiontoo instead ofdataDirection