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] Support new legendStats and old valuesInLegend saved object properties #181035

Closed
2 tasks
mbondyra opened this issue Apr 17, 2024 · 1 comment · Fixed by #181953
Closed
2 tasks

[Lens] Support new legendStats and old valuesInLegend saved object properties #181035

mbondyra opened this issue Apr 17, 2024 · 1 comment · Fixed by #181953
Assignees
Labels
Feature:Lens Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects

Comments

@mbondyra
Copy link
Contributor

mbondyra commented Apr 17, 2024

Background:

Lens currently supports the Legend functionality, which displays the current values of the chart in the legend.

Screenshot 2024-04-17 at 13 19 20 Screenshot 2024-04-17 at 13 19 34

This functionality is set by adding the following properties in Lens saved objects:

  1. For xy charts, we set state.visualization.valuesInLegend===true
  2. For partition charts, we set state.visualization.layers[0...n].showValuesInLegend===true

We want to extend this functionality to display multiple values in charts' legend.

Screenshot 2024-04-17 at 13 53 33

The simple boolean would not work to allow different metrics in legend. Instead we should use an array of LegendStats to store the data:

enum LegendStats {
   values: 'values',
   average: 'average',/// etc...
} 

Changes needed

  1. For xy charts, we need to:
    a. If state.visualization.valuesInLegend===true, set a new property state.visualization.legend.legendStats: ['values']
    a. remove state.visualization.valuesInLegend property if exists.

  2. For partition charts, we need to
    a. If state.visualization.layers[0...n].showValuesInLegend===true, set a new property state.visualization.layers[0...n].legendStats: ['values']
    a. remove state.visualization.layers[0...n].showValuesInLegend property if exists.

This would require a 2-stage work to ensure backwards compatibility and merge these two stages at least a week apart to ensure serverless separate releases:

Work plan

@mbondyra mbondyra added Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens labels Apr 17, 2024
@elasticmachine
Copy link
Contributor

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

@kibanamachine kibanamachine added this to Long-term goals in Lens Apr 17, 2024
@mbondyra mbondyra self-assigned this Apr 17, 2024
@mbondyra mbondyra changed the title [Lens] Migrate valuesInLegend saved object property to legendStats [Lens] Support valuesInLegend saved object property to legendStats Apr 23, 2024
@mbondyra mbondyra changed the title [Lens] Support valuesInLegend saved object property to legendStats [Lens] Support new legendStats and old valuesInLegend saved object properties Apr 23, 2024
mbondyra added a commit that referenced this issue Apr 26, 2024
…on for legend statistics (#180917)

## Summary

This PR addresses phase one of
#181035.

Doesn't introduce any user facing changes. 
It starts supporting a new saved object property `legendStats` while
supporting a old `valuesInLegend` property. In this PR, `legendStats:
['values']` and `valuesInLegend:true` are treated as equal. When loading
the saved object, `valuesInLegend:true` is transformed to
`legendStats:['values']`. After loading the document, the Lens app logic
is built around the new `legendStats` property.
When user saves the saved object, we do a reverse operation- we save the
runtime state `legendStats:['values']` as `valuesInLegend: true` to
ensure backwards compatibility.


![image](https://github.com/elastic/kibana/assets/4283304/5e09b062-b4d3-424d-b8a8-79a08f4d6260)


Changes for runtime state:

- For xyCharts, the `valuesInLegend?: boolean ` property is replaced
with a more extensible `legend.legendStats?: LegendStats[]` interface

- For partition charts, the `showValuesInLegend?: boolean` property is
replaced with `legendStats?: LegendStats[]`.

after loading - in initialize function:

```ts
export function convertToRuntime(
  state: XYPersistedState,
  annotationGroups?: AnnotationGroups,
  references?: SavedObjectReference[]
) {
  const outputState = needsInjectReferences(state)
    ? injectReferences(state, annotationGroups, references)
    : state;
  if ('valuesInLegend' in outputState) {
    return convertToLegendStats(outputState);
  }
  return outputState;
}
```

before saving :

```ts
export function convertToPersistable(state: XYState) {
  const persistableState: XYPersistedState = convertToValuesInLegend(state);
  /.../
}
```

In the future the `legendStats` prop would contain also other types of
stats -see the [issue](#176583).
mbondyra added a commit that referenced this issue May 3, 2024
…ration for legend statistics (#181953)

## Summary

To the code owners,

This PR updates the saved object of Lens with the following changes: the
property `valuesInLegend: boolean` is converted to the new property
`legendStats: ["currentAndLastValue"]` (cartesian) or `legendStats:
["value"]`(partition) to accommodate displaying additional legend
statistics in the future. There are no user-facing changes introduced. I
have updated all saved object shapes in the code to adhere to the new
structure. While the old structure will continue to function properly,
this update ensures cleaner code.

Fixes #181035 (phase 2). It's is
a continuation of #180917.

In comparison to the phase 1, we've removed the conversion of
legendStats to valuesInLegend which was initially added in the previous
PR to ensure backward compatibility for serverless applications.

<img width="807" alt="Screenshot 2024-04-25 at 10 51 50"
src="https://github.com/elastic/kibana/assets/4283304/6c23d035-f64d-4fb1-96c6-d30392ae6752">

I won't merge till the end of the week as it has to be released
separately from phase 1.

---------

Co-authored-by: Cauê Marcondes <55978943+cauemarcondes@users.noreply.github.com>
Co-authored-by: Alex Szabo <alex.szabo@elastic.co>
yuliacech pushed a commit to yuliacech/kibana that referenced this issue May 3, 2024
…ration for legend statistics (elastic#181953)

## Summary

To the code owners,

This PR updates the saved object of Lens with the following changes: the
property `valuesInLegend: boolean` is converted to the new property
`legendStats: ["currentAndLastValue"]` (cartesian) or `legendStats:
["value"]`(partition) to accommodate displaying additional legend
statistics in the future. There are no user-facing changes introduced. I
have updated all saved object shapes in the code to adhere to the new
structure. While the old structure will continue to function properly,
this update ensures cleaner code.

Fixes elastic#181035 (phase 2). It's is
a continuation of elastic#180917.

In comparison to the phase 1, we've removed the conversion of
legendStats to valuesInLegend which was initially added in the previous
PR to ensure backward compatibility for serverless applications.

<img width="807" alt="Screenshot 2024-04-25 at 10 51 50"
src="https://github.com/elastic/kibana/assets/4283304/6c23d035-f64d-4fb1-96c6-d30392ae6752">

I won't merge till the end of the week as it has to be released
separately from phase 1.

---------

Co-authored-by: Cauê Marcondes <55978943+cauemarcondes@users.noreply.github.com>
Co-authored-by: Alex Szabo <alex.szabo@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Lens Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
No open projects
Lens
  
Long-term goals
2 participants