Decouple actions from embeddables: step 1 #44503
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Pinging @elastic/kibana-app-arch |
f378893 to
34a463f
Compare
💔 Build Failed |
streamich
reviewed
Aug 30, 2019
| const triggerContext = {}; | ||
| if (isEmbeddable(embeddable) && isTriggerContext(triggerContext)) { | ||
| if (isEmbeddable(embeddable)) { | ||
| // @ts-ignore | ||
| const result = await action.isCompatible({ |
There was a problem hiding this comment.
It is not recommended to use @ts-ignore, if there is some TypeScript type mismatch you can cast to specific type or cast to any.
(action as any).isCompatible
(action.isCompatible as any)
(action as SomeType).isCompatible34a463f to
304f791
Compare
💔 Build Failed |
2bd4f90 to
fb08367
Compare
💚 Build Succeeded |
…bout checking isCompatible.
💚 Build Succeeded |
ppisljar
approved these changes
Sep 5, 2019
| context.triggerContext && | ||
| context.triggerContext.filters !== undefined | ||
| ); | ||
| return Boolean(root.getInput().filters !== undefined && context.filters !== undefined); |
There was a problem hiding this comment.
check embeddable exists as well
There was a problem hiding this comment.
Yep, should be done in latest commit!
…couple-action-embeds
💚 Build Succeeded |
stacey-gammon
added a commit
to stacey-gammon/kibana
that referenced
this pull request
Sep 5, 2019
* Decouple actions from embeddables: step 1 * prefer as any instead of is-ignore * Remove unneccessary test, no more triggerContext to be null. * Fix bug and fix the test that should have caught it. Be more strict about checking isCompatible.
stacey-gammon
added a commit
that referenced
this pull request
Sep 5, 2019
* Decouple actions from embeddables: step 1 * prefer as any instead of is-ignore * Remove unneccessary test, no more triggerContext to be null. * Fix bug and fix the test that should have caught it. Be more strict about checking isCompatible.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes second part of #40491
Second part will be to pull into a separate plugin.
fixes #44342
Dev Docs
The action interface no longer requires an embeddable and
triggerContextto be passed in. The shape ofActionContextis now completely up to the specific action implementation.Previously:
Now: