[discuss] Remove app url changes in the side nav

[Dosant]

Currently Discover, Dashboard and Visualize app (not Lens) use the kbnUrlTracker helper to manipulate the URL to the app's link in the nav bar.

kibana/src/plugins/kibana_utils/public/state_management/url/kbn_url_tracker.ts

Line 46 in 74e6282

export function createKbnUrlTracker({

Example usage in discover:

kibana/src/plugins/discover/public/utils/initialize_kbn_url_tracking.ts

Lines 44 to 93 in 74e6282

	const {
	appMounted,
	appUnMounted,
	stop: stopUrlTracker,
	setActiveUrl: setTrackedUrl,
	restorePreviousUrl,
	} = createKbnUrlTracker({
	// we pass getter here instead of plain `history`,
	// so history is lazily created (when app is mounted)
	// this prevents redundant `#` when not in discover app
	getHistory: getScopedHistory,
	baseUrl,
	defaultSubUrl: '#/',
	storageKey: `lastUrl:${core.http.basePath.get()}:discover`,
	navLinkUpdater$,
	toastNotifications: core.notifications.toasts,
	stateParams: [
	{
	kbnUrlKey: '_g',
	stateUpdate$: plugins.data.query.state$.pipe(
	filter(
	({ changes }) => !!(changes.globalFilters || changes.time || changes.refreshInterval)
	),
	map(async ({ state }) => {
	const { isFilterPinned } = await import('@kbn/es-query');
	return {
	...state,
	filters: state.filters?.filter(isFilterPinned),
	};
	})
	),
	},
	],
	shouldTrackUrlUpdate: () => {
	return getUrlTracking().enabled;
	},
	onBeforeNavLinkSaved: (newNavLink: string) => {
	// Do not save SEARCH_SESSION_ID into nav link, because of possible edge cases
	// that could lead to session restoration failure.
	// see: https://github.com/elastic/kibana/issues/87149
	if (newNavLink.includes(SEARCH_SESSION_ID_QUERY_PARAM)) {
	newNavLink = replaceUrlHashQuery(newNavLink, (query) => {
	delete query[SEARCH_SESSION_ID_QUERY_PARAM];
	return query;
	});
	}
	return newNavLink;
	},
	});

Original Goals

This functionality had two goals:

Restore app's state when navigating back

The first goal was to preserve app's state when navigating between apps. This is how it worked:

1. In Discover get into some not default app state.
2. The app state in Discover is preserved to the current URL
3. Then navigate to the Dashboard
4. kbnUrlTracker sees that the user navigates away from the Discover, picks the current URL with all the state, and updates the link to the Discover app putting all the state there
5. After navigating the Dashboard, user navigates back to Discover using the side nav. As users clicks on the link with the old state in it, Discover opens with the previous state.

This worked through the links because in the past Kibana had hard reloads when navigating between apps (no sure why not session storage). Today it would be more straightforward if this would be implemented simply in-memory.
As I understand, since dashboard removed most of the state from the URL, this was re-implemented there using memory or session storage.

Transfer global state between apps (time range and pinned filters)

This was also used to transfer time range and pinned filters changes between apps. When the "global" state changed, all the links in the sidebar where updates with the current time range and pinned filters. This way when user navigated to the app, the state of those was restored from the link.

This is likely redundant now because this state can be kept in-memory in filter manager and time filter service

Why Remove This

Some reasons why I think we should consider removing that functionality. Then apps should decide if they want to drop the feature they got from this or re-implement in a more simple way:

1. Code is complicated and is messing with the URLs. Can be re-implemented using in-memory and session storage means.
2. By default everything from the URL is preserved in the link. This leads to unwanted state restorations. This was a thing we hit when trying to enable URL syncing for the table list view [Table list view] Add state in URL #145517 (comment) . The workaround could be to manually exclude unwanted url parts like we did for the search session id:

   kibana/src/plugins/discover/public/utils/initialize_kbn_url_tracking.ts

   Lines 81 to 89 in 74e6282

   	// Do not save SEARCH_SESSION_ID into nav link, because of possible edge cases
   	// that could lead to session restoration failure.
   	// see: https://github.com/elastic/kibana/issues/87149
   	if (newNavLink.includes(SEARCH_SESSION_ID_QUERY_PARAM)) {
   	newNavLink = replaceUrlHashQuery(newNavLink, (query) => {
   	delete query[SEARCH_SESSION_ID_QUERY_PARAM];
   	return query;
   	});
   	}

3. The behavior is not consistent across Kibana. Some users even consider this a bug before ([Table list view] Add state in URL #145517 (comment)). I think ideally long-term we probably should approach this in a holistic way and help to make a consistent behavior across all apps

cc @ThomThomson, @sebelga

[elasticmachine]

Pinging @elastic/kibana-presentation (Team:Presentation)

[elasticmachine]

Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations)

[elasticmachine]

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

[sebelga]

Thanks for opening this issue @Dosant 👍

I personally think that we should avoid as much as possible changing how a user interacts with the browser and what they expect from it. If the main menu link takes me to "/dashboard/home" I expect this link/behaviour to be consistent no mater what' I've been doing before.

If apps see the need to restore state, like you said there are different options, be it memory through a shared service or session storage.

For the UX it can be pretty cool to have a banner on top when navigating back: "Do you want to restore your previous state?" and maybe even some toggles to let the user restore partial states (time range, filters, sorting...)

[elasticmachine]

Pinging @elastic/appex-sharedux (Team:SharedUX)

[vadimkibana]

+1 for not porting this functionality to Serverless, I've always found this "feature" very confusing in the on-prem Kibana; something that we should remove. So, hopefully if we can stop doing this in Serverless that is one step towards also removing it in on-prem.

[ThomThomson]

+1 from the presentation team on removing this functionality. This will help a lot with removing complexity from the app, and will lead to more consistent behaviour.

@Dosant, are we considering moving forward with removing this on prem?