[Discuss] Embeddable API improvements

[streamich]

Existing work

• Panel title
• Remove special handling for savedObjectId

Future possibilities for improvement

Use Presentable interface

Move Presentable interface from ui_actions into kibana_utils and make EmbeddableFactory extend the Presentable interface. This would make embeddable factories consistent with action factories. Presentable interface should have all fields necessary for presenting embeddable factories to users.

Use UiComponent interface

Use UiComponent interface for specifying UI building block that embeddable renders, instead of render(), destroy(), reload() and calling super.render() and super.destroy() and manually subscribing to "input" and "output" observables.

Simplify Embeddable API interfaces

Consider simplifying embeddable API surface to something very simple like below. Remove non-essential fields from the interfaces and consider if there are better places where those fields could live.

// New embeddable factory and embeddable interfaces.
export interface EmbeddableFactoryDefinition<Input> extends Partial<Presentable> {
  create(input$: Input | Subject<Input>): IEmbeddable<Input>;
}

export interface IEmbeddable<Input> {
  input$: Subject<Input>;
  Component: UiComponent<Input>;
  supportedTriggers(): Array<keyof TriggerContextMapping>;
}

// Registering an embeddable.
plugins.embeddable.registerEmbeddableFactory({
  id: 'lens',
  create: (props) => ({
    Component: reactToUiComponent(props => (
      <div>This is lens</div>
    )),
    supportedTriggers: () => ['VALUE_CLICK_TRIGGER'],
  }),
});

// Rendering an embeddable in a React component.
<div>
  <plugins.embeddable.ReactEmbeddable id="lens" input={{}} />
</div>

Improve/standartize "output"

• Consider removing and/or normalizing concept of embeddable's "output".
  • Title
  • .embeddableLoaded
• Consider if trigger context can take over some of the responsibilities of "output".
  • .editable
  • .editUrl
  • embeddable.getIsContainer()

Improve/standartize "input"

An input of the embeddable could be input$ observable instead of input value.

new Embeddable(input$);
// instead of
emb = new Embeddable(input);
emb.updateInput(newInput);

Input could be just read-only stream of values without ability to updateInput and without an embedded concept of "explicit input".

"Explicit input"

Make concept of "explicit input" more explicit (no pun intended). Possibly remove it from Embeddable itself and create a higher level abstraction which is an embeddable with explicit input.

Possibly for dashboard embeddables the "explicit input" could be built on top of the regular input.

interface DashboardEmbeddableInput<T> {
  viewMode: 'view' | 'edit';
  explicitInput: T;
  onExplicitInputChange: (explicitInputPatch: Partial<T>) => void;
}

Improve DX of embeddables in React code

• Expose <ReactEmbeddable> connected component from embeddable plugin's start contract, users would be able to use it directly to render embeddables

  <div>
    <plugins.embeddable.ReactEmbeddable id="lens" input={{}} />
  </div>

• Add integration for the above component to kibana_react.

• Expose embeddable child panel from contract.

cc @stacey-gammon @elastic/kibana-app-arch @elastic/kibana-app

Part of #71857

[stacey-gammon]

"Explicit input"

"Explicit" input is only a concept because of containers really. The idea for putting getExplicitInput on every factory was the following hypothetical scenario:

I have a TimeRangeEmbeddable and it's input is

interface TimeRangeEmbeddable extends EmbeddableInput {
  timeRange: TimeRange;
}

I want to add this to a DashboardContainer, which includes timeRange in its InheritedChildInput. I should be able to do so without passing in TimeRange explicitly because it's part of inherited input, when it belongs to such a container.

The best approach here is a bit tough to figure out. You can't do:

const embeddable = factory.create(input);
container.addNewEmbeddable(embeddable);

because an embeddable can't be instantiated without all of its required input data.
So instead the interface is:

  Container.addNewEmbeddable(type: string, explicitInput: Partial<EEI>):

The explicit input that is passed in is retrieved via factory.getExplicitInput();. The container handles merging it with its own InheritedChildInput when instantiating the child.

This is awkward, not properly type checked, and not even a needed feature right now. If I had to do it again I would try to simplify it.

If we decided something like this would be a nice feature to have, ideally I think we need a formal way to check which children can be added to which containers depending on their InheritedChildInput. With our needs right now though, this is probably overkill.

I'd be happy to see it simplified.

[stacey-gammon]

Expose connected component from embeddable plugin's start contract, users would be able to use it directly to render embeddables

Sgtm, though we have quite a few helpers already that are pretty close to this. We have

  <EmbeddableFactoryRenderer
    getEmbeddableFactory={getEmbeddableFactory}
    type={HELLO_WORLD_EMBEDDABLE}
    input={{ id: '1234' }}/>

Stateless, but only needs the one extra prop. We also have:

  <embeddableServices.EmbeddablePanel embeddable={embeddable} />

More clean up ideas that might be worth exploring

• Turning Embeddable from an abstract class to an interface with a helper creation method.
• Renaming EmbeddableFactory to EmbeddableDefinition and changing registerEmbeddableFactory to registerEmbeddableDefinition. Calling it a Factory seems to have caused a lot of confusion.

[streamich]

Team asked to give examples, so below I'm just giving more detailed examples for some things.

Use Presentable interface

Currently EmbeddableFactory has fields like type and getDisplayName.

interface EmbeddableFactory {
  type: string;
  getDisplayName: () => string;
}

For consistency with other places and reusability we could make EmbeddableFactory extend Presentable interface instead.

interface EmbeddableFactory extends Presentable<void> {
}

Use UiComponent interface

Currently, to render embeddable user has to do something like below:

class MyEmbeddable extends Embeddable {
  render(el) {
    super.render(el);
    ReactDOM.render(() => {
      // Subscribe to this.getInput$()
      // Subscribe to this.getOutput$()
    }, el);
  }

  destroy() {
    super.destroy();
    ReactDom.unmountComponentAtNode(this.domEl);
  }
}

The idea is to somehow simplify that, by using UiComponent interface, maybe like so:

abstract class Embeddable {
  Component: UiComponent;
}

Then implementers of embeddable would do something like:

class MyEmbeddable extends Embeddable {
  Component = reactToUiComponent(() => {
     // Subscribe to this.getInput$()
     // Ideally we would not have "output"
  });
}

Or alternatively interface could be with props:

abstract class Embeddable {
  Component: UiComponent<Input>;
}

And then input is provided to component through props.

class MyEmbeddable extends Embeddable {
  Component = reactToUiComponent((input) => {
    return <div>{input.title}</div>;
  });
}

Improve DX in React code

A number of improvements can be done here.

1. embeddable plugin could return few pre-connected components from its plugin contract. Like for embeddable itself and one for embeddable child panel.

For "embeddable itself" it would mean we can render just a single embeddable (and as Stacey points out above we have something similar).

<plugins.embeddable.ReactEmbeddable id="lens" input={{}} />

We could also export a component for a child panel. (This is something @flash1293 expressed interest in.)

<plugins.embeddable.ReactEmbeddableChildPanel id="..." container={{}} />

That way the circled props in red below would disappear, as they would be pre-connected by the embeddable plugin:

2. We could add helpers for embeddable plugin to kibana_react plugin, that way Embeddable React components would be possible to import statically.

import { Embeddable } from 'src/plugins/kibana_react';

<Embeddable id="lens" input={{}} />

To achieve that an application at its root would set up a React context provider.

import { KibanaContextProvider } from 'src/plugins/kibana_react';

<KibanaContextProvider services={{ embeddable }}>
  <MyApp />
</KibanaContextProvider>

3. Another thing could be to use React Context internally in embeddable plugin itself so we don't need to pass all the dependencies through props to every component multiple steps down.

[ppisljar]

Presentable interface: no strong opinion, i would love to see where else this would be usefull if we would to move it to a kibana_urls

UIComponent: i am definetely for doing this, as well as doing it everywhere else we can make use of it.

simplify interface: seems something that would take quite some time, are you sure thats all we need ?

improve output: would be nice, not sure if its possible

improve input: no strong feeling, both achieve the same, i would not spend to much time refactoring this.

react component: yes!

i think it would make sense to create separate issues for some of those which are very concerete:

• UiComponent
• React Component

[vadimkibana]

Thank you for contributing to this issue, however, we are closing this issue due to inactivity as part of a backlog grooming effort. If you believe this feature/bug should still be considered, please reopen with a comment.