Skip to content

[Embeddable Rebuild] Make React embeddables work in Canvas#179667

Merged
Heenawter merged 24 commits intoelastic:mainfrom
Heenawter:fix-canvas-react-embeddables_2024-03-28
Apr 3, 2024
Merged

[Embeddable Rebuild] Make React embeddables work in Canvas#179667
Heenawter merged 24 commits intoelastic:mainfrom
Heenawter:fix-canvas-react-embeddables_2024-03-28

Conversation

@Heenawter
Copy link
Copy Markdown
Contributor

@Heenawter Heenawter commented Mar 28, 2024

Closes #179548

Summary

This PR makes it so that React embeddables will now work in Canvas. It does so by doing two main things:

  1. It ensures that the ReactEmbeddableRenderer is used in Canvas when an embeddable exists in the React embeddable registry - if it does not exist, it continues to use the legacy embeddable factory's render method.

    Since Canvas auto-applies all changes and doesn't have save functionality like Dashboard does, we must keep track of changes as they happen and update the expression to match the new Embeddable input - therefore, I had to add a onAnyStateChange callback to the ReactEmbeddableRenderer as a backdoor for Canvas. As a bonus to this, embeddables that previously didn't respect inline editing (such as the Image embeddable) will start to work once they are converted!

  2. It adds a new trigger (ADD_CANVAS_ELEMENT_TRIGGER) specifically for registering an embeddable to the Canvas add panel menu. This trigger can be attached to the same action that the Dashboard ADD_PANEL_TRIGGER is attached to - this makes it super simple to add React embeddables to Canvas:

    uiActions.registerAction<EmbeddableApiContext>({
      id: ADD_EMBEDDABLE_ACTION_ID,
      isCompatible: async ({ embeddable }) => { ... },
      execute: async ({ embeddable }) => { ... },
      getDisplayName: () => { ... },
    });
    
    // register this action to the Dashboard add panel menu:
    uiActions.attachAction('ADD_PANEL_TRIGGER', ADD_EMBEDDABLE_ACTION_ID);  
    
    // register this action to the Canvas add panel menu:
    uiActions.attachAction('ADD_CANVAS_ELEMENT_TRIGGER', ADD_EMBEDDABLE_ACTION_ID);

As a small cleanup, I also replaced some inline embeddable expressions with embeddableInputToExpression - this is because I was originally missing | render at the end of my expression, and I didn't catch it because the expressions I was comparing it to were all declared inline. This should help keep things more consistent.

How to Test

I attached the ADD_EUI_MARKDOWN_ACTION_ID action to the new ADD_CANVAS_ELEMENT_TRIGGER, so the new EUI Markdown React embeddable should show up in the Canvas add panel menu. Ensure that this React embeddable can be added to a workpad, and make sure it responds to edits as expected:

Screen.Recording.2024-04-01.at.3.56.04.PM.mov

Checklist

For maintainers

@Heenawter Heenawter added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// loe:small Small Level of Effort release_note:skip Skip the PR/issue when compiling release notes impact:critical This issue should be addressed immediately due to a critical level of impact on the product. Feature:Canvas labels Mar 28, 2024
@Heenawter Heenawter self-assigned this Mar 28, 2024
@Heenawter Heenawter force-pushed the fix-canvas-react-embeddables_2024-03-28 branch from c44c189 to 7e13d30 Compare April 1, 2024 18:51
@Heenawter
Copy link
Copy Markdown
Contributor Author

/ci

@Heenawter
Copy link
Copy Markdown
Contributor Author

/ci

@Heenawter Heenawter force-pushed the fix-canvas-react-embeddables_2024-03-28 branch 2 times, most recently from 68d8859 to da56fe0 Compare April 1, 2024 20:00
@Heenawter Heenawter force-pushed the fix-canvas-react-embeddables_2024-03-28 branch from da56fe0 to 0228319 Compare April 1, 2024 20:04
@Heenawter
Copy link
Copy Markdown
Contributor Author

/ci

@Heenawter
Copy link
Copy Markdown
Contributor Author

/ci

@Heenawter
Copy link
Copy Markdown
Contributor Author

/ci

@Heenawter
Copy link
Copy Markdown
Contributor Author

