[Lens] Stack as percentage #70703
[Lens] Stack as percentage #70703
Conversation
|
@wylieconlon @mbondyra @cchaos @AlonaNadler Looking for feedback on this one as well. |
|
@elasticmachine merge upstream |
|
The pr failed for me, anything Im missing? |
|
@AlonaNadler updated the PR, should work again. |
…a into lens/stack-as-percentage
|
@flash1293 when it comes to how to offer the functionality, I agree with you, for me the most friendly would be to present it as another chart type (until we have 30 types of charts I think we're safe with going this direction and I don't think we plan many more soon). Also, it's worth to mention, we need icons for main chart switcher and the xy chart one:
For the layer xy chart one, maybe it makes sense to add a tooltip with name because it's not that clear only looking at the icons. |
|
@mbondyra Thanks for the review, I didn't expect this failure so it's good to know :) I will make sure I get this one when cleaning up the PR. I remember some early mock ups with a search bar and filters in the chart switcher - if it gets too busy we can introduce something like this. |
|
Question: What makes the configuration of a Stacked vs Percentage stacked chart so different that it needs to be a separate chart type? |
|
@cchaos It's not about difference, more about discoverability and consistent ui - we are treating stacked charts vs non-stacked counterparts the same way and you could ask the same question there. None of the xy chart variants "need" to be a different chart type, we could also present all of these settings as a set of UI controls like it's done in Visualize: Happy to change it to a switch somewhere though, this is exactly what I want to discuss here :) |
|
I do think there is an added ease-of-use to supplying this chart as a specific chart type, but I also want to ensure that users who may start in a normal bar or stacked bar can easily change it (or know how to change it) to display as percentages. So a couple thoughts on that (and these might already be addressed, just want to reiterate them):
As for the chart type picker and the icons, we are starting to amass quite a bit of different x/y charts in particular. I can create some icons to indicate these new chart types, but I think we need to go further and:
|
|
@cchaos Thanks for the feedback.
I don't see any issues here as soon as Wylies bugfix for multi layer chart switches is solved. Technically this pretty similar to stacking.
You mean the newly introduced "Settings" popover? We can add an option in there as well to convert all of the layers to percentage mode, but take into account we can also configure charts with only partial percentage mode ("Mixed XY chart")
Both of these sound great, could you throw together a quick mock up? Then I can adjust the chart picker on this PR |
No I mean the config popover, the one where you select the aggregation and field. I can get you a mock next week |
Oh, that's a change in how the percentage mode is applied - right now it happens on a per layer basis, but putting it in there would imply it's a per metric setting. |
|
I'm playing with the PR and trying to understand percentages better. From my understanding a percentage chart doesn't make sense to have multiple y-axis unless they are on separate layers. See this example:
So should we restrict percentage charts to a single y-axis per layer? If so, then it seems that this setting IS metric-based and not necessarily layer based. But we'd have to have some crazy logic about allowing the selection the metric config popover only if there's a single y-axis. We could possibly revisit this option later and concentrate it on just being a chart type for now. Is there also a short name for it? It's quite a mouthful |
|
@cchaos I agree in most cases a percentage chart with multiple metrics on y axis doesn't make sense, but it can make sense if the two metrics are in relation to each other, e.g. About the naming - the only alternative I can think of would be "ratio chart", but it might get confusing considering a quick function named "Filter ratio" or something along those lines. Do we even need to put the full name in the chart switcher like this? |
|
Feedback so far. I admit it was hard to test and kept failing on me, changes to intervals, and number of terms in the breakdown by didnt seem to take effect.
From empty state - if users started by selecting one of the stack % charts:
|
|
@AlonaNadler Fixed the bug with the area percentage chart. I couldn't reproduce the problem with the unchangeable terms and date histogram params, could you share steps to reproduce for that? About the transitions: I like those in general, but we don't have a way to represent "timestamp (on entire time range)" - this is both a problem for building the suggestion as well as configuring it in general (as there is no "entire time range" setting, and using the custom interval comes with its own problems). Also, simply from a UX standpoint - will the user understand how to change that X axis to something meaningful? Not saying it's impossible but it would be a lot of effort to make that work smoothly, so I wonder whether there is a better way? As it's the least effort in the current system I would recommend using auto interval for that date histogram for now to keep the effort for this feature low and make a note for this when we get to improving suggestions in general. |
|
Discussed this with @AlonaNadler and the best option to handle transitions with percentage mode charts is to allow an empty x axis dimension in xy charts in general and behave like Visualize by rendering just a single data point. Percentage mode chart maps bucket dimension to break down first to always render meaningful percentage. |
x-pack/plugins/lens/public/xy_visualization/xy_visualization.tsx
Outdated
Show resolved
Hide resolved
| @@ -45,6 +45,24 @@ export function getSuggestions({ | |||
| table.columns.every((col) => col.operation.dataType !== 'number') || | |||
| table.columns.some((col) => !columnSortOrder.hasOwnProperty(col.operation.dataType)) | |||
| ) { | |||
| if (table.changeType === 'unchanged' && state) { | |||
There was a problem hiding this comment.
For the future:
I've been noticing that now that we have stacked percentage charts, I'm expecting the pie charts to transition to these instead of the regular XY charts. The smallest code change I can think of that would let us do this is to provide a "suggestion context" object like we've previously considered, and this would let the XY charts decide how to apply suggestions based on the previous chart type.
| pattern: '0,0[00]%', | ||
| }, | ||
| }; | ||
| } |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Can we use two decimal point as the maximum by default? |
…a into lens/stack-as-percentage
@cchaos Could you check and approve the scss changes? Pretty sure they are inherited your PR on top of this branch anyway. |
x-pack/plugins/lens/public/pie_visualization/render_function.tsx
Outdated
Show resolved
Hide resolved
| formatter = { | ||
| id: 'percent', | ||
| params: { | ||
| pattern: '0.[00]%', |
There was a problem hiding this comment.
There's no customization option for the user?
There was a problem hiding this comment.
I would also like to point out that the current slider for the decimals is really laggy and may need some sort of debounce. https://share.getcloudapp.com/rRu7AK6N
There was a problem hiding this comment.
@cchaos that looks like an existing issue for pie charts- this is the XY chart where there's no slider
There was a problem hiding this comment.
It's configurable by configuring the percentage format in the metric dimension. I think it's a good approach for now, if changing the format will become super common we can think about exposing it more prominently.
Let's iterate on that in separate PRs.
@cchaos I created a separate issue for the slider (#77039), as it's not really related to the changes here
| formatter = { | ||
| id: 'percent', | ||
| params: { | ||
| pattern: '0.[00]%', |
There was a problem hiding this comment.
I would also like to point out that the current slider for the decimals is really laggy and may need some sort of debounce. https://share.getcloudapp.com/rRu7AK6N
|
@elasticmachine merge upstream |
|
@wylieconlon Are you fine with the formatting solution for the scope of this PR? |
|
@elasticmachine merge upstream |
…a into lens/stack-as-percentage
💚 Build SucceededBuild metrics@kbn/optimizer bundle module count
page load bundle size
History
To update your PR or re-run it, just comment with: |
Related: #57389
Fixes: #72320
This PR implements a "percentage mode" for area and bar charts. There are quite some uncertainties around this I want to explore with this PR.
TODOs
Functionality
There are multiple options how to offer this functionality - it could be a flag in the chart level settings, a layer level setting or even a metric level setting. However I think this is best shown as a new "chart type" in the popover:
We would need an icon similar to the "stacked" chart icons.
This also includes a suggestion to show the current chart as percentage if a stacked chart is used:
It's possible to use a percentage layer along with a non-percentage layer:
Transitions between charts are described in #70703 (comment)
This PR also changes the default percentage formatting to maximum two decimal places (instead of always showing three)
Implementation
On a technical level these are simply new series types of the xy chart:
'bar_percentage_stacked' | 'bar_horizontal_percentage_stacked' | 'area_percentage_stacked'. If the type includespercentage, percentage mode will be activated on the series spec. Also, the axis formatter will be overwritten with a percentage formatter.This PR does not implement the "Other" bucket for the terms operation - I get why this is related but I think there are still a lot of valid use cases for a percentage mode even without this feature. IMHO we should split it out into a separate PR.