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 dashboard to refresh visualizations when the refresh button is clicked #27260

Closed
wants to merge 1 commit into from

Conversation

lukasolson
Copy link
Member

Summary

Fixes #23329.

This PR updates dashboard so that clicking on the refresh button actually refreshes visualizations.

As of #19172, visualizations don't inherit from the dashboard's search source. As a result, when the search source is updated (even when clicking on the "refresh" button), they don't automatically get the new data that's fetched.

This PR adds an additional property to some of the dashboard components that allows parent components to call reload() programmatically, rather than only when state changes happen.

Honestly I'm not too thrilled with this change, so I'm open to feedback about other ways we could accomplish this same thing. As far as I could tell the only mechanism for communication between the panels and the dashboard was via state changes.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@lukasolson lukasolson added review Feature:Dashboard Dashboard related features release_note:fix v7.0.0 Team:Visualizations Visualization editors, elastic-charts and infrastructure v6.6.0 labels Dec 14, 2018
@lukasolson lukasolson self-assigned this Dec 14, 2018
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@elasticmachine
Copy link
Contributor

💔 Build Failed

@lukasolson
Copy link
Member Author

Looks like I need to update some tests and add one to test this behavior.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@timroes
Copy link
Contributor

timroes commented Dec 17, 2018

Honestly I'm not too thrilled with this change, so I'm open to feedback about other ways we could accomplish this same thing. As far as I could tell the only mechanism for communication between the panels and the dashboard was via state changes.

I personally like the idea of having an explicit reload method on the embeddables. I think that is the better long term solution since we're trying to have everything between the outer layer and the inner embeddable passed and handled explicitly, rather than just triggering some events on some underlying classes.

I am not yet familar enough with the dashboard code, to check if that's a good way to keep track of all the embeddables. Maybe @stacey-gammon could elaborate on that part of the code a bit?

@stacey-gammon
Copy link
Contributor

The embeddables are all stored on the redux tree, so if you add a RELOAD action, I think you can do this without the registering/deregistering stuff, and I think without $scope.reload.

@lukasolson - lmk if you want to sync up and go over to see if we can do this without any extra angular stuff.

@lukasolson
Copy link
Member Author

Closed in favor of #27353.

@lukasolson lukasolson closed this Jan 2, 2019
@lukasolson lukasolson deleted the fix/dashboard-refresh branch December 2, 2019 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Dashboard Dashboard related features release_note:fix review Team:Visualizations Visualization editors, elastic-charts and infrastructure v6.6.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants