Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Workspace][Feature] Left navigation menu adjustment #192

Merged
merged 14 commits into from
Oct 18, 2023
2 changes: 2 additions & 0 deletions src/core/public/chrome/chrome_service.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ const createStartContractMock = () => {
const startContract: DeeplyMockedKeys<InternalChromeStart> = {
getHeaderComponent: jest.fn(),
navLinks: {
setNavLinks: jest.fn(),
getNavLinks$: jest.fn(),
getAllNavLinks$: jest.fn(),
has: jest.fn(),
get: jest.fn(),
getAll: jest.fn(),
Expand Down
9 changes: 8 additions & 1 deletion src/core/public/chrome/nav_links/nav_link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,22 @@ export interface ChromeNavLink {
* Disables a link from being clickable.
*
* @internalRemarks
* This is only used by the ML and Graph plugins currently. They use this field
* This is used by the ML and Graph plugins. They use this field
* to disable the nav link when the license is expired.
* This is also used by recently visited category in left menu
* to disable "No recently visited items".
*/
readonly disabled?: boolean;

/**
* Hides a link from the navigation.
*/
readonly hidden?: boolean;

/**
* Links can be navigated through url.
*/
readonly externalLink?: boolean;
}

/** @public */
Expand Down
143 changes: 126 additions & 17 deletions src/core/public/chrome/nav_links/nav_links_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,12 @@ import { NavLinksService } from './nav_links_service';
import { take, map, takeLast } from 'rxjs/operators';
import { App } from '../../application';
import { BehaviorSubject } from 'rxjs';
import { ChromeNavLink } from 'opensearch-dashboards/public';

const availableApps = new Map([
['app1', { id: 'app1', order: 0, title: 'App 1', icon: 'app1' }],
[
'app2',
{
id: 'app2',
order: -10,
title: 'App 2',
euiIconType: 'canvasApp',
},
],
['app2', { id: 'app2', order: -10, title: 'App 2', euiIconType: 'canvasApp' }],
['app3', { id: 'app3', order: 10, title: 'App 3', icon: 'app3' }],
['chromelessApp', { id: 'chromelessApp', order: 20, title: 'Chromless App', chromeless: true }],
]);

Expand All @@ -66,7 +60,110 @@ describe('NavLinksService', () => {
start = service.start({ application: mockAppService, http: mockHttp });
});

describe('#getNavLinks$()', () => {
describe('#getAllNavLinks$()', () => {
it('does not include `chromeless` applications', async () => {
expect(
await start
.getAllNavLinks$()
.pipe(
take(1),
map((links) => links.map((l) => l.id))
)
.toPromise()
).not.toContain('chromelessApp');
});

it('sorts navLinks by `order` property', async () => {
expect(
await start
.getAllNavLinks$()
.pipe(
take(1),
map((links) => links.map((l) => l.id))
)
.toPromise()
).toEqual(['app2', 'app1', 'app3']);
});

it('emits multiple values', async () => {
const navLinkIds$ = start.getAllNavLinks$().pipe(map((links) => links.map((l) => l.id)));
const emittedLinks: string[][] = [];
navLinkIds$.subscribe((r) => emittedLinks.push(r));
start.update('app1', { href: '/foo' });

service.stop();
expect(emittedLinks).toEqual([
['app2', 'app1', 'app3'],
['app2', 'app1', 'app3'],
]);
});

it('completes when service is stopped', async () => {
const last$ = start.getAllNavLinks$().pipe(takeLast(1)).toPromise();
service.stop();
await expect(last$).resolves.toBeInstanceOf(Array);
});
});

describe('#getNavLinks$() when non null', () => {
// set filtered nav links, nav link with order smaller than 0 will be filtered
beforeEach(() => {
const filteredNavLinks = new Map<string, ChromeNavLink>();
start.getAllNavLinks$().subscribe((links) =>
links.forEach((link) => {
if (link.order !== undefined && link.order >= 0) {
filteredNavLinks.set(link.id, link);
}
})
);
start.setNavLinks(filteredNavLinks);
});

it('does not include `app2` applications', async () => {
expect(
await start
.getNavLinks$()
.pipe(
take(1),
map((links) => links.map((l) => l.id))
)
.toPromise()
).not.toContain('app2');
});

it('sorts navLinks by `order` property', async () => {
expect(
await start
.getNavLinks$()
.pipe(
take(1),
map((links) => links.map((l) => l.id))
)
.toPromise()
).toEqual(['app1', 'app3']);
});

it('emits multiple values', async () => {
const navLinkIds$ = start.getNavLinks$().pipe(map((links) => links.map((l) => l.id)));
const emittedLinks: string[][] = [];
navLinkIds$.subscribe((r) => emittedLinks.push(r));
start.update('app1', { href: '/foo' });

service.stop();
expect(emittedLinks).toEqual([
['app1', 'app3'],
['app1', 'app3'],
]);
});

it('completes when service is stopped', async () => {
const last$ = start.getNavLinks$().pipe(takeLast(1)).toPromise();
service.stop();
await expect(last$).resolves.toBeInstanceOf(Array);
});
});

describe('#getNavLinks$() when null', () => {
it('does not include `chromeless` applications', async () => {
expect(
await start
Expand All @@ -79,7 +176,19 @@ describe('NavLinksService', () => {
).not.toContain('chromelessApp');
});

it('sorts navlinks by `order` property', async () => {
it('include `app2` applications', async () => {
expect(
await start
.getNavLinks$()
.pipe(
take(1),
map((links) => links.map((l) => l.id))
)
.toPromise()
).toContain('app2');
});

it('sorts navLinks by `order` property', async () => {
expect(
await start
.getNavLinks$()
Expand All @@ -88,7 +197,7 @@ describe('NavLinksService', () => {
map((links) => links.map((l) => l.id))
)
.toPromise()
).toEqual(['app2', 'app1']);
).toEqual(['app2', 'app1', 'app3']);
});

it('emits multiple values', async () => {
Expand All @@ -99,8 +208,8 @@ describe('NavLinksService', () => {

service.stop();
expect(emittedLinks).toEqual([
['app2', 'app1'],
['app2', 'app1'],
['app2', 'app1', 'app3'],
['app2', 'app1', 'app3'],
]);
});

Expand All @@ -123,7 +232,7 @@ describe('NavLinksService', () => {

describe('#getAll()', () => {
it('returns a sorted array of navlinks', () => {
expect(start.getAll().map((l) => l.id)).toEqual(['app2', 'app1']);
expect(start.getAll().map((l) => l.id)).toEqual(['app2', 'app1', 'app3']);
});
});

Expand All @@ -148,7 +257,7 @@ describe('NavLinksService', () => {
map((links) => links.map((l) => l.id))
)
.toPromise()
).toEqual(['app2', 'app1']);
).toEqual(['app2', 'app1', 'app3']);
});

it('does nothing on chromeless applications', async () => {
Expand All @@ -161,7 +270,7 @@ describe('NavLinksService', () => {
map((links) => links.map((l) => l.id))
)
.toPromise()
).toEqual(['app2', 'app1']);
).toEqual(['app2', 'app1', 'app3']);
});

it('removes all other links', async () => {
Expand Down
44 changes: 35 additions & 9 deletions src/core/public/chrome/nav_links/nav_links_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,16 @@ export interface ChromeNavLinks {
*/
getNavLinks$(): Observable<Array<Readonly<ChromeNavLink>>>;

/**
* Get an observable for a sorted list of all navlinks.
*/
getAllNavLinks$(): Observable<Array<Readonly<ChromeNavLink>>>;

/**
* Set navlinks.
*/
setNavLinks(navLinks: ReadonlyMap<string, ChromeNavLink>): void;

/**
* Get the state of a navlink at this point in time.
* @param id
Expand Down Expand Up @@ -132,36 +142,52 @@ export class NavLinksService {
// manual link modifications to be able to re-apply then after every
// availableApps$ changes.
const linkUpdaters$ = new BehaviorSubject<LinksUpdater[]>([]);
const navLinks$ = new BehaviorSubject<ReadonlyMap<string, NavLinkWrapper>>(new Map());
const displayedNavLinks$ = new BehaviorSubject<ReadonlyMap<string, ChromeNavLink> | undefined>(
undefined
);
const allNavLinks$ = new BehaviorSubject<ReadonlyMap<string, NavLinkWrapper>>(new Map());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can move navLinks$ here so it align with existing pattern.

    const navLinks$ = new BehaviorSubject<ReadonlyMap<string, ChromeNavLink> | undefined>(
    undefined
  );


combineLatest([appLinks$, linkUpdaters$])
.pipe(
map(([appLinks, linkUpdaters]) => {
return linkUpdaters.reduce((links, updater) => updater(links), appLinks);
})
)
.subscribe((navlinks) => {
navLinks$.next(navlinks);
.subscribe((navLinks) => {
allNavLinks$.next(navLinks);
});

const forceAppSwitcherNavigation$ = new BehaviorSubject(false);

return {
getNavLinks$: () => {
return navLinks$.pipe(map(sortNavLinks), takeUntil(this.stop$));
return combineLatest([allNavLinks$, displayedNavLinks$]).pipe(
map(([allNavLinks, displayedNavLinks]) =>
displayedNavLinks === undefined ? sortLinks(allNavLinks) : sortLinks(displayedNavLinks)
),
takeUntil(this.stop$)
);
},

setNavLinks: (navLinks: ReadonlyMap<string, ChromeNavLink>) => {
displayedNavLinks$.next(navLinks);
},

getAllNavLinks$: () => {
return allNavLinks$.pipe(map(sortLinks), takeUntil(this.stop$));
},

get(id: string) {
const link = navLinks$.value.get(id);
const link = allNavLinks$.value.get(id);
return link && link.properties;
},

getAll() {
return sortNavLinks(navLinks$.value);
return sortLinks(allNavLinks$.value);
},

has(id: string) {
return navLinks$.value.has(id);
return allNavLinks$.value.has(id);
},

showOnly(id: string) {
Expand Down Expand Up @@ -209,9 +235,9 @@ export class NavLinksService {
}
}

function sortNavLinks(navLinks: ReadonlyMap<string, NavLinkWrapper>) {
function sortLinks(links: ReadonlyMap<string, NavLinkWrapper | ChromeNavLink>) {
return sortBy(
[...navLinks.values()].map((link) => link.properties),
[...links.values()].map((link) => ('properties' in link ? link.properties : link)),
'order'
);
}
Loading
Loading