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 angular stack overflow by changing apply filters popover directiv… #48559

Merged
merged 1 commit into from Oct 18, 2019

Conversation

lizozom
Copy link
Contributor

@lizozom lizozom commented Oct 17, 2019

Summary

Fix angular stack overflow on dashboard application, by changing apply filters popover directive implementation

The bug was caused by a combination of now passing in the notification service into IndexPatterns and the custom way that the applyFiltersPopover directive used to watch filters and modify state in it.

This PR changes the way applyFiltersPopover watches state. It also moves modifying the state into the React component.

bee4212

Checklist

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

For maintainers

@lizozom lizozom added bug Fixes for quality problems that affect the customer experience review v8.0.0 Team:AppArch v7.5.0 labels Oct 17, 2019
@lizozom lizozom self-assigned this Oct 17, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@lizozom lizozom added the release_note:skip Skip the PR/issue when compiling release notes label Oct 17, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@mattkime
Copy link
Contributor

mattkime commented Oct 17, 2019

I'm unclear if this aims to solve all stack overflow issues or just a specific subset. I'm running the branch locally and looking at discover - adding and removing filters causes stack overflow errors.

Another action in discover which causes stack overflow - https://cl.ly/f62c957e90a8

@lizozom
Copy link
Contributor Author

lizozom commented Oct 18, 2019

@mattkime this is specific to the errors on dashboard. Sorry for not mentioning it in the issue. Now it seems we have more than one problem!

Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience release_note:skip Skip the PR/issue when compiling release notes review v7.5.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants