Embeddable examples on the platform and included with `--run-examples` flag #52111

stacey-gammon · 2019-12-03T19:32:46Z

Embeddable examples built and tested on the platform

Builds off #52027 and adds some new embeddable examples that are in the new platform. Also adds more text to make it more helpful, so it's not just for testing purposes but built to help developers as well.

Run with yarn start --run-examples and navigate to the Embeddable explorer to play around.

In addition to the examples, some code changes were made as well:

Embeddable exposes getEmbeddableFactory on application mount context
New EmbeddableRoot react helper component that takes care of rendering the embeddable on a react root node.
New EmbeddableFactoryRenderer react helper component that takes care of getting the right factory, and instantiating a new embeddable with the given input, and then mounting and rendering it to the DOM.
New withEmbeddableSubscription react helper HOC which takes care of the inner react component binding itself to state changes from the embeddable object.

Some extra clean up:

test/examples test suite wasn't running on ci, now fixed.

More things to follow:

EmbeddableFactoryRenderer is not type safe in regard to the input shape and the type of embeddable. Adding more type safety, similar to how the data search services does it would be great. Filed: [Embeddables] Extra type safety #52887
Dashboard container example
More examples to help explain/test more of the concepts.

fyi @streamich @oatkiller

stacey-gammon · 2019-12-11T20:02:19Z

src/plugins/embeddable/public/lib/embeddables/with_subscription.tsx

+import * as Rx from 'rxjs';
+import { IEmbeddable, EmbeddableInput, EmbeddableOutput } from './i_embeddable';
+
+export const withEmbeddableSubscription = <


cc @streamich -- am adding this helper react class. All my examples ended up with the same subscription code, etc. Lmk if you have any suggestions.

This withEmbeddableSubscription higher order component seems to be OK. I'm planning to improve Embeddable DX in general for React, so you would not need to write such helpers.

elasticmachine · 2019-12-12T17:09:07Z

Pinging @elastic/kibana-app-arch (Team:AppArch)

stacey-gammon · 2019-12-13T14:36:43Z

@elasticmachine merge upstream

lizozom

Added a couple of UX oriented comments.
Overall LGTM

lizozom · 2019-12-13T15:34:28Z

x-pack/plugins/advanced_ui_actions/public/can_inherit_time_range.test.ts

-  HelloWorldEmbeddable,
-  HelloWorldContainer,
-} from '../../../../src/plugins/embeddable/public/lib/test_samples';
+import { HelloWorldContainer } from '../../../../src/plugins/embeddable/public/lib/test_samples';


Will this import work once #52447 is merged?

yea, there is a /* eslint-disable */ surrounding it (as there was originally). The example plugins actually make this easier because they can export the examples at the top level, while we don't want that in here. I've thought about replacing all these samples with ones from the examples repo but some really are only built for testing not so much examples. Some of this could still probably be cleaned up and more example embeddables used for testing.

lizozom · 2019-12-13T16:59:46Z

examples/embeddable_examples/public/plugin.ts