/ci

@Heenawter
Copy link
Copy Markdown
Contributor Author

/ci

@Heenawter
Copy link
Copy Markdown
Contributor Author

/ci

@Heenawter
Copy link
Copy Markdown
Contributor Author

/ci

Comment on lines +58 to +60
public readonly hasTrigger = (triggerId: string): boolean => {
return Boolean(this.triggers.get(triggerId));
};
Copy link
Copy Markdown
Contributor Author

@Heenawter Heenawter Apr 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than swallowing the error that is thrown when trying to attach an action to a trigger that doesn't exist, I wanted a cleaner way to ensure that the trigger exists before attaching the action - this is useful for preventing blocking errors from being thrown in situations where a trigger is known to be conditionally registered.

For example, the new Canvas add panel trigger is not registered in Serverless because the Canvas plugin does not exist in this version - so, before attaching an action to this trigger, we should verify that it exists like so: https://github.com/elastic/kibana/pull/179667/files#diff-65c205deed13f182061f156d76c8108c8b68a0d51d27100e36e22263ca087b3e

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice clean implementation of this.

@Heenawter
Copy link
Copy Markdown
Contributor Author

/ci

@Heenawter Heenawter marked this pull request as ready for review April 2, 2024 23:02
@Heenawter Heenawter requested review from a team as code owners April 2, 2024 23:02
@elasticmachine
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

@sebelga sebelga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@botelastic botelastic bot added the Feature:Embedding Embedding content via iFrame label Apr 3, 2024
@ThomThomson ThomThomson self-requested a review April 3, 2024 17:46
Copy link
Copy Markdown
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM! Ran this locally and went through the motions with legacy embeddable types (lens & maps) and with the EUI React Embeddable.

Everything runs well and works as expected (better than it used to even). NIce work!

Comment on lines +58 to +60
public readonly hasTrigger = (triggerId: string): boolean => {
return Boolean(this.triggers.get(triggerId));
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice clean implementation of this.

@Heenawter Heenawter enabled auto-merge (squash) April 3, 2024 20:10
@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
canvas 1213 1216 +3
dashboard 440 441 +1
dashboardEnhanced 66 67 +1
discover 775 776 +1
discoverEnhanced 38 39 +1
embeddable 111 112 +1
embeddableEnhanced 38 39 +1
inputControlVis 74 75 +1
lens 1379 1380 +1
links 127 128 +1
maps 1159 1160 +1
ml 1974 1975 +1
presentationPanel 91 92 +1
slo 669 670 +1
urlDrilldown 40 41 +1
visualizations 410 411 +1
total +18

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 43 40 -3
@kbn/presentation-publishing 139 146 +7
embeddable 452 453 +1
uiActions 101 103 +2
total +7

Async chunks

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

id before after diff
canvas 1015.0KB 1017.5KB +2.4KB
dashboard 389.9KB 389.9KB +54.0B
total +2.5KB

Page load bundle

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

id before after diff
canvas 13.5KB 14.1KB +597.0B
dashboardEnhanced 15.4KB 15.4KB +8.0B
embeddable 65.6KB 65.9KB +251.0B
lens 46.4KB 46.4KB +60.0B
presentationPanel 42.5KB 42.6KB +54.0B
uiActions 20.3KB 20.4KB +58.0B
total +1.0KB
Unknown metric groups

API count

id before after diff
@kbn/presentation-containers 44 41 -3
@kbn/presentation-publishing 166 175 +9
embeddable 557 559 +2
uiActions 147 149 +2
total +10

History

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

cc @Heenawter

@Heenawter Heenawter merged commit 4cc61b9 into elastic:main Apr 3, 2024
@Heenawter Heenawter deleted the fix-canvas-react-embeddables_2024-03-28 branch April 3, 2024 22:29
@kibanamachine kibanamachine added v8.14.0 backport:skip This PR does not require backporting labels Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Canvas Feature:Embedding Embedding content via iFrame impact:critical This issue should be addressed immediately due to a critical level of impact on the product. loe:medium Medium Level of Effort project:embeddableRebuild release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v8.14.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Embeddable Rebuild] React-based embeddables broken on Canvas

6 participants