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

[Lens] always retain source order for multi-metric partition chart layers #151949

Merged

Conversation

drewdaemon
Copy link
Contributor

@drewdaemon drewdaemon commented Feb 22, 2023

Summary

Fix #151006

This PR includes two changes

  1. the partition_vis expression renderer now orders partitions by the source order. This overrides the default descending-by-size partition sorting behavior in elastic-charts that was being followed previously
  2. the dimension buttons and metric column IDs sent to the expression are now both kept in the order specified by the datasource. This allows users to order slices by reordering the dimensions in the layer panel as they can with other visualization types (XY, table). This also fixed a drag-and-drop bug.

When users upgrade, they will see the partitions in their multi-metric charts reorder if the dimensions were out of order with the datasource. That seems fine to me since they can just drag the dimensions into the configuration they had before if they notice/care.

Screen.Recording.2023-02-23.at.2.05.41.PM.mov

Checklist

@drewdaemon drewdaemon added Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens labels Feb 22, 2023
@drewdaemon drewdaemon marked this pull request as ready for review February 23, 2023 20:56
@drewdaemon drewdaemon requested a review from a team as a code owner February 23, 2023 20:56
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations)

@drewdaemon
Copy link
Contributor Author

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
expressionPartitionVis 52.6KB 52.9KB +280.0B
lens 1.3MB 1.3MB +70.0B
total +350.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

Although the bug is fixed, now I cant order correctly if I have a terms agg/ Here is an example with the alphabetical ordering:

lens

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

I was wrong, this is how it works in main too. It works differently that agg based, this is why I was confused. Fix LGTM

@drewdaemon drewdaemon merged commit 662bd9a into elastic:main Feb 28, 2023
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Feb 28, 2023
Copy link
Contributor

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

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

Code changes LGTM

sloanelybutsurely pushed a commit to sloanelybutsurely/kibana that referenced this pull request Mar 8, 2023
bmorelli25 pushed a commit to bmorelli25/kibana that referenced this pull request Mar 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Lens release_note:fix Team:Visualizations Visualization editors, elastic-charts and infrastructure v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Lens] Coloring problem on multi metric partition chart
6 participants