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.

Sign up for GitHub

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

Jump to bottom

[Discover] Add "Hide chart" / "Show chart" persistence #88603

Merged
merged 15 commits into from
Feb 10, 2021
Merged

[Discover] Add "Hide chart" / "Show chart" persistence #88603

merged 15 commits into from
Feb 10, 2021

Conversation

kertal
Copy link
Member

@kertal kertal commented Jan 18, 2021 •
edited
Loading

Summary

PR #83633 introduced "show/hide" chart in Discover. This PR adds hideChart to application state and saved search.
image

  • This change is propagated to URL (app state), opening shared links of Discover with a hidden chart, will also lead to a hidden chart
  • The state of the chart is also saved when saving a search
  • When the chart is hidden, no aggregated data is requested
  • When switching the chart from hidden to displayed, this leads to requesting data (since aggregated data needs to be requested)

Fixes #86293

Checklist

@kertal kertal added the Feature:Discover Discover Application label Jan 18, 2021
@kertal kertal self-assigned this Jan 18, 2021
@kertal
Copy link
Member Author

kertal commented Jan 25, 2021

@elasticmachine merge upstream

kertal added 2 commits January 27, 2021 09:32
@kertal
Rename hideHistogram to hideChart
a85a754 
- it's more generic and also it's the text displayed in the UI, maybe we let the user chose another visualization in the future?
@kertal
Merge branch 'kertal-pr-2020-01-18-discover-persist-chart-state' of g…
7206db5 
…ithub.com:kertal/kibana into kertal-pr-2020-01-18-discover-persist-chart-state
@kertal kertal changed the title [Discover] Add hideHistogram to state and saved search [Discover] Add "Hide chart" / "Show chart" persistence Jan 27, 2021
@kertal
Copy link
Member Author

kertal commented Feb 2, 2021

@elasticmachine merge upstream

@spalger
Copy link
Contributor

spalger commented Feb 3, 2021

jenkins test this

@kertal
Copy link
Member Author

kertal commented Feb 5, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

merge conflict between base and head

@kertal
Copy link
Member Author

kertal commented Feb 8, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

merge conflict between base and head

@kertal kertal added Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.12.0 v8.0.0 release_note:feature Makes this part of the condensed release notes labels Feb 9, 2021
@kertal kertal marked this pull request as ready for review February 9, 2021 09:38
@kertal kertal requested a review from a team February 9, 2021 09:38
@elasticmachine
Copy link
Contributor

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

@kertal kertal requested a review from majagrubic February 9, 2021 09:38
@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
discover 420.2KB 420.8KB +590.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
discover 69.1KB 69.1KB +40.0B

Saved Objects .kibana field count

Every field in each saved object type adds overhead to Elasticsearch. Kibana needs to keep the total field count below Elasticsearch's default limit of 1000 fields. Only specify field mappings for the fields you wish to search on or query. See https://www.elastic.co/guide/en/kibana/master/development-plugin-saved-objects.html#_mappings

id before after diff
search 10 11 +1

History

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

majagrubic
majagrubic approved these changes Feb 10, 2021
View reviewed changes
Copy link
Contributor

@majagrubic majagrubic left a comment

Choose a reason for hiding this comment

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

Tested this in Chrome on Mac OSX for persisting the show/hide histogram on saved searches and works as expected.

@kertal kertal merged commit 40f44a9 into elastic:master Feb 10, 2021
@kertal kertal mentioned this pull request Feb 10, 2021
Merged
kertal added a commit to kertal/kibana that referenced this pull request Feb 10, 2021
@kertal
[Discover] Add "Hide chart" / "Show chart" persistence (elastic#88603)
faeb713
kertal added a commit that referenced this pull request Feb 10, 2021
@kertal
[Discover] Add "Hide chart" / "Show chart" persistence (#88603) (#90961)
bd8642b
@LeeDr LeeDr mentioned this pull request Feb 24, 2021
Closed
@timroes timroes added release_note:enhancement and removed release_note:feature Makes this part of the condensed release notes labels Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@majagrubic majagrubic majagrubic approved these changes

Assignees

@kertal kertal

Labels
Feature:Discover Discover Application release_note:enhancement Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.12.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Discover] Improve "hide/show chart" to be persistable
6 participants
@kertal @spalger @kibanamachine @elasticmachine @majagrubic @timroes