Kibana app migration: Shim dashboard#48913
Conversation
💚 Build Succeeded |
kertal
left a comment
There was a problem hiding this comment.
Code LGTM 👍 , tested locally comparing side by side the non-shimmed version, and since I merged it into my Discover PR, this was also tested this way. Just one note, the icon directive seems to be missing
Just add .directive('icon', reactDirective => reactDirective(EuiIcon)) to your inner angular directive to make it work
lizozom
left a comment
There was a problem hiding this comment.
In terms of @elastic/kibana-app-arch services, LGTM
I will follow up with a PR deprecating filter-bar directives.
|
Good catch @kertal ! |
💚 Build Succeeded |
ppisljar
left a comment
There was a problem hiding this comment.
i think you shouldn't be exporting any legacy angular stuff from np_ready plugins. we plan to remove that soon, and if you depend on any of those items (as you plan to keep angular past 8.0) you should copy the relevant code inside your plugin.
|
@ppisljar PR #50661 will create a So instead of copying those code snippets into each separate plugin, we plan to just move them into this plugin. I just didn't do this as part of this PR because it is touching enough places already. |
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
|
Jenkins, test this. |
💚 Build Succeeded |
|
🎉 !!!! |

Shims dashboard app and uses local angular instance instead of references to global angular to be able to move this to the new platform without getting rid of angular first.
There are quite some changes outside of the dashboard app itself in here, so I'll try to put together a guide for how to review this:
uiModuleswhich we can't use in our local angular. This PR exports the raw direcrtive definitions so they can get registered in a local angular module as well.requireDefaultIndexmagic flag on the route got removed because this requires uiRoutes we won't carry over (not used in many places anyway). The replacement is a function that has to be called in the onlyresolveof a route, if the returned promise resolves the actual data can be fetched. That slightly changes the code structure in discover because it was fetching the index pattern and the saved search in two separateresolveprops.requireDefaultIndexhelper function.ui/capabilities/route_setup(KibanaApp/AppArch?): Got moved tosrc/legacy/ui/public/legacy_compat/angular_config.tsxbecause it's currently also using theaddSetupWorkhook.legacy_compatangular stuff (Platform?):configureAppAngularModulewas using NP dependencies directly, to be able to use the same function also in the new platform this was changed to take them via parameter - also be adjusted in legacy chromesrc/legacy/ui/public/chrome/api/angular.jssrc/legacy/ui/public/timefilter/setup_router.ts(AppArch?): Expose the logic to wire the global state instance with the NP data plugintimefilterstate so it's usable in the local (and make sure it's also unsubscribing correctly)navigationdep through to the angular directive creation.Dev docs
The route flag
requireDefaultIndexmaking sure there are index patterns and thedefaultIndexadvanced setting is set was removed.The same functionality can be achieved by using the helper function
ensureDefaultIndexPatternfromui/legacy_compatwithin theresolveobject of a route.