[Embeddable Refactor] Create Decoupled Presentation Panel #172017

ThomThomson · 2023-11-27T19:36:03Z

Summary

This PR contains much of the prep work required to decouple the Embeddables system from Kibana in anticipation of its deprecation and removal.

To accomplish this, several new systems and concepts have been added to Kibana which will eventually replace portions of the Embeddable framework. An overview of each new concept can be found in this PR description. If you would like more information, the architecture document goes into more detail.

The Presentation Panel

Previously, the generic frame component that linked React components to the UI Actions library was tightly coupled to the Embeddables system and the Embeddables plugin. This PR removes that coupling.

Now, any React component can be linked to the UI Actions library by passing it as a prop to the newly created PresentationPanel compoent. This linkage is created via an API object, which is passed from the component via the use imperative handle hook. Methods and state on this API will be used by the Presentation Panel. For instance, if the API publishes a title, the latest value of that title will be displayed in the panel header.

Publishing Subjects

This API system works great as-is for completely imperative things like functions, onEdit for instance, but for state which can change, we need a standardized system which can publish individual pieces of state in a reactive form. To fulfil this requirement, this PR introduces Publishing subjects.

Publishing subjects are essentially read only RXJS behavior subjects. This allows APIs to pass around changeable pieces of state, react to those changes, and get the last published value.

Note

Publishing subjects are meant to be readonly because they are not meant to be a source of truth. Instead, they are meant to forward or publish state from another source of truth, whether it's from a Use State, some other RXJS subject, from Redux, or some other state management system.

A number of convenience functions are included to allow translating state from React components, or state from legacy Embeddables into Publishing Subjects. Additionally, hooks are included to allow components to render the latest state from the publishing subjects.

UI Actions changes

A number of changes have been made to both the UI Actions framework itself, and a number of Actions registered with those frameworks.

Dashboard & Presentation Panel Actions

Because UI Actions can now theoretically be linked to any component via the new Presentation Panel component, actions which were tightly coupled with the old Embeddable system should be refactored to list their actual dependencies declaratively, which effectively decouples them from the embeddable system.

For instance, instead of the edit panel action requiring an Embeddable instance as its context, it instead requires any API object with an onEdit method and a viewMode currently set to edit. This allows for more clarity in requirements, and allows for more flexibility in what each action actually does.

Note

It's important to note that this PR leaves some code in place to ensure interoperability between Actions which are still tightly coupled to Embeddables, and actions which have been updated. This includes:

The name of the key in all context objects is still embeddable. This should eventually be updated to api
The new API methods and state accessors are simply appended onto the existing Embeddable instances. This means that actions which rely on legacy methods can do so alongside actions that rely on the new methods.

Subscribe to compatibility changes

Two new optional methods have been added to the UI Action definition: subscribeToCompatibilityChanges and couldBecomeCompatible. These methods are meant to allow actions to declare exactly which pieces of state from a given API will cause them to become compatible or incompatible.

Previously, any state change to any embeddable or its parent would cause the entire tree of Actions to re-calculate their compatibility state. With these changes in place, that code has been removed, and any action which is expected to frequently change its compatibility state in response to some other state can do so via this system.

Architecture / Progress

This PR accomplishes some of the items on our roadmap towards the complicated architecture which will temporarily exist to ease our transition towards the new Embeddables framework. Below, you can see the pieces of this architecture added by this PR, and the pieces which will be added by the followup.

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

…-fix'

…fix'

…anel' into createPresentationPanel

…fix'

…ocs'

…anel

…anel' into createPresentationPanel

…fix'

…x Dashboard replace panel

…anel' into createPresentationPanel

…anel

…fix'

…anel

…d malformed filters crashing Lens

…anel

…fix'

…eatePresentationPanel

…anel

nreese · 2024-01-17T15:20:14Z

src/plugins/embeddable/public/lib/embeddables/compatibility/edit_legacy_embeddable.tsx

+import { isExplicitInputWithAttributes } from '../embeddable_factory';
+import { EmbeddableInput } from '../i_embeddable';
+
+const getLatestAppId = async (): Promise<string | undefined> => {


Could this be simplified to just return core.application.currentAppId$.pipe(first()).toPromise();

Actually, toPromise is deprecated. instead, you could just use await lastValueFrom(core.application.currentAppId$)

I'm not sure that lastValueFrom will work properly in this case. I think it waits for the observable to complete.

Good call, firstValueFrom works perfectly here and makes the code much shorter! Updated this in 610daf5

nreese · 2024-01-17T15:45:56Z

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


    if (this.parentSubscription) {
      this.parentSubscription.unsubscribe();
    }
    return;
  }

+  public async untilInitializationFinished(): Promise<void> {
+    return new Promise((resolve) => {


How about just return lastValueFrom (this.initializationFinished)

I think lastValueFrom will work great here, because I complete the stream when the initialization is finished. Let me try it out.

How about using firstValueFrom then?

can confirm that lastValueFrom worked great here! Updated this in 610daf5. Thanks for the suggestion!

I had to revert this change because of a jest test failure. I've opened a followup issue to see what went wrong here.

nreese · 2024-01-17T15:53:01Z

src/plugins/inspector/public/adapters/has_inspector_adapters.ts

+/**
+ * The interface that the adapters used to open an inspector have to fullfill.
+ */
+export interface Adapters {


This type already exists and is declared in src/plugins/inspector/common/adapters/types.ts

Good call, updated that in 610daf5! Best not to duplicate the any here.

nreese · 2024-01-17T17:10:28Z

src/plugins/inspector/public/index.ts

 export * from './types';
-export * from '../common/adapters';


you will need to export Adapters from common/adapters

nreese

kibana-presentation changes LGTM other than some minor comments. This is an impressive accomplishment and is a huge step forward for the new embeddable system. Thanks for all of the hard work and high quality changes.

code review, tested in chrome

nreese · 2024-01-17T18:22:40Z

...esentation_panel/public/panel_actions/customize_panel_action/custom_time_range_badge.test.ts

+    const onChange = jest.fn();
+    updateTimeRange(mockTimeRange);
+    updateTimeRange(undefined);
+    action.subscribeToCompatibilityChanges(context, onChange);


Move updates to after subscriptions is created

nreese · 2024-01-17T18:28:13Z

...plugins/presentation_panel/public/panel_actions/edit_panel_action/edit_panel_action.test.tsx

+  it('calls onChange when view mode changes', () => {
+    const onChange = jest.fn();
+    updateViewMode('view');
+    action.subscribeToCompatibilityChanges(context, onChange);


Call updateViewMode after creating subscription

nreese · 2024-01-17T18:30:50Z

src/plugins/presentation_panel/public/panel_component/presentation_panel.tsx

+    );
+  }
+
+  if (loading || !value?.Panel || !value?.unwrappedComponent)


Should an error state be displayed if loading=false and value.Panel or value.unwrappedComponent do not exist? In this case, the panel will get stuck showing loading screen forever. I think an error would help, especially for developers, who are the ones most likely to trigger this case.

Good call. I've updated the error section above to also trigger when (!loading && (!value?.Panel || !value?.unwrappedComponent))

nreese · 2024-01-17T18:36:44Z

src/plugins/presentation_panel/public/panel_component/presentation_panel_internal.tsx

+
+  const [initialLoadComplete, setInitialLoadComplete] = useState(!dataLoading);
+  if (dataLoading === false && !initialLoadComplete) {
+    setInitialLoadComplete(true);


Should setInitialLoadComplete be inside an effect. State should not be set from rendering.

This code was just copied over from #171238. Refer to this comment for more info on why it was done this way. I'd be okay with wrapping this in a useEffect.

kibana-ci · 2024-01-17T22:11:53Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: ed734ba

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`dashboard`	392	418	+26
`embeddable`	117	97	-20
`inspector`	59	60	+1
`lens`	1212	1235	+23
`links`	92	115	+23
`presentationPanel`	-	78	+78
total			+131

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-containers`	-	33	+33
`@kbn/presentation-library`	-	10	+10
`@kbn/presentation-publishing`	-	106	+106
`embeddable`	440	419	-21
`inspector`	96	100	+4
`presentationPanel`	-	11	+11
`uiActions`	93	103	+10
total			+153

Async chunks

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

id	before	after	diff
`dashboard`	379.4KB	382.3KB	+2.8KB
`discover`	564.1KB	564.2KB	+36.0B
`embeddable`	12.3KB	2.0KB	-10.2KB
`lens`	1.4MB	1.4MB	+75.0B
`links`	24.8KB	24.9KB	+28.0B
`maps`	2.9MB	2.9MB	-12.0B
`presentationPanel`	-	8.2KB	+8.2KB
`visualizations`	270.6KB	270.4KB	-252.0B
total			+643.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`embeddable`	7	8	+1
`presentationPanel`	-	3	+3
total			+4

Page load bundle

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

id	before	after	diff
`dashboard`	36.5KB	36.9KB	+455.0B
`embeddable`	78.9KB	60.6KB	-18.2KB
`embeddableEnhanced`	6.7KB	7.0KB	+299.0B
`expressions`	98.7KB	98.9KB	+127.0B
`inspector`	22.1KB	22.2KB	+140.0B
`lens`	41.0KB	41.2KB	+130.0B
`presentationPanel`	-	40.4KB	+40.4KB
`uiActions`	19.8KB	20.3KB	+564.0B
total			+23.8KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/presentation-containers`	-	34	+34
`@kbn/presentation-library`	-	10	+10
`@kbn/presentation-publishing`	-	137	+137
`embeddable`	541	519	-22
`inspector`	123	127	+4
`presentationPanel`	-	11	+11
`uiActions`	135	149	+14
total			+188

async chunk count

id	before	after	diff
`embeddable`	2	1	-1
`presentationPanel`	-	1	+1
total			-0

ESLint disabled line counts

id	before	after	diff
`@kbn/presentation-publishing`	-	2	+2
`embeddable`	7	4	-3
`presentationPanel`	-	6	+6
total			+5

References to deprecated APIs

id	before	after	diff
`dashboard`	25	30	+5
`embeddable`	25	22	-3
total			+2

Total ESLint disabled count

id	before	after	diff
`@kbn/presentation-publishing`	-	2	+2
`embeddable`	9	6	-3
`presentationPanel`	-	6	+6
total			+5

History

💔 Build #188000 failed d2eb105
💔 Build #187994 failed 0d900568cf4f1fdea25d82fec44dccea77279921
💔 Build #187991 failed ecf34ac04e797a55a53db52f2adab40f92d9ccce
💔 Build #187985 failed 102c1904e8e2c287391f4b4bd96dba848c27c158
💔 Build #187975 failed 610daf53bea0ab740eea302f9d06b77896fd0ccb

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

…2017) Closes elastic#167426 Closes elastic#167890 Contains much of the prep work required to decouple the Embeddables system from Kibana in anticipation of its deprecation and removal. Co-authored-by: Nathan Reese <[email protected]>

…r badges (#186102) Closes #185914 ## Summary The linked regression is a result of #172017, which accidentally removed tooltip support from badges - this PR adds it back so that the controls deprecation badge now shows the tooltip as expected on both hover and focus: <p align="center"> <img src="https://github.com/elastic/kibana/assets/8698078/aca8075b-bce5-4fa6-b4b9-e38e00aefad8" alt="Gif showcasing the deprecation tooltip"/> </p> I also ensured that the tooltip contents are available for screen readers by adding a conditional `aria-label` - if a tooltip is not provided, then the screen reader will read the badge's contents instead. **How to Test** 1. Create a legacy input control embeddable by adding the following to your URL: `/app/visualize#/create?type=input_control_vis` 2. Add that panel to a dashboard 3. You should see the deprecation badge and its corresponding tooltip :+1: ### Checklist - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

initialize presentation panel and presentation publishing packages

2ac2782

ThomThomson changed the title ~~[Embeddable Refactor] Created Decoupled Presentation Panel~~ [Embeddable Refactor] Create Decoupled Presentation Panel Nov 27, 2023

kibanamachine and others added 20 commits November 27, 2023 20:54

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

4fe15eb

…-fix'

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

050712c

…fix'

Initialize readmes and add tests

4130eb2

remove Dashboard from embeddable tests

67a679d

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

af34cb7

…anel' into createPresentationPanel

Fix & add jest tests

4041175

remove accidental modifications, fix tsconfig

be1b357

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

067b3fc

…fix'

[CI] Auto-commit changed files from 'node scripts/generate codeowners'

f396fd1

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

48a2238

…ocs'

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

c2d7a88

…anel

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

84e0e04

…anel' into createPresentationPanel

fix jest tests & types

e1aea30

fix test subjects

907b80d

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

e5c6593

…fix'

revert class name changes, make drilldown notification responsive, fi…

ebf90b1

…x Dashboard replace panel

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

0fcca6a

…anel' into createPresentationPanel

fix more tests

83fc26b

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

a3d55e9

…anel

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

9874b83

…fix'

ThomThomson mentioned this pull request Dec 11, 2023

[Embeddable] Fix embeddable panel loader #173000

Merged

ThomThomson and others added 7 commits January 3, 2024 12:47

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

04cd8a4

…anel

Update translations

0ebe79f

change package naming convention

d95a171

add error handling and refactor presentation_panel. Try catch to avoi…

6c03cf7

…d malformed filters crashing Lens

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

7322dc7

…anel

Finish merge

31d7696

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

082df6e

…fix'

ThomThomson added 2 commits January 16, 2024 17:39

Merge remote-tracking branch 'origin/createPresentationPanel' into cr…

215964b

…eatePresentationPanel

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

b341128

…anel

nreese reviewed Jan 17, 2024

View reviewed changes

ThomThomson force-pushed the createPresentationPanel branch 3 times, most recently from ecf34ac to 0d90056 Compare January 17, 2024 18:02

Review feedback, fix i18n

d2eb105

ThomThomson force-pushed the createPresentationPanel branch from 0d90056 to d2eb105 Compare January 17, 2024 18:25

nreese approved these changes Jan 17, 2024

View reviewed changes

ryankeairns approved these changes Jan 17, 2024

View reviewed changes

undo lastValueFrom change, fix jest tests, apply review feedback

ed734ba

ThomThomson mentioned this pull request Jan 17, 2024

[Embeddables] Use lastValueFrom in untilInitializationFinished #175068

Closed

ThomThomson merged commit 64ebaff into elastic:main Jan 17, 2024
54 checks passed

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

This was referenced Jan 22, 2024

[Dashboard] Phase Events not Reported #175234

Closed

[Embeddable Refactor] Publish phase events #175245

Merged

ThomThomson added the project:embeddableRebuild label Feb 2, 2024

Heenawter assigned ThomThomson Feb 2, 2024

nreese mentioned this pull request Feb 20, 2024

[Dashboard] Inspect panel action displays "untitled" instead of panel title #176870

Closed

ThomThomson mentioned this pull request Mar 7, 2024

[Embeddable] Flexible Titling System #148680

Closed

Heenawter mentioned this pull request Apr 18, 2024

[Dashboard] Descriptions not showing up on by-reference panels after upgrade #181172

Closed

Heenawter mentioned this pull request Jun 12, 2024

[Embeddable Rebuild] Add tooltip support to PresentationPanel header badges #186102

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Embeddable Refactor] Create Decoupled Presentation Panel #172017

[Embeddable Refactor] Create Decoupled Presentation Panel #172017

ThomThomson commented Nov 27, 2023 •

edited

Loading

nreese Jan 17, 2024

nreese Jan 17, 2024 •

edited

Loading

ThomThomson Jan 17, 2024

ThomThomson Jan 17, 2024 •

edited

Loading

nreese Jan 17, 2024

ThomThomson Jan 17, 2024

nreese Jan 17, 2024

ThomThomson Jan 17, 2024 •

edited

Loading

ThomThomson Jan 17, 2024

nreese Jan 17, 2024

ThomThomson Jan 17, 2024 •

edited

Loading

nreese Jan 17, 2024

nreese left a comment

nreese Jan 17, 2024

ThomThomson Jan 17, 2024

nreese Jan 17, 2024

ThomThomson Jan 17, 2024

nreese Jan 17, 2024

ThomThomson Jan 17, 2024

nreese Jan 17, 2024

ThomThomson Jan 17, 2024

kibana-ci commented Jan 17, 2024

API count

async chunk count

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

[Embeddable Refactor] Create Decoupled Presentation Panel #172017

[Embeddable Refactor] Create Decoupled Presentation Panel #172017

Conversation

ThomThomson commented Nov 27, 2023 • edited Loading

Summary

The Presentation Panel

Publishing Subjects

UI Actions changes

Dashboard & Presentation Panel Actions

Subscribe to compatibility changes

Architecture / Progress

Checklist

Choose a reason for hiding this comment

nreese Jan 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThomThomson Jan 17, 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

ThomThomson Jan 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThomThomson Jan 17, 2024 • edited Loading

Choose a reason for hiding this 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibana-ci commented Jan 17, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

async chunk count

ESLint disabled line counts

References to deprecated APIs

Total ESLint disabled count

History

ThomThomson commented Nov 27, 2023 •

edited

Loading

nreese Jan 17, 2024 •

edited

Loading

ThomThomson Jan 17, 2024 •

edited

Loading

ThomThomson Jan 17, 2024 •

edited

Loading

ThomThomson Jan 17, 2024 •

edited

Loading