Allow AppUpdater to change URL for app in Nav

[joshdover]

The legacy chrome allows legacy apps to have a "lastSubUrl" which is automatically set to the user's last location in an application.

This field allows users to easily switch back and forth between applications without losing their state. One flow that is currently supported is "open in new tab". In this case, the user gets a new tab, where the application opened opens to the last location, but all other applications are "fresh" in that tab.

Currently, legacy applications use the now deprecated core.chrome.navLinks.update API. This has been superceded by the appUpdater$ API. We should add support to this API for updating the deep link for a given application so that New Platform applications can avoid using this deprecated API without sacrificing functionality.

Related to #55433

\cc @flash1293

[elasticmachine]

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

[flash1293]

Related: I will introduce this functionality for the fake apps of the local application service in #55818

[pgayvallet]

Currently, legacy applications use the now deprecated core.chrome.navLinks.update API. This has been supersdeded by the appUpdater$

Does this need only concerns legacy apps, or are we talking about new apps too? (For new apps, I though is was decided that plugins should use some sort of persistent storage instead of relying on lastSubUrl that is going to be removed post 8.0)

We should add support to this API for updating the deep link for a given application so that New Platform applications can avoid using this deprecated API without sacrificing functionality.

With introduction of Application concept, there is now two way to 'navigate' between apps. One is a user-triggered navigation by clicking on a navlink, the other is using the programmatic ApplicationService.navigateTo API. Is this supposed to impact both?

The legacy chrome allows legacy apps to have a "lastSubUrl" which is automatically set to the user's last location in an application.

Implementation-wise, we are probably going to need another field than url, as url is actually used by the router's Route to match/mount the app.

[flash1293]

@pgayvallet

Does this need only concerns legacy apps, or are we talking about new apps too? (For new apps, I though is was decided that plugins should use some sort of persistent storage instead of relying on lastSubUrl that is going to be removed post 8.0)

We discussed this, but the problem with this approach is that it doesn't support the "open in new tab" flow without breaking other things. If we use session storage in a plugin to restore the last visited url, "open in new tab" looses the current sub url state. If we use local storage, the user can't use two independent tabs without overwriting each others state and causing weird behavior.

With introduction of Application concept, there is now two way to 'navigate' between apps. One is a user-triggered navigation by clicking on a navlink, the other is using the programmatic ApplicationService.navigateTo API. Is this supposed to impact both?

It would be possible to work around it when it's just impacting the nav links, but it would certainly be easier if it would impact both.

[pgayvallet]

It would be possible to work around it when it's just impacting the nav links, but it would certainly be easier if it would impact both.

is that it doesn't support the "open in new tab" flow

We will have to introduce this subUrl concept for NP app in the AppUpdater and propagate it to their navlink then, as ATM the navlink is not using navigateToApp when right clicking / opening in a new tab:

kibana/src/core/public/chrome/ui/header/nav_link.tsx

Lines 67 to 78 in da54657

	onClick(event: MouseEvent) {
	if (
	!legacyMode && // ignore when in legacy mode
	!legacy && // ignore links to legacy apps
	!event.defaultPrevented && // onClick prevented default
	event.button === 0 && // ignore everything but left clicks
	!isModifiedEvent(event) // ignore clicks with modifier keys
	) {
	event.preventDefault();
	navigateToApp(navLink.id);
	}
	},

kibana/src/core/public/chrome/ui/header/nav_link.tsx

Lines 58 to 60 in da54657

	if (legacy) {
	href = url && !active ? url : baseUrl;
	}

[pgayvallet]

@flash1293 what is the urgency on that?

[flash1293]

@pgayvallet

We will have to introduce this subUrl concept for NP app in the AppUpdater and propagate it to their navlink then, as ATM the navlink is not using navigateToApp when right clicking / opening in a new tab:

That sounds right, the anchor html element in the nav bar should contain the deep url in the href attribute.

It's not urgent as the apps that use this behavior are currently not actually using the application service, but an angular-integrated "local application service" providing the same API: src/legacy/core_plugins/kibana/public/local_application_service/local_application_service.ts
This allows us to incrementally migrate over the individual apps to use the API of the application service while still running in angular without losing the ability for the user to switch between shimmed/migrated and legacy apps without a page reload.

I can simulate the new updater behavior there for now using the deprecated navlinks service update method. By 7.8 / 8.0-alpha we are planning to remove the "local application service" and switch the apps over to the actual core application service, by then it should support updating the url.

[joshdover]

Since the kibana-app team has designed their applications around this API existing (as @flash1293 mentioned above), I think we should try to get this in this before they finish their client-side migration.

Worst case, if we don't they can copy the workaround from local_application_service into each app.

[pgayvallet]

A few questions come to mind:

• Should this lastSubUrl state be persisted on the client-side in any way?

As we are currently navigating between NP and legacy apps, most changes of app triggers a page reload, so any in-memory state of this lastSubUrl thing would be lost. I don't know what our actual needs are on that and how this was implemented in legacy?

• Should this only impact the navlinks or also add a default path when using navigateToApp?

It would be possible to work around it when it's just impacting the nav links, but it would certainly be easier if it would impact both.

Out current API looks like navigateToApp: async (appId, { path, state }: { path?: string; state?: any } = {})

Do we want to adapt it so that path fallbacks to this lastSubUrl when unspecified during the call?

• Does this API only need to handle a lastSubUrl, or does it also requires lastState

Not sure how that would reflect on the url of the navlinks, but navigateToApp do provide a state parameter. Will there be need to handle this as well?

[flash1293]

As we are currently navigating between NP and legacy apps, most changes of app triggers a page reload, so any in-memory state of this lastSubUrl thing would be lost. I don't know what our actual needs are on that and how this was implemented in legacy?

For our use case that's not necessary, the createKbnUrlTracker utility takes this job:
https://github.com/elastic/kibana/blob/master/src/plugins/kibana_utils/public/state_management/url/kbn_url_tracker.ts#L193

Should this only impact the navlinks or also add a default path when using navigateToApp?

It should also be used as a default for navigateToApp, state is not necessary, at least for our use case.

[pgayvallet]

For our use case that's not necessary, the createKbnUrlTracker utility takes this job

To be sure to understand, you mean the createKbnUrlTracker would handle the persistence and call our to-be-introduced API to set the lastSubUrl during setup/startup?

[flash1293]

@pgayvallet yes, exactly.

[flash1293]

@pgayvallet Not sure whether it helps, but for reference: this is how the kibana_legacy service is currently extending the app updater type:

kibana/src/plugins/kibana_legacy/public/plugin.ts

Line 44 in e87d994

export type AngularRenderedAppUpdater = (

The additional property is called activeUrl. We don't have to follow this for the core service.

[pgayvallet]

@flash1293

I was going to add an optional defaultPath property to App and then to AppUpdatableFields

kibana/src/core/public/application/types.ts

Line 203 in 48a33ab

export interface App<HistoryLocationState = unknown> extends AppBase {

kibana/src/core/public/application/types.ts

Line 190 in 48a33ab

export type AppUpdatableFields = Pick<AppBase, 'status' | 'navLinkStatus' | 'tooltip'>;

That way, an application can define a default path even when calling application.register (not sure if it will be of any use, but it's needed anyway as the updatable fields are technically a subset of the app).

Then, most importantly, plugins will then be able to update this defaultPath using the AppUpdater API ( app.$updater )

Then I was going to impact the behavior of both the NavLink components and the navigateTo API:

• for navigateTo

kibana/src/core/public/application/types.ts

Line 648 in 48a33ab

navigateToApp(appId: string, options?: { path?: string; state?: any }): Promise<void>;

When path is unspecified in options, I would use the defaultPath from the application if present.

• for NavLink

when defaultPath is specified on an application, it will be appended to the basePath for the url:

kibana/src/core/public/chrome/ui/header/nav_link.tsx

Lines 56 to 60 in da54657

	let href = navLink.baseUrl;
	if (legacy) {
	href = url && !active ? url : baseUrl;
	}

Somethink like (naive example)

let href = `${navLink.baseUrl}/${navLink.defaultPath}`; 

I think that would answer the need. Do you see anything I would have missed or do you have any comment?