[Embeddable] Add unified error UI

[dokmic]

Summary

Resolves #141676.

Before:

After:

Checklist

• Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
• Unit or functional tests were updated or added to match the most common scenarios
• 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

[elasticmachine]

Pinging @elastic/kibana-app-services (Team:AppServicesUx)

[MichaelMarcialis]

Thanks so much for making these changes, @dokmic! Leaving you a few small comments below for your review.

[MichaelMarcialis]

Would it make sense to also include a title prop (along with an appropriate titleSize) for this EuiEmptyPrompt component (either dynamic or static)? If a static title, would something like Unable to render visualization work?

[dokmic]

I tried to add that similarly to the button, but it requires significant changes in the code to get the embeddable type. I would rather leave that without the title prop. Especially since the error appearance is similar to what we had before.

[Heenawter]

Code review: Primarily looked at embeddable and dashboard_error_handling.ts changes, but did briefly look through Lens/Visualize changes as well - left one tiny nit, but otherwise looks good!

Tested locally:

1. Seems to be some clipping when the error is present on a super small panel:

   

2. Visualization panels should probably show the Edit button for a missing dataview error - right now, it only seems to work for when fields are missing:

   

[drewdaemon]

I tested missing data view (thanks @Heenawter ) and an error thrown from within the expression chain. Looks good. Code LGTM on the vis editors side.

[dokmic]

@MichaelMarcialis @Heenawter I've updated the PR according to your feedback -- all the findings are fixed now. Could you please take another look?

[Heenawter]

@dokmic Taking another look at this :)

1. Seems like the "No data view" error embeddable no longer includes a link to edit the visualization for Lens panels.

   

   Also, for Visualize panels, it looks like... the entire panel is now clickable, even though there is minimal indication of this besides the cursor change? I wonder if we could still limit the button to the "Edit in <X> editor to fix the error" text, while making it more clear that the text is clickable - for example, making the text blue/underlined. @MichaelMarcialis I know you recommended changes to the button appearance, so diverting to your opinion here :)

   

2. The behaviour between panel types is very different when the index is deleted (but the data view remains) - for example, some throw an error whilst others simply report no data. Is this to be expected?

   Screen.Recording.2022-10-28.at.4.32.17.PM.mov

3. Looks like both the Visualize and Maps "No data view" errors are still clipping:

   

4. Maps doesn't seem to have a call to action, despite the panel being clickable:

   

5. Not sure if there are plans to add this error UI to TSVB/Vega visualizations as well? The panel on the left is Aggregation based, whereas the panel on the right is TSVB.

   

   And is what it looks like for a Vega visualization:

   

   However, from the user's perspective. all three of these panels are both considered "Visualize" panels so it's a bit confusing that there are three different ways of displaying the error.

[dokmic]

@Heenawter Thanks for such an extensive review! Those cases are coming deep from the Lens and Visualizations code. I've opened #144224 to cover them, which should be done separately by @elastic/kibana-vis-editors as they know this area the best.

[vadimkibana]

Code changes LGTM

[MichaelMarcialis]

+1 to @Heenawter's comments mentioned above. I understand the desire to want to split off some of these as separate issues/PRs, but I think comments 1, 3, and 4 should be addressed here before this PR is merged. Specifically in regard to comment 1, my recommendation is that the entire panel should not be clickable. Instead, only a button within the panel should allow clicking to the edit action. I just wanted the previous EuiButton component switched to an EuiButtonEmpty component.

[dokmic]

@MichaelMarcialis @Heenawter Those are not even errors for the embeddable component. They come from the expressions invoked inside the visualizations. Changing the code to throw an error breaks some other places, and fixing that as part of this requires more effort than with the original PR.
Those edge cases should be covered by the owning team, as they are capable of testing the entire error handling in those plugins.

[Heenawter]

@dokmic Sorry, I guess I'm a bit confused :)

In my original review, I recall that

1. the Lens panel had a clickable link to edit it and
2. in Visualize panels, it was only the button that was clickable and not the entire panel like it is currently

In my second review, I was simply noting that this behaviour was different than what I remember during my original review - the original behaviour was preferable to me and hence why I brought it up (although I am obviously open to discussion on whether or not these changes were made for a reason that has not yet been mentioned here). Considering these changes have occurred between my first and second review, I think it still makes sense to fix this behaviour in this PR, no? Unless these changes are somehow unrelated to your PR - please, let me know if I'm misunderstanding!

I understand that some of the things noted in my second review are edge cases, and I'm 100% fine with these being tackled in a follow up by the respective teams. Just would appreciate some clarification. Sorry about holding this up!

[dokmic]

@Heenawter Thanks for clarifying this. After trying to reproduce that, I realized that the problems you discovered are relevant to the main branch. It seems like you haven't rebuilt Kibana before the second attempt. Could you please try once more?

1. Seems like the "No data view" error embeddable no longer includes a link to edit the visualization for Lens panels.

Should be resolved once rebuilt.

2. The behaviour between panel types is very different when the index is deleted (but the data view remains) - for example, some throw an error whilst others simply report no data. Is this to be expected?

Works as expected.

3. Looks like both the Visualize and Maps "No data view" errors are still clipping:

Should be fixed once rebuilt.

4. Maps doesn't seem to have a call to action, despite the panel being clickable:

Works as expected.

5. Not sure if there are plans to add this error UI to TSVB/Vega visualizations as well? The panel on the left is Aggregation based, whereas the panel on the right is TSVB.

TSVB and Vega should be refactored separately by the relevant team as it is not super straightforward how the error is encapsulated there.

In the meantime, I've fixed the missing data view error for the visualizations. Now, it is aligned with the rest.

cc @MichaelMarcialis

[Heenawter]

Thank you so much for clarifying + fixing where necessary! I did rebuild Kibana before my second review, but I didn't restart my Elasticsearch client and so I forgot to remove + re-import the sample data so that was probably part of the problem. Sorry about that! 🙇

Tested again and everything worked great (minus the stuff that will be fixed in the follow up) 🎉

[MichaelMarcialis]

Thanks for making these changes, @dokmic! LGTM!

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 4da17ef

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
embeddable	79	80	+1

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
visualizations	768	766	-2

Async chunks

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

id	before	after	diff
lens	1.3MB	1.3MB	+509.0B
visualizations	246.6KB	246.3KB	-301.0B
total			+208.0B

Page load bundle

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

id	before	after	diff
embeddable	68.9KB	69.1KB	+156.0B

Unknown metric groups

API count

id	before	after	diff
visualizations	798	796	-2

ESLint disabled in files

id	before	after	diff
osquery	1	2	+1

ESLint disabled line counts

id	before	after	diff
enterpriseSearch	19	21	+2
fleet	58	64	+6
osquery	108	113	+5
securitySolution	440	446	+6
total			+19

Total ESLint disabled count

id	before	after	diff
enterpriseSearch	20	22	+2
fleet	66	72	+6
osquery	109	115	+6
securitySolution	517	523	+6
total			+20

History

• 💔 Build #85433 failed 86bea8f
• 💔 Build #85295 failed 370cbee
• 💚 Build #83896 succeeded b22997e
• 💔 Build #83696 failed e8024c9
• 💛 Build #83604 was flaky fcb640b

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