diff --git a/src/core/public/chrome/chrome_service.test.ts b/src/core/public/chrome/chrome_service.test.ts index 082ffbfa16ed..25fb9d35b54b 100644 --- a/src/core/public/chrome/chrome_service.test.ts +++ b/src/core/public/chrome/chrome_service.test.ts @@ -46,6 +46,7 @@ import { HeaderVariant } from './constants'; class FakeApp implements App { public title: string; + public appRoute: string; public mount = () => () => {}; constructor( @@ -54,6 +55,7 @@ class FakeApp implements App { public headerVariant?: HeaderVariant ) { this.title = `${this.id} App`; + this.appRoute = this.id; } } const store = new Map(); @@ -78,12 +80,18 @@ function defaultStartDeps(availableApps?: App[]) { uiSettings: uiSettingsServiceMock.createStartContract(), overlays: overlayServiceMock.createStartContract(), workspaces: workspacesServiceMock.createStartContract(), + updateApplications: (() => {}) as (applications?: App[]) => void, }; if (availableApps) { - deps.application.applications$ = new Rx.BehaviorSubject>( + const applications$ = new Rx.BehaviorSubject>( new Map(availableApps.map((app) => [app.id, getAppInfo(app) as PublicAppInfo])) ); + deps.application.applications$ = applications$; + deps.updateApplications = (applications?: App[]) => + applications$.next( + new Map(applications?.map((app) => [app.id, getAppInfo(app) as PublicAppInfo])) + ); } return deps; @@ -285,6 +293,31 @@ describe('start', () => { ] `); }); + + it('should use correct current app id to tell if hidden', async () => { + const apps = [new FakeApp('alpha', true), new FakeApp('beta', false)]; + const startDeps = defaultStartDeps(apps); + const { navigateToApp } = startDeps.application; + const { chrome } = await start({ startDeps }); + const visibleChangedArray: boolean[] = []; + const visible$ = chrome.getIsVisible$(); + visible$.subscribe((visible) => visibleChangedArray.push(visible)); + + await navigateToApp('alpha'); + + await navigateToApp('beta'); + startDeps.updateApplications(apps); + + expect(visibleChangedArray).toMatchInlineSnapshot(` + Array [ + false, + false, + true, + true, + true, + ] + `); + }); }); describe('header variant', () => { diff --git a/src/core/public/chrome/chrome_service.tsx b/src/core/public/chrome/chrome_service.tsx index 39f9f0fbc320..10613b9a2f04 100644 --- a/src/core/public/chrome/chrome_service.tsx +++ b/src/core/public/chrome/chrome_service.tsx @@ -160,18 +160,32 @@ export class ChromeService { const isEmbedded = new URL(location.hash.slice(1), location.origin).searchParams.has('embed'); this.isForceHidden$ = new BehaviorSubject(isEmbedded); + /** + * There is a staled closure issue here. + * For example, when currentAppId$ is going through A -> B -> C and + * the application.applications$ just get changed in B, then it will always use B as the currentAppId + * even though the latest appId now is C. + */ + let currentAppId: string | undefined; + const appHidden$ = merge( // For the isVisible$ logic, having no mounted app is equivalent to having a hidden app // in the sense that the chrome UI should not be displayed until a non-chromeless app is mounting or mounted of(true), application.currentAppId$.pipe( - flatMap((appId) => - application.applications$.pipe( + flatMap((appId) => { + // Update the currentAppId to latest + currentAppId = appId; + return application.applications$.pipe( map((applications) => { - return !!appId && applications.has(appId) && !!applications.get(appId)!.chromeless; + return ( + !!currentAppId && + applications.has(currentAppId) && + !!applications.get(currentAppId)!.chromeless + ); }) - ) - ) + ); + }) ) ); this.isVisible$ = combineLatest([appHidden$, this.isForceHidden$]).pipe(