[Embeddables rebuild] Decouple add panel trigger #176110

ThomThomson · 2024-02-01T21:14:01Z

Summary

This PR decouples the ADD_PANEL_TRIGGER from the Embeddables framework by making it take a PresentationContainer as context instead.

This PR also adds the ability to create new panels to the PresentationContainer interface. This allows us to register actions to create new React Embeddables.

How to test

The example has been updated to include an action that creates Eui Markdown React Embeddables. You should be able to run yarn start --run-examples then build Dashboards with Eui Markdown panels.

…decoupleAddPanelTrigger

ThomThomson · 2024-02-05T17:34:31Z

/ci

…fix'

ThomThomson · 2024-02-05T17:43:49Z

/ci

…decoupleAddPanelTrigger' into embeddableRefactor/decoupleAddPanelTrigger

ThomThomson · 2024-02-05T21:32:13Z

/ci

elasticmachine · 2024-02-05T22:53:24Z

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

stratoula · 2024-02-06T07:29:07Z

/ci

stratoula

Changes LGTM, tested ES|QL panel creation and everything works as expected. Very nice simplification Devon at least on the consumer side. Me gusta! 👏

nreese

Really nice to see it all come together.

nreese · 2024-02-06T19:47:44Z

examples/embeddable_examples/public/react_embeddables/eui_markdown_react_embeddable.tsx

