Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace kbn_top_nav with a new React component #39981

Closed
11 tasks done
lizozom opened this issue Jun 30, 2019 · 6 comments · Fixed by #43168
Closed
11 tasks done

Replace kbn_top_nav with a new React component #39981

lizozom opened this issue Jun 30, 2019 · 6 comments · Fixed by #43168

Comments

@lizozom
Copy link
Contributor

lizozom commented Jun 30, 2019

Replace the kbn_top_nav angular component with a react component, in a manner that's as backwards compatible as possible, but still allow apps to use the new class directly as they move to React.

Apps using kbn-top-nav

Cleanup

Configuration

kbn_top_nav receives it's configuration via the config attribute. At the moment, this format won't change, to increase backwards compatibility. The only item that will be deprecated is template.

This is an example of a menu item configuration:

[{
      id: 'new',
      label: 'new'
      description: 'New Search',
      run: function () { kbnUrl.change('/discover'); },
      testId: 'discoverNewButton',
}, 
...
];

Currently kbn_top_nav supports the following options:

  • id (key is still supported on the directive, but not in the React component)
  • label
  • description
  • run
  • template - to be deprecated
  • testId
  • disableButton - a value or a function, which if truthy, disables the menu item.
  • tooltip - Is it needed?

Template deprecation

The new TopNavMenu won't be responsible for rendering the actions. Rather, those will be rendered by the run method, and convenience methods (showPanel, showModal) may be used.

Therefore, any existing menus that provide a template for kbn_top_nav to render, need to be migrated to use run functions instead.

Transcluded items

Previously, top nav was extended (most frequently with search bar \ query bar \ filter bar \ date picker) by transcluding those directives inside the kbn-top-nav directive.

Moving to React, the new TopNavMenu won't support adding custom views into it.
Instead, it supports rendering search bar \ query bar \ filter bar \ date picker, by passing their options directly to TopNavMenu (or to the reactDirective, in case of Angular code).

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch

@peteharverson
Copy link
Contributor

Note that work is already underway in ML to replace the Angular based top nav with a React component - see #39325.

@lukeelmers
Copy link
Member

@lizozom I think this was closed by #40262, but feel free to re-open if that's not the case.

@lizozom
Copy link
Contributor Author

lizozom commented Jul 24, 2019

@lukeelmers The issue is actually not closed yet.

We still need to do the following tasks:

I'll keep this issue open to track these tasks.

@lizozom lizozom reopened this Jul 24, 2019
@lizozom lizozom added the v7.4.0 label Jul 24, 2019
@flash1293
Copy link
Contributor

@lizozom Is there a hard time limit prior to 8.0 for this? I probably won't get around to do this for a few more months.

@lizozom
Copy link
Contributor Author

lizozom commented Jul 24, 2019

@flash1293 I would really like to push this in 7.4
If we could do a quick session and review some of the code, I might be able to do it myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants