Skip to content

Fix angular stack overflow by changing apply filters popover directiv…#48559

Merged
alexwizp merged 1 commit intoelastic:masterfrom
lizozom:bug/dashbord-stack-overflow
Oct 18, 2019
Merged

Fix angular stack overflow by changing apply filters popover directiv…#48559
alexwizp merged 1 commit intoelastic:masterfrom
lizozom:bug/dashbord-stack-overflow

Conversation

@lizozom
Copy link
Copy Markdown
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
Copy Markdown
Contributor

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

@lizozom lizozom force-pushed the bug/dashbord-stack-overflow branch from 367ebeb to 2aea7e5 Compare October 17, 2019 17:09
@lizozom lizozom added the release_note:skip Skip the PR/issue when compiling release notes label Oct 17, 2019
@lizozom lizozom requested review from alexwizp and ppisljar October 17, 2019 17:34
@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@mattkime
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor

@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.

5 participants