+  // Create and register an action which allows this embeddable to be created from
+  // the dashboard toolbar context menu.
+  // -----------------------------------------------------------------------------
+  uiActions.registerAction<EmbeddableApiContext>({


How about moving action registration to plugin.setup method. That way, the action can be moved to its own file as well.

Done in 192fd05

nreese · 2024-02-06T19:52:51Z

packages/presentation/presentation_containers/interfaces/presentation_container.ts

 }

 export type PresentationContainer = Partial<PublishesViewMode> &
  PublishesLastSavedState & {
+    addNewPanel: <ApiType extends unknown = unknown>(
+      panel: PanelPackage,
+      silent?: boolean


how about renaming silent to something less cryptic, like displaySuccessToast

I actually purposefully went more crypic with this one, because the presentation container interface could be fulfilled with any implementation, and they may not use a toast or anything of the sort.

How about displaySuccess? Its vague on how success is displayed but conveys what this value toogles.

Sure that works! displaySuccess also has the bonus of being the default when no argument is passed in. Likely, most implementations will want to avoid the toast.

done in 192fd05

nreese · 2024-02-06T20:04:06Z

src/plugins/dashboard/public/dashboard_container/embeddable/dashboard_container.tsx

+      throw new EmbeddableFactoryNotFoundError(panelPackage.panelType);
+    }
+
+    if (this.trackPanelAddMetric) {


Should trackPanelAddMetric be moved to top of function so it can track react embeddables as well?

Good call, done in 192fd05

nreese · 2024-02-06T20:04:54Z

src/plugins/dashboard/public/dashboard_container/embeddable/dashboard_container.tsx

+      this.setScrollToPanelId(newEmbeddable.id);
+      this.setHighlightPanelId(newEmbeddable.id);
+
+      if (!silent) {


Should toast also be displayed when react embeddables are added?

Good call. Done in 192fd05

…decoupleAddPanelTrigger' into embeddableRefactor/decoupleAddPanelTrigger

…decoupleAddPanelTrigger

kibana-ci · 2024-02-06T21:59:49Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 70bcc77

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #31 / cases security and spaces enabled: trial get_connectors push needsToBePushed sets needs to push to false when the status of a case was changed after the last push
[job] [logs] FTR Configs #57 / fleet rejects request to create a new fleet server hosts if host url is different from default

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`@kbn/presentation-publishing`	115	114	-1
`embeddable`	452	454	+2
total			+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`dashboard`	385.8KB	382.0KB	-3.8KB
`lens`	1.4MB	1.4MB	-88.0B
total			-3.9KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`embeddable`	65.8KB	65.9KB	+125.0B
`lens`	41.9KB	42.3KB	+406.0B
`presentationPanel`	40.4KB	40.4KB	+41.0B
total			+572.0B

Unknown metric groups

API count

id	before	after	diff
`embeddable`	557	559	+2

History

💛 Build #191545 was flaky b18a51f
💚 Build #191544 succeeded b18a51f
💛 Build #191488 was flaky 35cc730
💔 Build #191455 failed a4d726c

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

nreese

LGTM
code review, tested in chrome

Decouples the `ADD_PANEL_TRIGGER` from the Embeddables framework by making it take a `PresentationContainer` as context instead.

decouple add panel trigger from legacy Embeddable framework

1e84b8c

ThomThomson added the project:embeddableRebuild label Feb 2, 2024

ThomThomson added 4 commits February 2, 2024 15:17

Merge remote-tracking branch 'upstream/main' into embeddableRefactor/…

7b46d6c

…decoupleAddPanelTrigger

update mocks

2dbc548

allow Dashboard to create React Embeddables

4c15762

Merge remote-tracking branch 'upstream/main' into embeddableRefactor/…

c0cb727

…decoupleAddPanelTrigger

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

a4d726c

…fix'

ThomThomson added 2 commits February 5, 2024 16:31

fix jest tests, make panel package initial state optional

241f9db

Merge remote-tracking branch 'refs/remotes/origin/embeddableRefactor/…

35cc730

…decoupleAddPanelTrigger' into embeddableRefactor/decoupleAddPanelTrigger

ThomThomson marked this pull request as ready for review February 5, 2024 22:53

ThomThomson requested review from a team as code owners February 5, 2024 22:53

Merge branch 'main' into embeddableRefactor/decoupleAddPanelTrigger

b18a51f

botelastic bot added the Feature:Embedding Embedding content via iFrame label Feb 6, 2024

stratoula approved these changes Feb 6, 2024

View reviewed changes

nreese self-requested a review February 6, 2024 19:09

nreese reviewed Feb 6, 2024

View reviewed changes

ThomThomson added 3 commits February 6, 2024 15:45

review feedback

192fd05

Merge remote-tracking branch 'refs/remotes/origin/embeddableRefactor/…

9806fda

…decoupleAddPanelTrigger' into embeddableRefactor/decoupleAddPanelTrigger

Merge remote-tracking branch 'upstream/main' into embeddableRefactor/…

70bcc77

…decoupleAddPanelTrigger

nreese approved these changes Feb 7, 2024

View reviewed changes

ThomThomson merged commit 23b4b53 into elastic:main Feb 7, 2024
19 checks passed

kibanamachine added v8.13.0 backport:skip This commit does not require backporting labels Feb 7, 2024

fkanout pushed a commit to fkanout/kibana that referenced this pull request Feb 8, 2024

[Embeddables rebuild] Decouple add panel trigger (elastic#176110)

3bcb372

Decouples the `ADD_PANEL_TRIGGER` from the Embeddables framework by making it take a `PresentationContainer` as context instead.

fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024

[Embeddables rebuild] Decouple add panel trigger (elastic#176110)

41fa32e

Decouples the `ADD_PANEL_TRIGGER` from the Embeddables framework by making it take a `PresentationContainer` as context instead.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Embeddables rebuild] Decouple add panel trigger #176110

[Embeddables rebuild] Decouple add panel trigger #176110

ThomThomson commented Feb 1, 2024 •

edited

Loading

ThomThomson commented Feb 5, 2024

ThomThomson commented Feb 5, 2024

ThomThomson commented Feb 5, 2024

elasticmachine commented Feb 5, 2024

stratoula commented Feb 6, 2024

stratoula left a comment

nreese left a comment

nreese Feb 6, 2024

ThomThomson Feb 6, 2024

nreese Feb 6, 2024

ThomThomson Feb 6, 2024

nreese Feb 6, 2024

ThomThomson Feb 6, 2024 •

edited

Loading

ThomThomson Feb 6, 2024

nreese Feb 6, 2024

ThomThomson Feb 6, 2024

nreese Feb 6, 2024

ThomThomson Feb 6, 2024

kibana-ci commented Feb 6, 2024

API count

nreese left a comment

[Embeddables rebuild] Decouple add panel trigger #176110

[Embeddables rebuild] Decouple add panel trigger #176110

Conversation

ThomThomson commented Feb 1, 2024 • edited Loading

Summary

How to test

ThomThomson commented Feb 5, 2024

ThomThomson commented Feb 5, 2024

ThomThomson commented Feb 5, 2024

elasticmachine commented Feb 5, 2024

stratoula commented Feb 6, 2024

stratoula left a comment

Choose a reason for hiding this comment

nreese left a 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

ThomThomson Feb 6, 2024 • 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

kibana-ci commented Feb 6, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Public APIs missing comments

Async chunks

Page load bundle

API count

History

nreese left a comment

Choose a reason for hiding this comment

ThomThomson commented Feb 1, 2024 •

edited

Loading

ThomThomson Feb 6, 2024 •

edited

Loading