From e44616e8fceef35edb70303bcb4aef57ccf4db78 Mon Sep 17 00:00:00 2001 From: Eli Perelman Date: Thu, 14 Nov 2019 09:36:03 -0600 Subject: [PATCH] Allow chromeless applications to hide left navbar link (#50060) --- .../nav_links/nav_links_service.test.ts | 76 ++++++++++++++----- .../chrome/nav_links/nav_links_service.ts | 24 +++--- .../test_suites/core_plugins/applications.ts | 8 +- 3 files changed, 76 insertions(+), 32 deletions(-) diff --git a/src/core/public/chrome/nav_links/nav_links_service.test.ts b/src/core/public/chrome/nav_links/nav_links_service.test.ts index 8c135b3c4c49f0..5a45491df28e7b 100644 --- a/src/core/public/chrome/nav_links/nav_links_service.test.ts +++ b/src/core/public/chrome/nav_links/nav_links_service.test.ts @@ -19,27 +19,34 @@ import { NavLinksService } from './nav_links_service'; import { take, map, takeLast } from 'rxjs/operators'; -import { LegacyApp } from '../../application'; +import { App, LegacyApp } from '../../application'; const mockAppService = { - availableApps: new Map(), - availableLegacyApps: new Map([ - [ - 'legacyApp1', - { id: 'legacyApp1', order: 0, title: 'Legacy App 1', icon: 'legacyApp1', appUrl: '/app1' }, - ], - [ - 'legacyApp2', + availableApps: new Map( + ([ + { id: 'app1', order: 0, title: 'App 1', icon: 'app1' }, { - id: 'legacyApp2', + id: 'app2', order: -10, + title: 'App 2', + euiIconType: 'canvasApp', + }, + { id: 'chromelessApp', order: 20, title: 'Chromless App', chromeless: true }, + ] as App[]).map(app => [app.id, app]) + ), + availableLegacyApps: new Map( + ([ + { id: 'legacyApp1', order: 5, title: 'Legacy App 1', icon: 'legacyApp1', appUrl: '/app1' }, + { + id: 'legacyApp2', + order: -5, title: 'Legacy App 2', euiIconType: 'canvasApp', appUrl: '/app2', }, - ], - ['legacyApp3', { id: 'legacyApp3', order: 20, title: 'Legacy App 3', appUrl: '/app3' }], - ]), + { id: 'legacyApp3', order: 15, title: 'Legacy App 3', appUrl: '/app3' }, + ] as LegacyApp[]).map(app => [app.id, app]) + ), } as any; const mockHttp = { @@ -58,6 +65,18 @@ describe('NavLinksService', () => { }); describe('#getNavLinks$()', () => { + it('does not include `chromeless` applications', async () => { + expect( + await start + .getNavLinks$() + .pipe( + take(1), + map(links => links.map(l => l.id)) + ) + .toPromise() + ).not.toContain('chromelessApp'); + }); + it('sorts navlinks by `order` property', async () => { expect( await start @@ -67,7 +86,7 @@ describe('NavLinksService', () => { map(links => links.map(l => l.id)) ) .toPromise() - ).toEqual(['legacyApp2', 'legacyApp1', 'legacyApp3']); + ).toEqual(['app2', 'legacyApp2', 'app1', 'legacyApp1', 'legacyApp3']); }); it('emits multiple values', async () => { @@ -78,8 +97,8 @@ describe('NavLinksService', () => { service.stop(); expect(emittedLinks).toEqual([ - ['legacyApp2', 'legacyApp1', 'legacyApp3'], - ['legacyApp2', 'legacyApp1', 'legacyApp3'], + ['app2', 'legacyApp2', 'app1', 'legacyApp1', 'legacyApp3'], + ['app2', 'legacyApp2', 'app1', 'legacyApp1', 'legacyApp3'], ]); }); @@ -105,7 +124,13 @@ describe('NavLinksService', () => { describe('#getAll()', () => { it('returns a sorted array of navlinks', () => { - expect(start.getAll().map(l => l.id)).toEqual(['legacyApp2', 'legacyApp1', 'legacyApp3']); + expect(start.getAll().map(l => l.id)).toEqual([ + 'app2', + 'legacyApp2', + 'app1', + 'legacyApp1', + 'legacyApp3', + ]); }); }); @@ -130,7 +155,20 @@ describe('NavLinksService', () => { map(links => links.map(l => l.id)) ) .toPromise() - ).toEqual(['legacyApp2', 'legacyApp1', 'legacyApp3']); + ).toEqual(['app2', 'legacyApp2', 'app1', 'legacyApp1', 'legacyApp3']); + }); + + it('does nothing on chromeless applications', async () => { + start.showOnly('chromelessApp'); + expect( + await start + .getNavLinks$() + .pipe( + take(1), + map(links => links.map(l => l.id)) + ) + .toPromise() + ).toEqual(['app2', 'legacyApp2', 'app1', 'legacyApp1', 'legacyApp3']); }); it('removes all other links', async () => { @@ -157,7 +195,7 @@ describe('NavLinksService', () => { "icon": "legacyApp1", "id": "legacyApp1", "legacy": true, - "order": 0, + "order": 5, "title": "Legacy App 1", } `); diff --git a/src/core/public/chrome/nav_links/nav_links_service.ts b/src/core/public/chrome/nav_links/nav_links_service.ts index a636ff878dd410..31a729f90cd932 100644 --- a/src/core/public/chrome/nav_links/nav_links_service.ts +++ b/src/core/public/chrome/nav_links/nav_links_service.ts @@ -99,17 +99,19 @@ export class NavLinksService { private readonly stop$ = new ReplaySubject(1); public start({ application, http }: StartDeps): ChromeNavLinks { - const appLinks = [...application.availableApps].map( - ([appId, app]) => - [ - appId, - new NavLinkWrapper({ - ...app, - legacy: false, - baseUrl: relativeToAbsolute(http.basePath.prepend(`/app/${appId}`)), - }), - ] as [string, NavLinkWrapper] - ); + const appLinks = [...application.availableApps] + .filter(([, app]) => !app.chromeless) + .map( + ([appId, app]) => + [ + appId, + new NavLinkWrapper({ + ...app, + legacy: false, + baseUrl: relativeToAbsolute(http.basePath.prepend(`/app/${appId}`)), + }), + ] as [string, NavLinkWrapper] + ); const legacyAppLinks = [...application.availableLegacyApps].map( ([appId, app]) => diff --git a/test/plugin_functional/test_suites/core_plugins/applications.ts b/test/plugin_functional/test_suites/core_plugins/applications.ts index 138e20b9877610..c16847dab9dc23 100644 --- a/test/plugin_functional/test_suites/core_plugins/applications.ts +++ b/test/plugin_functional/test_suites/core_plugins/applications.ts @@ -91,14 +91,18 @@ export default function({ getService, getPageObjects }: PluginFunctionalProvider await testSubjects.existOrFail('fooAppPageA'); }); + it('chromeless applications are not visible in apps list', async () => { + expect(await appsMenu.linkExists('Chromeless')).to.be(false); + }); + it('navigating to chromeless application hides chrome', async () => { - await appsMenu.clickLink('Chromeless'); + await PageObjects.common.navigateToApp('chromeless'); await loadingScreenNotShown(); expect(await testSubjects.exists('headerGlobalNav')).to.be(false); }); it('navigating away from chromeless application shows chrome', async () => { - await browser.goBack(); + await PageObjects.common.navigateToApp('foo'); await loadingScreenNotShown(); expect(await testSubjects.exists('headerGlobalNav')).to.be(true); });