[Global header] Create API to register application menu links

[ryankeairns]

Describe the feature:
As part of the new global header, application menus will be relocated from within the page to the right end of the lower header bar as seen below:

Describe a specific use case for the feature:
As a plugin author/product team, I should be able to add my application menu to the header from my plugin using a framework-independent API.

This new header API should support existing menu designs by accommodating the following content:

• Dynamic menus that change based upon state (e.g. read vs edit modes)
• Mixed menus that display both links and buttons
• Links and buttons that display popover menus (e.g. a Share menu)

cc:/ @joshdover @myasonik @alexfrancoeur

[joshdover]

From a plugin perspective, I think we should provide this API via a new HTMLElement parameter exposed to the mount function that is called when an application is mounted. This allows for a consistent API and enables plugins to use React portals for mounting part of their app tree within this element.

Specifically, I think a headerActionsElement or something similar should be added to the AppMountParameters type, providing an API like:

core.application.register({
  id: 'myApp',
  mount({ element, headerActionsElement }) {
    ReactDOM.render(
      <MyApp headerActionsElement={headerActionsElement} />,
      element
    );

    return () => { ReactDOM.unmountComponentAtNode(element) };
  }
);

const MyApp = ({ headerActionsElement }) => {
  // Simple example, probably your whole app would not be these buttons :)
  return ReactDOM.createPortal(
    <HeaderActions />,
    headerActionsElement
  );
};

I think the tricky part will be actually implementing this API. As it is right now, the Chrome UI is separate from the Application UI code, though they are mounted within the same React tree. Most likely we will need to pass a React ref to both the Chrome component and the app router component so that the HTMLElement can be accessed by the app's mount function.

There could be some timing issues here as well that we may need to workaround to ensure that the headerActionsElement and main app element are both "fresh" before we render the next application. By "fresh" I mean they are new div elements that are not shared with the previous app to minimize any issues from apps not cleaning up properly.

[elasticmachine]

Pinging @elastic/kibana-core-ui (Team:Core UI)

[joshdover]

This new header API should support existing menu designs by accommodating the following content:

* Dynamic menus that change based upon state (e.g. read vs edit modes)

* Mixed menus that display both links and buttons

* Links and buttons that display popover menus (e.g. a Share menu)

How many of these features do we want to implement in the Core API vs. allowing applications to do this themselves (optionally, with some helper components)?

My previous comment was made with the assumption that we would allow applications to render anything they want in that space, but if we want to enforce consistency in how that UI looks then we may want to use a different approach. If we do want to enforce/encourage consistency we can either have a more constrained API that provides these features, or we could provide helper React components that provide these features.

More constrained API

• Pros:
  • Ensures that all UI implementations have consistent styling, layout, and content
• Cons:
  • Makes implementing an MVP more work
  • Makes the Core API more complex
  • Makes this space less flexible
  • May be an awkward API if it's not React-based (which no Core APIs are)

Helper React components

• Pros:
  • More natural for applications to integrate with
  • Can provide the same consistency
  • Allows plugins to use this space for something that does not follow the out-of-the-box UI pattern
• Cons:
  • Less obvious / discoverable
  • May lead to inconsistency if applications don't use the helpers

I lean heavily towards "helper react components" option, but wanted to gut check this with the UI developers. Additionally, should we consider these helper components as in-scope for this issue (and worked on by the Platform team) or would the Core UI team prefer to implement these components?

[ryankeairns]

@joshdover I completely agree with option 2 and these can be considered out of scope for the initial version. My main objective in pointing out those use cases was simply that we keep this open-ended enough that we don't preclude teams from essentially keeping the menu they currently have.

As for the helpers, I think we're safe to punt on that for now since a) it's no different than what we allow today b) this entire effort seems to only impact apps under the Kibana grouping (as of now; 3rd party plugins aside) and c) our biggest blocker to this effort is simply having an API for this new header to land... so keeping it simple is in our best interest :)

Thanks for catching that point and providing options.

[myasonik]

What sort of helper components do you think we'd need? All the stuff that goes into there right now is basically EUI buttons, popovers, and context menus. What else do we need?

[joshdover]

Great, we'll keep the scope for this issue to just implementing the basic API I outlined above (or similar).

What sort of helper components do you think we'd need? All the stuff that goes into there right now is basically EUI buttons, popovers, and context menus. What else do we need?

