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

[Fix] The overwritten series color from the legend is not visible when I create a visualization from a dashboard #93435

Merged
merged 6 commits into from
Mar 8, 2021

Conversation

stratoula
Copy link
Contributor

@stratoula stratoula commented Mar 3, 2021

Summary

Fixes #93379

This bug occurs only when the dashboard.allowByValueEmbeddables: true.
If the user creates a visualization from the dashboard and overwrites the series color from the legend, the color is depicted on the dashboard. But when the user chooses to edit the visualization from the dashboard, the change is not depicted. It seems that while the uiState is saved correctly is not populated on the saved visualization, which results in the bug.

This PR fixes the bug, and also I have added a unit test for the getVisualizationInstanceFromInput method.

New-Dashboard-Elastic (2)

Checklist

…t visible when I create a visualization from a dashboard --no-verify
@stratoula stratoula changed the title [Visualizations] Fixes the overwritten series color from legend is no… [Fix] The overwritten series color from the legend is not visible when I create a visualization from a dashboard Mar 3, 2021
@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@stratoula stratoula added Feature:Visualizations Generic visualization features (in case no more specific feature label is available) v7.12.0 v7.13.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Mar 4, 2021
@stratoula stratoula marked this pull request as ready for review March 4, 2021 09:03
@stratoula stratoula requested a review from a team March 4, 2021 09:03
@stratoula stratoula added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Mar 4, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@stratoula stratoula added the Feature:Dashboard Dashboard related features label Mar 4, 2021
@stratoula stratoula requested a review from ThomThomson March 4, 2021 09:04
Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Glad that you covered this, thanks Stratoula!
The jest tests are looking great

Copy link
Contributor

@wylieconlon wylieconlon left a comment

Choose a reason for hiding this comment

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

I tested this PR and didn't find that it solved the problem for me. Steps to reproduce:

  1. Open an empty dashboard
  2. Click "add panel" and choose area chart
  3. Split series using terms aggregation, then add the X axis date histogram
  4. Save to dashboard
  5. Edit one of the colors from the dashboard
  6. Edit the visualization

The colors aren't shown in the visualization in this case.

@stratoula
Copy link
Contributor Author

stratoula commented Mar 5, 2021

@wylieconlon the colors that are changed by the dashboard and not by the visualization are saved on the dashboard and not on the visualization. The uiState from the dashboard is going to be deprecated by the way. This PR solves the problem when the user changes the colors from the visualization. The behavior that you describe is intended.

@wylieconlon
Copy link
Contributor

@stratoula that feels pretty unintuitive to me, but I won't block this PR for it- it definitely works from visualization to dashboard, but not the other way. cc @ThomThomson for this weird behavior

@stratoula
Copy link
Contributor Author

stratoula commented Mar 5, 2021

I won't disagree on that :) it is but I assume this is one of the reasons the presentation team wants to remove it and give a better UX to the users

@ThomThomson
Copy link
Contributor

@wylieconlon, this absolutely does have to be removed, and is tracked in: #88712

Unfortunately, it will be a minor breaking change because users will lose all color customizations stored in dashboards.

@stratoula
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 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
visualize 99.1KB 99.3KB +117.0B

History

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

@mbondyra
Copy link
Contributor

mbondyra commented Mar 8, 2021

LGTM, tested on Chrome 🆗

@stratoula stratoula merged commit 96c360b into elastic:master Mar 8, 2021
stratoula added a commit to stratoula/kibana that referenced this pull request Mar 8, 2021
…n I create a visualization from a dashboard (elastic#93435)

* [Visualizations] Fixes the overwritten series color from legend is not visible when I create a visualization from a dashboard  --no-verify

* Remove unecessary code

* Fix types

Co-authored-by: Kibana Machine <[email protected]>
stratoula added a commit to stratoula/kibana that referenced this pull request Mar 8, 2021
…n I create a visualization from a dashboard (elastic#93435)

* [Visualizations] Fixes the overwritten series color from legend is not visible when I create a visualization from a dashboard  --no-verify

* Remove unecessary code

* Fix types

Co-authored-by: Kibana Machine <[email protected]>
stratoula added a commit that referenced this pull request Mar 8, 2021
…n I create a visualization from a dashboard (#93435) (#93905)

* [Visualizations] Fixes the overwritten series color from legend is not visible when I create a visualization from a dashboard  --no-verify

* Remove unecessary code

* Fix types

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
stratoula added a commit that referenced this pull request Mar 8, 2021
…n I create a visualization from a dashboard (#93435) (#93906)

* [Visualizations] Fixes the overwritten series color from legend is not visible when I create a visualization from a dashboard  --no-verify

* Remove unecessary code

* Fix types

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Dashboard Dashboard related features Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.12.0 v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Dashboard] Legend color overwrite is lost when I save to a dashboard
6 participants