[Maps] Map embeddable #31473
[Maps] Map embeddable #31473
Conversation
|
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
|
jenkins, test this |
💔 Build Failed |
💚 Build Succeeded |
💔 Build Failed |
💚 Build Succeeded |
💔 Build Failed |
💚 Build Succeeded |
💚 Build Succeeded |
💚 Build Succeeded |
💚 Build Succeeded |
💚 Build Succeeded |
💚 Build Succeeded |
There was a problem hiding this comment.
This is real cool. Seeing the filters applied 🤤
Mostly cosmetic feedback.
There seems to be a bug, and we should fix this here.
window resizing doesn't 100% work
To reproduce:
- open a dashboard, make sure the browser window only takes about half your screen
- maximize the browser window
--> the widgets on the dashboards stretch, but the mapbox-map stays in it's original size
x-pack/plugins/maps/public/angular/services/gis_map_saved_object_loader.js
Show resolved
Hide resolved
x-pack/plugins/maps/public/embeddable/map_embeddable_factory.js
Outdated
Show resolved
Hide resolved
| * @param {HTMLElement} domNode | ||
| * @param {ContainerState} containerState | ||
| */ | ||
| render(domNode, containerState) { |
There was a problem hiding this comment.
not sure if we should monitor this domNode for a resize (see other comment).
…eload and refresh timer
|
@thomasneirynck The resize bug is resolved now. |
| if (lastWidth === window.innerWidth | ||
| && lastHeight === window.innerHeight && this._mbMap) { | ||
| this._mbMap.resize(); | ||
| this._checker.on('resize', () => { |
There was a problem hiding this comment.
nice, this is more clean too.
I don't think we need to wrap this in an animation-frame callback. It assumes we'd get resize events at a rate exceeding 60fps, and that mapbox would not be optimizing for map.resize() method calls internally. It'd suprise me if both were true. I think we should be fine just calling resize on each event.
There was a problem hiding this comment.
@thomasneirynck I have removed the animation frame. Looks like EUI has a ResizeObserver that we could use instead of ui/public/resize_checker. The problem is that ResizeObserver was only added in EUI 7.2.0 and Kibana is currently on EUI 7.1.0. I will make a separate PR to clean this up more and replace resize_checker (which is based on angular) with ResizeObserver once Kibana EUI has been upgraded.
There was a problem hiding this comment.
Or, I can just revert the changes to this file and the resize issue can be completely addressed in a separate PR since resize_checker is going to be replaced anyways.
There was a problem hiding this comment.
it's good to fix it here imho
OK with new PR where we migrate to eui's ResizeObserver later
There was a problem hiding this comment.
There was some reason we needed this previously but I'm comfortable with getting rid of it and letting it re-emerge if necessary. Really might not be needed anymore.
💚 Build Succeeded |
💚 Build Succeeded |
This PR registers
mapEmbeddableFactoryProviderwithEmbeddableFactoriesRegistryProviderallowing maps to be embedded any where embeddables are supportedSince there is no way to add map embeddables to a dashboard through the UI, testing is a little difficult. The best way is to run the functional tests and stop them once the ES archives have been loaded. Then open the dashboard "map embeddable example" and manually experiment with that dashboard.