Depends on how far we want to go. In the last meeting we discussed wanting to support collapsing this area and having different display modes for different screen sizes. I could imagine a component like this that handles this for you:

<HeaderAppControls
  primaryAction={{ title: "Save", onClick: save }}
  actions={[
    { title: "Fullscreen", onClick: toggleFullcreen, fallbackIcon: "" },
    { title: "Cancel", onClick: cancel, fallbackIcon: "" },
  ]}
/>

[elasticmachine]

Pinging @elastic/kibana-platform (Team:Platform)

[ryankeairns]

For version 1 then, the header would presumably have some sort of wrapper (e.g. EuiFlexGroup) for the menu area within which I can pass a set of EUI components, right?

If so, then we'll likely need to document the structure of that, at a minimum. Something like "Wrap each EuiButton / EuiButtonEmpty in an EuiFlexItem"... or do we say 'Pass an array of links (EuiButtons)' and the header wraps each in an EuiFlexItem?

I don't mean to dictate the actual solution, but this has me wondering how we could automatically convert to an overflow menu for smaller screens. Which, in turn, seems to have an impact on the shape of the API perhaps.

cc:/ @cchaos

[myasonik]

I don't think the API is going to know anything about EUI... It's just exposing a div or something to slot content into.

For now, I think teams can just put in whatever they want into that slot. And we can rollout a helper component per Josh's example if we want to standardize but that can come down the line.

Until then, teams can be/are responsible for handling their own narrow viewports and such.

[pgayvallet]

I don't think the API is going to know anything about EUI... It's just exposing a div or something to slot content into.
For now, I think teams can just put in whatever they want into that slot. And we can rollout a helper component per Josh's example if we want to standardize but that can come down the line.

I also think this pragmatic approach is the best compromise to have the feature on time.

BTW, do we have any idea on when the UI part of the new header is going to be delivered, to be able to start working on this specific issue?

[cchaos]

The only thing left to deliver for this is a pattern. All of the components exist in EUI at this point. The menu links should simply be a list of EuiHeaderLink(s) which you can see an example from the POC. There may be some tweaks to the existing component like better mobile/a11y support but there won't be anything new created.

https://github.com/cchaos/kibana-8-nav/blob/ba3d837a169852ce06fa3f8fded40047deef744d/src/components/kibana/chrome/chrome.tsx#L103-L117

[bmcconaghy]

@pgayvallet @joshdover we are hoping to deliver the new Kibana header in 7.10, so we will need this API somewhere in the middle of that release if that is going to be realistic.

[joshdover]

@pgayvallet @joshdover we are hoping to deliver the new Kibana header in 7.10, so we will need this API somewhere in the middle of that release if that is going to be realistic.

Don't see a problem with that from our end. The API itself probably won't be very much work, but we're blocked on having the DOM there to work with. Once there's a feature branch with the new header implementation, we can get started.

[lizozom]

@ryankeairns @joshdover @bmcconaghy this functionality is available (with different design) in the TopNavMenu component, allowing mixing buttons and links and even 3rd party plugins registering items into existing menus. Maybe that could be helpful somehow?

[bmcconaghy]

@myasonik would the component that @lizozom mentions be suitable for our purposes here?

[ryankeairns]

Just had a quick side chat with @lizozom ...

This is the component that handles the menus for Discover, Dashboard, and Visualize (others?).

During a planning discussion, we briefly discussed the existence of this component and that adapting it for the new header would move several menus in one fell swoop. It's definitely something we need to consider, so I'll set up some time for us all to chat further before we head down the path of adapting this component.

[AlonaNadler]

I haven't read the entire issue.
I wanted to express my concern regarding moving the menu to the right. My concern is based mostly on the dashboard behavior and how the call to action is in the top nav.
Moving the top nav menu to the right will have a few problems:

• The call to action is not bold enough there, having it next to the refresh and in another lighter color makes it an easy miss.
• People usually click on create new either after clicking edit or after coming from the listing page and creating new dashboard. Users usually scan a page in a certain way which will likely have them miss the create new. As another example, till 7.0 new users constantly miss the time picker which was located on the right top nav (in 7.0 it was lowered and mostly expanded)
  This is how users usually scan a page :
  

[myasonik]

RE: The TopNavMenu component
Though this component is close and covers some similar ground to what we need, I don't think it's ultimately what we want to use...

1. It locks us into React+EUI which is something the Platform team has said they want to avoid
2. It handles a lot more than what we expect this area to be used for
3. At the same time, it's pretty prescriptive about the things that you can put into it which I think is something we've said we want to avoid

[ryankeairns]

Related, I have a user test session lined up that covers this change as well - I'll be conducting that in the coming weeks. I realize that will take time, but my suspicions align with Alona's.

It seems to me we should probably head toward a separate, generic, solution that allows teams to migrate as they see fit. These test results may also push us to consider some design changes.

Let's suppose we were able to make sufficient changes that all were comfortable with moving the menus in our 'core' apps... then we could adapt TopNavMenu to feed its menu links into the header via the API. There are many if's preceding such work, but this is how I'm currently thinking about it.

[lizozom]

@AlonaNadler @myasonik I do want to stress that IMO we shouldn't create a difference between the apps using TopNavMenu and other apps. At the moment, navigation is mostly uniform and I think we should keep it a priority with this effort.

Whatever component we're planning to build, should gracefully handle transitioning from the current navigation to the new one for all apps.

[AlonaNadler]

I don't have an objection to consistency.
I have concerns having the action we want the users to take on the top right side.
One option is to remove the dashboard create panel from the top nav

[ryankeairns]

Related to that last point, I've been working on some alternatives to (or additional placement of) that Create new/Add panel button for Dashboard. I'll share this week.

[ryankeairns]

This work can be tacked on to #72331

[ryankeairns]

I did a quick scan and it appears that the TopNavMenu component is used by the following apps:

• Discover
• Dashboard
• Visualize
• Lens
• Graph
• Maps
• Timelion
• Plugin generator (?)
• Dev Tools > Console (?)

The one app not included in this list, which uses a similar menu design, is Canvas. @clintandrewhall is building a PoC that moves the menu into the new header, so this should resolve itself. That said, I'm not certain the PoC is using the TopNavMenu component.

Other than Canvas, I don't see this pattern being used elsewhere (this includes the Obs, Security, and Ent Search solutions). It seems once the header API is ready that updating the TopNavMenu should be the only task remaining to physically move all the menus.

[pgayvallet]

@joshdover

Going back on your comment in #68524 (comment)

Specifically, I think a headerActionsElement or something similar should be added to the AppMountParameters type, providing an API like:

The Chrome UI is separate from the Application UI code, though they are mounted within the same React tree. Most likely we will need to pass a React ref to both the Chrome component and the app router component so that the HTMLElement can be accessed by the app's mount function

There could be some timing issues here as well that we may need to workaround to ensure that the headerActionsElement and main app element are both "fresh" before we render the next application. By "fresh" I mean they are new div elements that are not shared with the previous app to minimize any issues from apps not cleaning up properly

I think I'm leaning more to a MountPoint setter approach than directly exposing the element to inject the content to.

The major upsides I see:

• it's going to be way easier for core to correctly cleanup when the user switch from one app to another, and to make ensuring any attempt to set the action menu from an unmounted is a no-op easier than moving the previous container out of the DOM.
• that would keep the cleanup internal to core (we would just have to call the returning Unmount function).
• Communication between the application and the chrome services would also be easier, as we would just expose an Observable<MountPoint> to the chrome header that would directly consume it and render the app menu in a reference that can be kept internal to the Header component.

It would look like something like that:

export interface AppMountParameters<HistoryLocationState = unknown> {
  ...
    /**
   * A function that can be used to set the mount point used to populate the application action container
   * in the chrome header.
   *
   * Calling this multiple time will erase the current content of the action menu with the mount from the latest call.
   *
   * Calling this after the application has been unmounted will have no effect.
   *
   * @param actionMount
   */
  setHeaderActionMenu: (actionMount: MountPoint) => void;
}

The only downside I see is that the action menu will be a totally independent react root, meaning that apps would not be able to use the portal trick to did in your example to easily perform state-sharing, and would need to use another way to share state and communicate between the actual app and the header menu react root.

This could be done by multiple ways, such as higher-level state management (redux), manually rerendering the actionMenu when needed, or re-calling setHeaderActionMenu. actionMenu to app communication could simply be done using callbacks I believe.

I don't have extensive knowledge of existing usages of the app menu (and what is going to be migrated to this new API), so I'm not exactly sure if this would be acceptable, or if this is blocker in any way.

@lizozom @joshdover could I get your insight here?

[pgayvallet]

Created #75422 to implement my proposal. Feedback welcome.