elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.76k stars 8.16k forks source link

[Embeddables Rebuild] Clean up `RenderCompleteDispatcher` and related attributes from all embeddables #179376

Open Heenawter opened 6 months ago

Heenawter commented 6 months ago

Describe the feature: Currently, for all legacy embeddables, we are ending up with duplicated data-render-complete attributes - (1) from the embeddable itself (due to the use of RenderCompleteDispatcher and/or setting the props manually somewhere in the embeddable code) and (2) from the wrapping PresentationPanel component:

Screenshot 2024-03-25 at 11 38 15 AM

Once every embeddable has been converted to the new React system, PresentationPanel should be entirely responsible for handling the data-render-complete, data-rendering-count, data-loading, data-rendering-error, and data-error attributes. As part of this work, we will need to check that no embeddables are continuing to use RenderCompleteDispatcher and/or setting these attributes manually - and, we can hopefully remove RenderCompleteDispatcher entirely once we are done.

Open questions: I'm not entirely sure what we want to do with the other data-* attributes - for example,data-title, data-shared-item, data-shared-items-container, ... etc. We should dig into exactly what these attributes are used for (if they are used at all) and try to centralize where these things are being set. Perhaps PresentationPanel, perhaps somewhere else.

elasticmachine commented 6 months ago

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

Heenawter commented 6 months ago

Marking this as blocked, since it is too high risk to do this until all embeddables have been converted.

mgiota commented 5 months ago

@Heenawter Regarding data-shared-item, it is needed for Kibana reporting screenshot tool, otherwise it gives a timeout error. I faced the timeout error in this issue and it turned out that solution was to add the data-shared-item, so I guess we need it in the new Embeddable framework as well. I added it as part of this PR.

I don't know more details about rest attributes, just wanted to share about data-shared-item. I must admit that it is not straightforward what this attribute is doing. But I guess part of the current issue you created, is to figure out what all these attributes are used for and possibly refactor.

Zacqary commented 1 month ago

Visualize embeddables need to dispatch a renderComplete event as well as setting data-render-complete in order to properly signal all the systems they need to interact with.

Not sure how many other embeddable types need to do this. If it's a wider issue we should also make sure they're dispatching this event on the framework level instead of having to make each embeddable type do it.