[Lens] Enable actions on Lens Embeddable

[dej611]

Summary

Fixes #93941

First MVP for embeddable actions.

• Refactor ExpressionRenderer main code as a useEmbeddableFactory hook
  • Internal refactor to use the hook
• EmbeddablePanel tweak to load from the Lens embeddable
  • Make "Universal" actions optional, and split the inspector action
  • Make actions filterable from the outside
• Expose an withActions prop from the Lens embeddable
  • Make the withActions work as filterable list removed
• Show an example of the new prop
• 🐛 Make time range action check parent existence
• Clean up the duplicate export_csv_action
  • Refactor plugin dependencies
• Write tests at different levels

Hovering top right corner

No hovering

Show all actions:

Inspector panel working:

Initial version

Show all actions:

Inspector panel working:

Few thoughts

• It would be nice to expose some way to filter (in/out) actions to show in the context menu: this requires the knowledge of the action ids which are not always exposed (i.e. csv download or inspector ).
  • Do we want this on the initial implementation?
  • In positive case, what's the best route to pursue? cc @stacey-gammon
• Didn't find a nice way to test specifically the Lens Embeddable here - I made an experiment but had to mock too many things. Is there's a best practice/example for that?
• The universalActions in the EmbeddablePanel have been split into two partitions: panel CRUD vs inspector groups. Is that ok or another grouping makes more sense in this context?
  • Given the re-use of the EmbeddablePanel out of dashboard now, does it still make sense to have the panel CRUD actions as universal with no validation? cc @stacey-gammon

Checklist

Delete any items that are not applicable to this PR.

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Documentation was added for features that require explanation or tutorials
• Unit or functional tests were updated or added to match the most common scenarios
• Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
• Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
• If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
• This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
• This was checked for cross-browser compatibility

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[dej611]

Updated the initial PR message. Added few questions for @stacey-gammon

[elasticmachine]

Pinging @elastic/kibana-app (Team:KibanaApp)

[spalger]

jenkins, test this

(restarting due to jenkins upgrade)

[flash1293]

Thanks for working on this @dej611

This looks pretty good, but it seems like it's pulling in too much:

• Badge with current time range - either hide or make configurable on Lens level
• dashed border and drop shadow - I guess it makes sense to just hide, if a user wants that they can add it manually
• cogwheel icon instead of three dots - I think three dots are a better option in this case

[dej611]

• Badge with current time range - either hide or make configurable on Lens level

Ok, I'll remove the badges.

• dashed border and drop shadow - I guess it makes sense to just hide, if a user wants that they can add it manually
• cogwheel icon instead of three dots - I think three dots are a better option in this case

I think these depend on the ViewMode set on the Embeddable inputs.

[flash1293]

I think these depend on the ViewMode set on the Embeddable inputs.

Is this separate from the renderMode we use for showing controls in the vis itself? If yes, then please change the view mode in the example. If no, I think we have to find another solution.

[dej611]

I think these depend on the ViewMode set on the Embeddable inputs.

Is this separate from the renderMode we use for showing controls in the vis itself? If yes, then please change the view mode in the example. If no, I think we have to find another solution.

Yes, changed from the example side.

• Badge with current time range - either hide or make configurable on Lens level

Badges have to be controlled by specific type. As for now the custom time one has been disabled from the Lens side, after founding other plugins are using a similar approach, but the consumer can always override it.

Will update the initial message with newer screenshots

[flash1293]

@elasticmachine merge upstream

[flash1293]

LGTM, great addition!

[streamich]

In the example plugin:

1. I'm not sure if I understand what "Change color" should do, but clicking it does not seem to have an effect, chart is still green.
2. "Edit in Lens" button opens a new tab, not sure if that is intended (I thought it would use single page app navigation).

[streamich]

Thanks, LGTM, tested on Mac/Chrome, all seems to be working.

What is the rationale for disabling by default time range action? Is it a breaking change?

[streamich]

nit: we could call it actionPredicate.

[streamich]

Why do we want to hide it by default? Is it a breaking change?

Suggested change

	// by default hide the time badges. Consumers can always override via disabledActions prop
	const input = { disabledActions: ['CUSTOM_TIME_RANGE_BADGE'], ...props };
	const input = { disabledActions: ['CUSTOM_TIME_RANGE_BADGE'], ...props };

[dej611]

Yes, probably this way it sounds like a breaking change. Maybe I could prevent the badge/notifications rendering via a special prop on the EmbeddablePanel itself

[streamich]

Maybe not for this PR, but I'm thinking we are implicitly overloading what <EmbeddableRoot> should do.

Maybe clearer would be something like this:

Suggested change

	if (!hasActions || loading || error || !embeddable) {
	return (
	<EmbeddableRoot embeddable={embeddable} loading={loading} error={error} input={input} />
	);
	}
	if (loading) {
	return (
	<EmbeddableSpinner />
	);
	}
	if (error) {
	return (
	<EmbeddableError error={error} />
	);
	}
	if (!hasActions || !embeddable) {
	return (
	<EmbeddableRoot embeddable={embeddable} loading={loading} error={error} input={input} />
	);
	}

[dej611]

But the EmbeddableRoot contains already some logic for these kind of states.
Maybe a nicer way to render this could be something like:

if(embeddable && hasActions){
  return <EmbeddablePanel .../>
}
return <EmbeddableRoot embeddable={embeddable} loading={loading} error={error} input={input} />

Let checks and same result

[streamich]

But the EmbeddableRoot contains already some logic for these kind of states.

That is my point as well, I am saying maybe it should not contain those.

1. It might be useful for <EmbeddableRoot> component itself, if those special cases are separate.
2. Those would be useful for <EmbeddablePanel> component, as now <EmbeddablePanel> relies on <EmbeddableRoot> to render, for example, the error state.

[flash1293]

@elasticmachine merge upstream

[dej611]

Updated the PR with latest fixes:

• filterActions => actionPredicate
• Handle badge and notifications at configuration level
• Fix EmbeddablePanel input update

[wylieconlon]

LGTM

[flash1293]

@elasticmachine merge upstream

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: f44a0d8
• Storybooks Preview

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
embeddable	381	383	+2

Page load bundle

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

id	before	after	diff
embeddable	180.3KB	181.5KB	+1.3KB
lens	40.0KB	40.8KB	+777.0B
uiActionsEnhanced	231.7KB	231.7KB	+19.0B
total			+2.0KB

Unknown metric groups

API count

id	before	after	diff
embeddable	451	453	+2

History

• 💚 Build #133061 succeeded af200a7
• 💚 Build #133019 succeeded ab7ed1a
• 💚 Build #132552 succeeded 532b838
• 💚 Build #132457 succeeded e1444b4
• 💔 Build #132438 failed 768e774

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

[streamich]

LGTM

[kibanamachine]

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.