elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.82k stars 8.21k forks source link

[Embeddables] Communicate Data View Ids #137493

Open ThomThomson opened 2 years ago

ThomThomson commented 2 years ago

How it works now Dashboard needs to know which data views its child embeddables use in order to provide appropriate options when creating a filter or a control. Currently, most embeddables communicate their data views via their output. Dashboard reads the output, casts it and tries to grab the data view object from it.

Problems with this approach:

What should we do instead?: Instead, it would be a better pattern to create a new interface something like ICommunicatesDataViews, which comes with a method to access a Subject<string> called $onDataViewChange that the embeddable can publish to when the data view changes.

Notice this subject is of type string because it should only publish the data view Id, because the data views in use should already be cached in the data views service. We can access them this way rather than passing the full class instance around.

Another good change would be to make the top nav bar / the unified search bar take an array of dataViewIds, rather than an array of pre-fetched Data View objects.

More advanced fix A more complex fix would be to add a system that lets embeddable parents easily subscribe to changes in certain states of their children.

CC @dokmic @flash1293, any feedback or other options is welcome!

elasticmachine commented 2 years ago

Pinging @elastic/kibana-presentation (Team:Presentation)

elasticmachine commented 2 years ago

Pinging @elastic/kibana-app-services (Team:AppServicesSv)

vadimkibana commented 2 years ago

In general we have this architecture, where embeddable reports their state through the embeddable "output". (I'm not saying I'm a big fan of it.) But we have it, and it seems that embeddable "output" can be used here to subscribe to all embeddable data view changes; the whole data view does not need to be extracted - the output observable can be mapped to just data view ID, and further observable distinctUntilChanged() can be used to not fire observable events if ID does not change, effectively achieving the same.

Something like:

const dataViewId$ = embeddable.output$.pipe(
  map(dataView => dataView.id),
  distinctUntilChanged(),
);
ThomThomson commented 2 years ago

@vadimkibana this is the current implementation.

ThomThomson commented 1 year ago

This will be resolved by the Embeddables Rebuild project.