+  }
+
+  public start(core: CoreStart, deps: EmbeddableExamplesStartDependencies) {
+    deps.embeddable.registerEmbeddableFactory(


Why would you register some in the setup phase and dome in the start?

I assume they internally consume services available only in the start phase, but how can we make this clearer to devs (including ourselves)?

Yep, because deps.embeddable.getEmbeddableFactory is only available in start, not setup. :( I could move them all to start... or make getEmbeddableFactory available in both. I'm not sure what the best thing is, I think this whole when to expose a registry getter we still don't have a super clear recommendation of. Unless we do and I am just not caught up yet. For now I'll add a comment.

lizozom · 2019-12-13T17:06:58Z

examples/embeddable_explorer/public/list_container_example.tsx

+            paddingSize="none"
+            role="figure"
+          >
+            <EmbeddableFactoryRenderer


I find the fact that filter applies only to inner items to be confusing
If you could hide empty items or gray them out, it would be much clearer.

Yea, I agree, but I wanted an example of a container passing data down to the child. I agree the scenario you mention would make more sense but we'd need them to implement something so the child could communicate "i have a match vs no match" to it's parent and we don't have that kind of communication yet.

Think of it like on the dashboard - we pass down the filters and the child does something with that, but the container never changes how the child is rendered.

It is something to think about though. I wonder if there is a better example I could do here. Maybe pass down "highlight" and highlight the search terms instead of filter by them? What do you think?

Actually that's a great idea - search highlight would make it much clearer

lizozom · 2019-12-13T17:24:09Z

src/plugins/embeddable/public/lib/embeddables/embeddable_root.tsx

+  error?: string;
+}
+
+export class EmbeddableRoot extends React.Component<Props> {


Couldn't EmbeddableFactoryRenderer and EmbeddableRoot be the same component?
It feels like they couldn't really exist one without the other, or could they?

EmbeddableFactoryRenderer encapsulates more logic, but EmbeddableRoot is helpful if you need a reference to the embeddable object, like in todo_embeddable_example. Adding some text there:

You may also notice this example uses `EmbeddableRoot` instead of the `EmbeddableFactoryRenderer` helper component. This is because it needs a reference to the embeddable to update it, and `EmbeddableFactoryRenderer` creates and holds the embeddable instance internally.

examples/embeddable_explorer/public/hello_world_embeddable_example.tsx

lizozom · 2019-12-13T17:41:58Z

examples/embeddable_examples/public/todo/todo_component.tsx

+  output: EmbeddableOutput;
+}
+
+export function TodoEmbeddableComponentInner({ input: { icon, title, task } }: Props) {


If it's a TODO app, I would expect to be able to add new items.
I could't figure out how to add a new item, just edit the existing one?

Not yet, but it's a good idea to add and I might be able to showcase the add panel flyout. Just felt like this PR was getting big so wanted to stop. There are def more examples of use cases I'd like to add.

examples/embeddable_explorer/public/application.tsx

…beddable-example

stacey-gammon · 2019-12-16T16:34:17Z

@lizozom - I updated the examples so that the parent does actually filter out the children (I realized the current infrastructure will actually support this). I also added highlighting. This change of code actually helped me realize some bugs:

The react components should not be calling embeddable.destroy() - just because it might not be rendered on the screen doesn't mean it's not still useful. For example, the parent container may choose not to render some children because they don't match the filter, but when the filter changes, they then should be re-rendered. So they are still children in the parent, just temporarily not on the screen.
With the above fix I realized that none of my embeddables were correctly implementing their destroy method. Need to call super.destroy and unmount the react node.

…beddable-example

streamich

LGTM

elasticmachine · 2019-12-16T19:28:49Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: b1eec65

History

💔 Build #15799 failed c153889
💚 Build #15585 succeeded fe0e6eb
💚 Build #15565 succeeded f51ffa8
💔 Build #15396 failed c725678
💚 Build #15254 succeeded 68741f3

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

…` flag (elastic#52111) * Add a new platform embeddable example plugin * Remove extra hello world test impl. * cleanup * code review updates * Change example to highlight and have parent filter out children * Fix deep comparison of embeddable prop * adjust help text

…` flag (#52111) (#53190) * Add a new platform embeddable example plugin * Remove extra hello world test impl. * cleanup * code review updates * Change example to highlight and have parent filter out children * Fix deep comparison of embeddable prop * adjust help text

…examples` flag (elastic#52111)" This reverts commit 7e67d1f.

stacey-gammon force-pushed the 2019-12-03-embeddable-example branch 2 times, most recently from 5331cc3 to 1363ae0 Compare December 3, 2019 21:58

stacey-gammon changed the title ~~[skip ci] 2019 12 03 embeddable example~~ [skip ci] Embeddable examples on the platform and part of examples dir Dec 3, 2019

stacey-gammon force-pushed the 2019-12-03-embeddable-example branch from 1363ae0 to 80c2e00 Compare December 3, 2019 22:02

stacey-gammon added the release_note:skip Skip the PR/issue when compiling release notes label Dec 3, 2019

stacey-gammon changed the title ~~[skip ci] Embeddable examples on the platform and part of examples dir~~ Embeddable examples on the platform and included with --run-examples flag Dec 4, 2019

stacey-gammon force-pushed the 2019-12-03-embeddable-example branch 3 times, most recently from 0285490 to 77f5eb2 Compare December 4, 2019 17:24

stacey-gammon mentioned this pull request Dec 4, 2019

Add top level examples folder and command to run, --run-examples. #52027

Merged

stacey-gammon force-pushed the 2019-12-03-embeddable-example branch 2 times, most recently from 041e4d3 to ad20043 Compare December 4, 2019 20:37

elastic deleted a comment from elasticmachine Dec 4, 2019

stacey-gammon mentioned this pull request Dec 11, 2019

Embeddable tests turned off: Functional tests bundle ui/new_platform twice #43087

Closed

stacey-gammon force-pushed the 2019-12-03-embeddable-example branch 4 times, most recently from d84bd5c to 86b893f Compare December 11, 2019 20:01

stacey-gammon commented Dec 11, 2019

View reviewed changes

stacey-gammon force-pushed the 2019-12-03-embeddable-example branch 8 times, most recently from 5c92648 to a3fe7c1 Compare December 12, 2019 15:06

elastic deleted a comment from elasticmachine Dec 12, 2019

stacey-gammon force-pushed the 2019-12-03-embeddable-example branch from a3fe7c1 to 3992a12 Compare December 12, 2019 15:12

stacey-gammon requested review from streamich and lizozom December 12, 2019 16:57

stacey-gammon added Feature:Embedding Embedding content via iFrame Team:AppArch v7.6.0 v8.0.0 labels Dec 12, 2019

stacey-gammon added the review label Dec 12, 2019

stacey-gammon added 3 commits December 12, 2019 15:33

Add a new platform embeddable example plugin

5284e31

Remove extra hello world test impl.

c6bf3b4

cleanup

68741f3

stacey-gammon force-pushed the 2019-12-03-embeddable-example branch from 3992a12 to 68741f3 Compare December 12, 2019 20:33

lizozom approved these changes Dec 13, 2019

View reviewed changes

stacey-gammon added 2 commits December 13, 2019 17:37

Merge branch 'master' of github.com:elastic/kibana into 2019-12-03-em…

a241d87

…beddable-example

code review updates

f51ffa8

stacey-gammon force-pushed the 2019-12-03-embeddable-example branch from c725678 to f51ffa8 Compare December 13, 2019 23:30

stacey-gammon added 2 commits December 14, 2019 08:24

Merge branch 'master' of github.com:elastic/kibana into 2019-12-03-em…

fe0e6eb

…beddable-example

Change example to highlight and have parent filter out children

6117caf

Merge branch 'master' of github.com:elastic/kibana into 2019-12-03-em…

c153889

…beddable-example

streamich approved these changes Dec 16, 2019

View reviewed changes

stacey-gammon added 2 commits December 16, 2019 12:46

Fix deep comparison of embeddable prop

69915e6

adjust help text

b1eec65

stacey-gammon merged commit 7e67d1f into elastic:master Dec 16, 2019

stacey-gammon mentioned this pull request Dec 16, 2019

[7.x] Embeddable examples on the platform and included with --run-examples flag (#52111) #53190

Merged

spalger added a commit to spalger/kibana that referenced this pull request Dec 17, 2019

Revert "Embeddable examples on the platform and included with `--run-…

4ea7a15

…examples` flag (elastic#52111)" This reverts commit 7e67d1f.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Embeddable examples on the platform and included with `--run-examples` flag #52111

Embeddable examples on the platform and included with `--run-examples` flag #52111

stacey-gammon commented Dec 3, 2019 •

edited

Loading

stacey-gammon Dec 11, 2019

streamich Dec 14, 2019

elasticmachine commented Dec 12, 2019

stacey-gammon commented Dec 13, 2019

lizozom left a comment

lizozom Dec 13, 2019 •

edited

Loading

stacey-gammon Dec 13, 2019

lizozom Dec 13, 2019

stacey-gammon Dec 13, 2019

lizozom Dec 13, 2019

stacey-gammon Dec 13, 2019

lizozom Dec 16, 2019

lizozom Dec 13, 2019

stacey-gammon Dec 13, 2019 •

edited

Loading

lizozom Dec 13, 2019

stacey-gammon Dec 13, 2019

stacey-gammon commented Dec 16, 2019 •

edited

Loading

streamich left a comment

elasticmachine commented Dec 16, 2019

Embeddable examples on the platform and included with --run-examples flag #52111

Embeddable examples on the platform and included with --run-examples flag #52111

Conversation

stacey-gammon commented Dec 3, 2019 • edited Loading

Embeddable examples built and tested on the platform

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Dec 12, 2019

stacey-gammon commented Dec 13, 2019

lizozom left a comment

Choose a reason for hiding this comment

lizozom Dec 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stacey-gammon Dec 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stacey-gammon commented Dec 16, 2019 • edited Loading

streamich left a comment

Choose a reason for hiding this comment

elasticmachine commented Dec 16, 2019

💚 Build Succeeded

History

Embeddable examples on the platform and included with `--run-examples` flag #52111

Embeddable examples on the platform and included with `--run-examples` flag #52111

stacey-gammon commented Dec 3, 2019 •

edited

Loading

lizozom Dec 13, 2019 •

edited

Loading

stacey-gammon Dec 13, 2019 •

edited

Loading

stacey-gammon commented Dec 16, 2019 •

edited

Loading