Skip to content

Replace legacy MapEmbeddable with MapComponent in Obs UX solution#182841

Merged
nickpeihl merged 3 commits intoelastic:mainfrom
nickpeihl:decouple-obs-ux-mapembeddable
May 9, 2024
Merged

Replace legacy MapEmbeddable with MapComponent in Obs UX solution#182841
nickpeihl merged 3 commits intoelastic:mainfrom
nickpeihl:decouple-obs-ux-mapembeddable

Conversation

@nickpeihl
Copy link
Copy Markdown
Contributor

Summary

Replaces the Maps embeddable with a new MapComponent in the Observability UX solution.

The Maps plugin now provides an easier to use MapComponent for consumers. The legacy MapEmbeddable factory is also being removed as part of #174960 which requires making changes to consumers.

For maintainers

@nickpeihl nickpeihl added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. project:embeddableRebuild v8.15.0 labels May 7, 2024
@nickpeihl nickpeihl requested a review from a team May 7, 2024 14:39
@nickpeihl nickpeihl requested a review from a team as a code owner May 7, 2024 14:39
@elasticmachine
Copy link
Copy Markdown
Contributor

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

@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

isLayerTOCOpen?: boolean;
mapCenter?: MapCenterAndZoom;
onInitialRenderComplete?: () => void;
renderTooltipContent?: RenderToolTipContent;
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.

Lets call this getTooltipRenderer. Its also added in #182737. The reason for get is that its value is only used once and not updated when the value changes. If its just renderTooltipContent then users of the Component would expect that changing the prop would be reflected in the component.

setLayerList: jest.fn(),
}),
}));
const mockMapsApi = {
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.

Same comment as in your other PR, lets rename as mockMapsStartApi or mockMapsStartService to avoid naming confusion with MapApi type

Copy link
Copy Markdown
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

LGTM
code review only

@kibana-ci
Copy link
Copy Markdown

kibana-ci commented May 9, 2024

💚 Build Succeeded

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 3.0MB 3.0MB +324.0B
ux 168.4KB 167.6KB -879.0B
total -555.0B

Canvas Sharable Runtime

The Canvas "shareable runtime" is an bundle produced to enable running Canvas workpads outside of Kibana. This bundle is included in third-party webpages that embed canvas and therefor should be as slim as possible.

id before after diff
module count - 5407 +5407
total size - 8.8MB +8.8MB
Unknown metric groups

ESLint disabled line counts

id before after diff
ux 12 9 -3

Total ESLint disabled count

id before after diff
ux 15 12 -3

History

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

Copy link
Copy Markdown
Contributor

@cauemarcondes cauemarcondes left a comment

Choose a reason for hiding this comment

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

LGTM

from: new Date(start).toISOString(),
to: new Date(end).toISOString(),
};
embeddable.updateInput({ timeRange });
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.

So there's no need to update the input anymore?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nope. The maps.Map component updates the embeddable input from the props that are passed into it.

https://github.com/elastic/kibana/pull/182841/files#diff-e9f828ecb480fd2220e2b823f7354bcd774467105b23267eb839a5a2490adcf4R71-R88

ux-map4.mp4

@nickpeihl nickpeihl merged commit 49a5d5e into elastic:main May 9, 2024
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label May 9, 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 ci:project-deploy-observability Create an Observability project project:embeddableRebuild release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-infra_services - DEPRECATED DEPRECATED - Use Team:obs-presentation. Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v8.15.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants