-
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
[Analyst Experience Components] Dashboard & Control Group APIs #150121
[Analyst Experience Components] Dashboard & Control Group APIs #150121
Conversation
…eDashboards/dashboardBuildingRefactor
…embeddable tools to centralize on embeddable, used this for dashboard
…eDashboards/dashboardBuildingRefactor
…eDashboards/dashboardBuildingRefactor
…dashboardBuildingRefactor
…//github.com/ThomThomson/kibana into portableDashboards/dashboardBuildingRefactor
…dashboardBuildingRefactor
…//github.com/ThomThomson/kibana into portableDashboards/dashboardBuildingRefactor
…dashboardBuildingRefactor
…//github.com/ThomThomson/kibana into portableDashboards/dashboardBuildingRefactor
…dashboardBuildingRefactor
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.
LGTM on hosts view
@@ -6,12 +6,15 @@ | |||
*/ | |||
|
|||
import React, { useCallback, useEffect, useRef } from 'react'; | |||
import { ControlGroupContainer, type ControlGroupInput } from '@kbn/controls-plugin/public'; | |||
import { | |||
ControlGroupAPI, |
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.
nit:
ControlGroupAPI, | |
type ControlGroupAPI, |
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.
ML changes LGTM
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.
LGTM from Security solution perspective.
…eDashboards/dashboardBuildingRefactor
x-pack/plugins/security_solution/public/dashboards/pages/details/index.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Angela Chuang <6295984+angorayc@users.noreply.github.com>
@elasticmachine merge upstream |
…dashboardBuildingRefactor
…//github.com/ThomThomson/kibana into portableDashboards/dashboardBuildingRefactor
…//github.com/ThomThomson/kibana into portableDashboards/dashboardBuildingRefactor
…dashboardBuildingRefactor
💚 Build Succeeded
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
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: cc @ThomThomson |
Summary
✅ Pre-Merge Flaky Test Check here
This PR includes a number of tightly coupled enhancements and cleanups. It:
Creates a Standard Analyst Experience Component API
This PR aligns the Portable Dashboard renderer and the Control Group renderer to a new API structure using
useImperativeHandle
rather than the overcomplicated and mostly unused wrapper provider system.Before
Architecture
The previous design had the embeddable instance return a wrapper component which provided access to redux tools like dispatch, actions & select.
Code Example
After
Architecture
The new design centralizes all of the redux tools directly onto the embeddable object, which is then transformed into an API. Instead of using a wrapper, any components are free to interact with the internal component state via the api object.
Code Example
Changes how Presentation team component renderers are consumed
This PR removes all instances of
withSuspense
used to load the dashboard renderer or the control group renderer. Instead, these will be exported from their plugins directly, with special care taken to ensure they don't balloon the bundle size. The slight increase to dashboard's page load bundle is due to the dashboard renderer being exported.Changes how Portable Dashboard Containers are initialized
This PR shifts async dashboard initialization logic from the
DashboardContainer
into theDashboardContainerFactory
. This simplifies a lot of code and state types:By reference / by value
input type.Redux Tools
This PR creates a generic implementation of redux tools that isn't coupled to the embeddable class. This will allow analyst experience components that match this new pattern to be created entirely without embeddables attached.
Test coverage
This PR restores jest unit tests that cover the rendering, lifecycles and state management of the dashboard creation process. These used to be defined here, but were marked flaky, skipped and subsequently removed. They have been reinstated in
src/plugins/dashboard/public/dashboard_container/embeddable/create/create_dashboard.test.ts
This PR also introduces tests for the portable dashboard renderer and for the control group renderer.
Follow-ups
A few follow-up PRs should:
Checklist
Delete any items that are not applicable to this PR.