Skip to content

Commit

Permalink
Fix navigateToApp logic when navigating to the current app. (elastic#…
Browse files Browse the repository at this point in the history
  • Loading branch information
pgayvallet committed Oct 16, 2020
1 parent cb6e99a commit 716c8c8
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 2 deletions.
10 changes: 8 additions & 2 deletions src/core/public/application/application_service.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -242,11 +242,17 @@ export class ApplicationService {
appId,
{ path, state, replace = false }: NavigateToAppOptions = {}
) => {
if (await this.shouldNavigate(overlays)) {
const currentAppId = this.currentAppId$.value;
const navigatingToSameApp = currentAppId === appId;
const shouldNavigate = navigatingToSameApp ? true : await this.shouldNavigate(overlays);

if (shouldNavigate) {
if (path === undefined) {
path = applications$.value.get(appId)?.defaultPath;
}
this.appInternalStates.delete(this.currentAppId$.value!);
if (!navigatingToSameApp) {
this.appInternalStates.delete(this.currentAppId$.value!);
}
this.navigate!(getAppUrl(availableMounters, appId, path), state, replace);
this.currentAppId$.next(appId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,34 @@ describe('ApplicationService', () => {
expect(history.entries.length).toEqual(2);
expect(history.entries[1].pathname).toEqual('/app/app1');
});

it('does not trigger navigation check if navigating to the current app', async () => {
startDeps.overlays.openConfirm.mockResolvedValue(false);

const { register } = service.setup(setupDeps);

register(Symbol(), {
id: 'app1',
title: 'App1',
mount: ({ onAppLeave }: AppMountParameters) => {
onAppLeave((actions) => actions.confirm('confirmation-message', 'confirmation-title'));
return () => undefined;
},
});

const { navigateToApp, getComponent } = await service.start(startDeps);

update = createRenderer(getComponent());

await act(async () => {
await navigate('/app/app1');
await navigateToApp('app1', { path: '/internal-path' });
});

expect(startDeps.overlays.openConfirm).not.toHaveBeenCalled();
expect(history.entries.length).toEqual(3);
expect(history.entries[2].pathname).toEqual('/app/app1/internal-path');
});
});

describe('registering action menus', () => {
Expand Down Expand Up @@ -331,6 +359,48 @@ describe('ApplicationService', () => {
expect(await getValue(currentActionMenu$)).toBe(mounter2);
});

it('does not update the observable value when navigating to the current app', async () => {
const { register } = service.setup(setupDeps);

let initialMount = true;
register(Symbol(), {
id: 'app1',
title: 'App1',
mount: async ({ setHeaderActionMenu }: AppMountParameters) => {
if (initialMount) {
setHeaderActionMenu(mounter1);
initialMount = false;
}
return () => undefined;
},
});

const { navigateToApp, getComponent, currentActionMenu$ } = await service.start(startDeps);
update = createRenderer(getComponent());

let mountedMenuCount = 0;
currentActionMenu$.subscribe(() => {
mountedMenuCount++;
});

await act(async () => {
await navigateToApp('app1');
await flushPromises();
});

expect(await getValue(currentActionMenu$)).toBe(mounter1);

await act(async () => {
await navigateToApp('app1');
await flushPromises();
});

expect(await getValue(currentActionMenu$)).toBe(mounter1);

// there is an initial 'undefined' emission
expect(mountedMenuCount).toBe(2);
});

it('updates the observable value to undefined when switching to an application without action menu', async () => {
const { register } = service.setup(setupDeps);

Expand Down

0 comments on commit 716c8c8

Please sign in to comment.