[Exploratory view] Allow ability add extra actions in lens embeddable#123713
[Exploratory view] Allow ability add extra actions in lens embeddable#123713shahzad31 merged 10 commits intoelastic:mainfrom
Conversation
flash1293
left a comment
There was a problem hiding this comment.
Thanks for working on this, this seems like a super useful feature. Pinging @stacey-gammon and @ppisljar - could you have a look as well whether this is how actions are supposed to be used? For a bunch of things this kind of "local" actions which don't go through the global registry seem pretty helpful.
|
|
||
| export type EmbeddableComponentProps = (TypedLensByValueInput | LensByReferenceInput) & { | ||
| withActions?: boolean; | ||
| withActions?: boolean | Action[]; |
There was a problem hiding this comment.
It seems a bit weird to re-use the existing prop for this - what about splitting out extraActions on this level as well?
There was a problem hiding this comment.
i find using existing prop as a client to be honest cleaner. I mean now if use has to pass extra actions, they just passed it as an array.
Adding an extra prop means, we will have to withActions true and then pass extraActions prop.
Happy to amend it, if you feel strong about it.
There was a problem hiding this comment.
It's definitely a nit, I'm just a bit concerned people will be confused (I was initially). Maybe we can further disambiguate and call it withDefaultActions and extraActions? That what you can also only show your special extra actions and hide the default ones.
|
Pinging @elastic/uptime (Team:uptime) |
| appId | ||
| ); | ||
|
|
||
| const hrefPath = createExploratoryViewUrl( |
There was a problem hiding this comment.
href and hrefPath seem to be equal to me when running this, is there a reason we need to call createExploratoryViewUrl twice? Are there cases where onlyPath being undefined will cause the first call to yield a divergent output?
There was a problem hiding this comment.
@justinkambic i have removed the part about undefined description.
There was a problem hiding this comment.
hrefPath is actually a route path. i think there is some room for improvement, i will add another function instead of modifying the current one.
There was a problem hiding this comment.
@justinkambic i updated the code to make it bit more clear
|
cc @vadimkibana |
💚 Build SucceededMetrics [docs]Module Count
Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
History
To update your PR or re-run it, just comment with: cc @shahzad31 |
justinkambic
left a comment
There was a problem hiding this comment.
Functionality LGTM - had one minor code mention, wanted to make sure it's desired.
| return true; | ||
| }, | ||
| execute: async (context: ActionExecutionContext<object>) => { | ||
| alert('I am an extra action'); |
There was a problem hiding this comment.
Do we want this code here?
There was a problem hiding this comment.
Yes, this is a lens embeddable example.
flash1293
left a comment
There was a problem hiding this comment.
LGTM, didn't test again. Thanks for this addition!
…fix-potential-race-condition-when-screenshotting * 'main' of github.com:elastic/kibana: (75 commits) [Reporting] Logging improvements while generating reports (elastic#123802) [Uptime] Default alert connectors email settings (elastic#123244) Update comparison series styles to match the main series (elastic#123858) [RAC][Uptime] remove extra dot from the uptime alert connector message (elastic#124000) [Exploratory view] Allow ability add extra actions in lens embeddable (elastic#123713) [SecuritySolution][Investigations] Add message about missing index in data view in analyzer (elastic#122859) [TSVB] Formatting in the left axis is not respected when I have two separate axis (elastic#123903) [Discover] Remove services from component dependencies (elastic#121691) Stop IM rule execution if there are no events (elastic#123811) [Security Solution][Endpoint] Update Fleet Trusted Apps and Host Isolation Exception cards to use exception list summary API (elastic#123900) [Security Solution][Exceptions] Switches modal to flyout component (elastic#123408) [Workplace Search] Fix bug where modal visible after deleting a group (elastic#123976) [Alerting] Remove state variables from action variable menu (elastic#123702) replace deprecated api usage (elastic#123970) Fix package policy merge logic for boolean values (elastic#123974) [Security Solution][Endpoint][Policy] Remove GET policy list api route (elastic#123873) Reenable alert_add test suite (elastic#123862) [Fleet] Remove usage of IFieldType in Fleet (elastic#123960) [Lists] Add an instance of `ExceptionListClient` with server extension points turned off to context object provided to callbacks (elastic#123885) [Maps] Add execution context (elastic#123651) ... # Conflicts: # x-pack/plugins/screenshotting/server/browsers/chromium/driver_factory/index.ts
| }: AddToCaseProps & { getToastText: (thaCase: Case | SubCase) => MountPoint<HTMLElement> }) => { | ||
| appId, | ||
| }: AddToCaseProps & { | ||
| appId?: 'security' | 'observability'; |
There was a problem hiding this comment.
Hey @shahzad31 👋 Just curious as to why we need to specify security here?
I'm probably missing something but wouldn't appId always be set to observability when it is defined?
cc: @cnasikas
There was a problem hiding this comment.
You are right. The appId should be observability for Observability. Also, AddToCaseProps contains appId?: 'security' | 'observability';

Summary
This requirement came out of while doing POC for security #120004
So extracting it from that PR into a separate PR.
Added ability to add extra actions in Lens embeddable and subsequently use that ability in exploratory view to add
Add to caseSaveExplore' actionsUpdated the example to describe use case
Testing
Start kibana with
yarn start --run-examplesfind observability exploratory view example
You should be able to use
ExploreandSave Vizactions from the cog icon