-
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
[ML] Decouple edit-anomaly-swimlane UI action #179073
[ML] Decouple edit-anomaly-swimlane UI action #179073
Conversation
Pinging @elastic/ml-ui (:ml) |
x-pack/plugins/ml/public/ui_actions/edit_swimlane_panel_action.tsx
Outdated
Show resolved
Hide resolved
4062c0d
to
9be954f
Compare
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.
Action conversions LGTM! Code only review, but all of the conventions are followed very well. Great usage of the existing APIs, + creating new ones.
Left one very minor nit.
entityFields: PublishingSubject<MlEntityField[] | undefined>; | ||
} | ||
|
||
export type AnomalyChartsEmbeddableApi = HasType<AnomalyExplorerChartsEmbeddableType> & |
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.
This type is really clean and straightforward. Nice work!
This is not necessary, but interfaces have slightly better editor time performance than types, so we're starting to recommend using the interface
syntax whenever possible. This could be rewritten like:
export interface AnomalyChartsEmbeddableApi extends HasType<AnomalyExplorerChartsEmbeddableType>, MlEmbeddableBaseApi, AnomalyChartsFieldSelectionApi {}
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.
Updated in 517e8a8
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.
Tested latest changes and LGTM.
Is the 'Clear selection' action also covered here, or does that not need to be decoupled?
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: cc @darnautov |
Summary
Part of #178375
Part of #174967