Use renderComplete handler in maps embeddable

[lizozom]

Summary

Use renderComplete handler in maps embeddable to correctly await maps loading.

Similar to #144243

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

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

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

[nreese]

@dmlemeshko is the ecommerce dashboard used in x-pack/performance/journeys/ecommerce_dashboard.ts the same as ecommerce sample data dashboard? Because the ecommerce sample data dashboard does have a map when Maps application is running.

[nreese]

@tsullivan Do components still need to set data-render-complete or is data-render-complete no longer needed when using embeddable.renderComplete?

[nreese]

cc @dokmic Do you have any insight into the #144421 (comment)?

[tsullivan]

@tsullivan Do components still need to set data-render-complete or is data-render-complete no longer needed when using embeddable.renderComplete?

You no longer need to set the data-render-complete attribute yourself. embeddable.renderComplete is a RenderCompleteDispatcher, and handles the setAttribute calls for you, to set the data attributes that are needed for reporting & performance tooling. For more info, check out #74504

[lizozom]

@nreese the map loading was taking much longer than the rest of the visualizations, so we moved them to a standalone test called ecommerce_dashboard_map_only.

And yes, no need to set data-render-complete yourself, but removing it caused several functional tests to fail, so I decided there was not much harm in keeping both for now. Feel free to remove the internal one if you like.

[dmlemeshko]

@dmlemeshko is the ecommerce dashboard used in x-pack/performance/journeys/ecommerce_dashboard.ts the same as ecommerce sample data dashboard? Because the ecommerce sample data dashboard does have a map when Maps application is running.

@nreese yes, that is the one. We decided to move map viz away from original ecommerce dashboard in its own journey (single viz on dashboard) and track performance separately.

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 168e3a3

Metrics [docs]

Async chunks

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

id	before	after	diff
maps	2.7MB	2.7MB	+170.0B

Unknown metric groups

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	106	111	+5
securitySolution	440	446	+6
total			+19

Total ESLint disabled count

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

History

• 💔 Build #85129 failed 34c5bc3
• 💚 Build #84811 succeeded 80cbe39
• 💔 Build #84750 failed 5912d70

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

[nreese]

Thanks for putting this PR together. While I agree that MapEmbeddable needs to be updated to use this.renderComplete, I also believe we need to clean up existing uses of data- attributes to avoid confusion and overlapping functionality.

For example, in the screen shot below, these 2 elements now contain duplicated data-* attributes.

This can be cleaned up by removing setting data- attributes in MapContainer

This PR also introduces different behavior for when data-render-complete is set to true. MapContainer implementation uses a timeout to fire data-render-complete after all layers have loaded to account for rendering time that is not possible to measure with a map. this.renderComplete implementation should include this timeout as well so data-render-complete is not set to true until a set amount of time. The existing 5000 timeout is likely excessive and maybe we can trim this to something smaller like 1000.

Another thing to consider is that MapEmbeddable may exist within other embeddables. One example is legacy tile_map and region_map visualizations. Those visualizations are now implemented as wrappers around MapEmbeddable so you have something like the below

  <VisualizeEmbeddable>
    <MapEmbeddable>
    </MapEmbeddable>
  </VisualizeEmbeddable>

This problem can be solved by only calling this.renderComplete when this._isSharable is true.

Can you provide some background into why this change is being made? Is map embeddable not functioning properly now? Is the existing behavior of data-render-complete incorrect. I think understanding the why will help answer some implementation questions.

Also, if this is something that is too involved, the Maps team can take over the effort.