-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Move react controls into controls plugin #190166
Move react controls into controls plugin #190166
Conversation
/ci |
/ci |
/ci |
/ci |
1 similar comment
/ci |
@elasticmachine merge upstream |
/ci |
/ci |
321359e
to
7c497b1
Compare
@elasticmachine merge upstream |
/ci |
3d50454
to
5b8c172
Compare
/ci |
Pinging @elastic/kibana-presentation (Team:Presentation) |
/ci |
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
async chunk count
ESLint disabled line counts
References to deprecated APIs
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes LGTM! Thank you for paying attention to the bundle size, because it could have easily ballooned. Good to see the compatibility check split out into its own file.
Also ran the example plugin. Not necessary to fix in this PR, but I'm seeing one small issue there:
The first time you create a control, there is no Data view selected. In main, the control group gets this info from the Dashboard - so I assume this is just something that is missing so far and will be resolved when we move this new version back into the Dashboard.
I already have a follow-up PR that resolves this and cleans up some types. Just want to limit the number of changes in this PR since its just supposed to be a move |
Move react_controls from examples plugin to controls plugin. This PR does not effect user facing features since it does not effect the legacy control group implementation or its usage. Future PRs will integrate the new control group with dashboard and ControlGroupRenderer and then remove the legacy control group implementation.
PR increases page load bundle size because of new action registration and the control group react embeddable registration. This will be a temporary increase as removing the legacy control group will remove the legacy embeddable factory registration, which is larger then react embeddable registration.