Navbar Extensions in KbnTopNav with Config controls #7508
Conversation
remove navbar-extensions directive from top nav markup
| const KbnTopNavController = Private(KbnTopNavControllerProvider); | ||
| const navbarExtensions = Private(RegistryNavbarExtensionsProvider); | ||
| const getNavbarExtensions = _.memoize(function (name) { |
There was a problem hiding this comment.
This should be moved outside of this controller function for the memoize to work correctly, no? Otherwise a new instance of these is made every time you use this directive.
If you move this you should also move the navbarExtensions along with it since this is the only place it's used.
|
@w33ble Man this is sexy, this is how it should've been done the first time. |
|
@panda01 well, it wouldn't have been possible without your work consolidating the top nav stuff ;) I'll go make those changes, thanks for taking a look. |
|
|
||
| // apply the defaults to individual options | ||
| _applyOptDefault(opt = {}) { | ||
| return defaults({}, opt, { | ||
| label: capitalize(opt.key), | ||
| label: opt.label || capitalize(opt.key), |
There was a problem hiding this comment.
I should test this, but this shouldn't be necessary, if you set opt.label on the raw object due to _.defaults. docs
|
Smooth @w33ble, other than the mentions, this looks phenomenal. Once those are in, I'll give it the acronym of approval. |
|
Also, we can close #7443 in exchange for this one, no? |
now that we no longer need to inject values from the code
makes the include happen once, and the memoize actually work as expected
|
Changes are in, waiting on the tests. #7443 is for the backport to 4.x; same functionality, different implementation since there's no kbnTopNav there. |
|
LGTM! |
Navbar Extensions in KbnTopNav with Config controls Former-commit-id: b583bf6
This PR changes the way Navbar Extensions work. It does away with the special directive and instead just loads the extensions out of the registry in the
kbnTopNavdirective directly. This simplifies how the menu is extended, and also allows the extensions to provide a Config template with application-specific controls.ui/registry/navbar_extensionsinkbnTopNavkbnTopNavmethods for claritylabelparameter, instead of only relying on thekeynavbarExtensionsdirective