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

removes extraHandlers from expressions renderer #58336

Merged
merged 2 commits into from
Feb 24, 2020

Conversation

ppisljar
Copy link
Member

Summary

removes extra handlers from expressions renderer

  • adds uiState handler

partly addresses #50897

@ppisljar ppisljar requested a review from a team as a code owner February 24, 2020 12:53
@ppisljar ppisljar added release_note:skip Skip the PR/issue when compiling release notes review Team:AppArch v7.7.0 v8.0.0 labels Feb 24, 2020
@elasticmachine
Copy link
Contributor

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

@ppisljar
Copy link
Member Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@ppisljar ppisljar merged commit 63d5e38 into elastic:master Feb 24, 2020
ppisljar added a commit to ppisljar/kibana that referenced this pull request Feb 24, 2020
ppisljar added a commit that referenced this pull request Feb 25, 2020
@clintandrewhall
Copy link
Contributor

clintandrewhall commented May 8, 2020

Bummer. This kills some of my Expressions PoC, as Canvas utilizes a number of extraHandlers to render... so ReactExpressionRenderer blows up with the shape renderer:

https://github.com/elastic/kibana/blob/master/x-pack/plugins/canvas/canvas_plugin_src/renderers/shape/index.js#L83

Screen Shot 2020-05-07 at 9 48 25 PM

Any plans to allow for additional handler methods? Or are we needing to update to the events param?

@ppisljar
Copy link
Member Author

we need to agree of fixed set of handlers for expression functions and for render functions as else its impossible to reuse any of those (if i have no idea what handlers do i need to give to some function, if it may be using anything).

onResize sounds like something we might want to add. are there any others that you are missing ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes review v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants