Fix navigateToApp logic when navigating to the current app.

[pgayvallet]

Summary

Fix logic of navigateToApp to consider when we are performing internal navigation (navigating to the current app).

When navigating to the current app:

• we don't execute 'should navigate' logic if the app registered an appLeaveHandler
• we don't clear the appActionMenu that the current app may have registered

Technically, when navigating to the same app, the AppContainer does not remount the application (call mount again), meaning that navigateTo(currentApp) was causing all the handlers registered during the initial mount to be cleaned.

cc @constancecchen

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 1973f66

Metrics [docs]

page load bundle size

id	before	after	diff
core	655.1KB	655.4KB	+292.0B

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[elasticmachine]

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

[mshustov]

KP Route doesn't know about plugin internal routes, why we use it to perform navigations inside an app?
We even have a test for this

kibana/src/core/public/application/integration_tests/router.test.tsx

Lines 300 to 309 in 3094ca1

	it('should not remount when changing pages within app', async () => {
	const { mounter, unmount } = mounters.get('app1')!;
	await navigate('/app/app1/page1');
	expect(mounter.mount).toHaveBeenCalledTimes(1);
	// Navigating to page within app does not trigger re-render
	await navigate('/app/app1/page2');
	expect(mounter.mount).toHaveBeenCalledTimes(1);
	expect(unmount).not.toHaveBeenCalled();
	});

[pgayvallet]

Well, apps can/are using this API to perform intra-app navigation, with utilities such as RedirectAppLinks, and overall all usages of navigateToApp(app, { path }) or navigateToUrl('{appRoute}/{path}') used to navigate within your own app. it was never meant to exclusively be used to execute app-to-app navigation.

This test is valid: it shouldn't remount the app. When navigating to the same prefix as the currently mounted app, it should basically just history.push to let the underlying app router catch the pushState event itself via its ScopedHistory instance.

The issue with intra-app link was only introduced when we added logic to navigateToApp before it actually performs the call to history.push (when we added leaveHandler, then quite recently, setAppActionMenu)

[mshustov]

could you add a comment why we don't call shouldNavigate? it's not obvious that shouldNavigate calls confirmation modal under the hood.