Skip to content

Commit

Permalink
[Workspace][Feature] Left navigation menu adjustment (#192)
Browse files Browse the repository at this point in the history 
* add util function to filter workspace feature by wildcard

Signed-off-by: Yulong Ruan <ruanyl@amazon.com>

* resolve conflict

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* update tests and snapshots

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* small adjustment to left menu

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* resolve git conflict

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* rename nav link service function

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* unit test for workspace plugin.ts

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* update snapshots

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

* optimize code

Signed-off-by: yuye-aws <yuyezhu@amazon.com>

---------

Signed-off-by: Yulong Ruan <ruanyl@amazon.com>
Signed-off-by: yuye-aws <yuyezhu@amazon.com>
Co-authored-by: Yulong Ruan <ruanyl@amazon.com>
  • Loading branch information
@yuye-aws @ruanyl @SuZhou-Joe
2 people authored and SuZhou-Joe committed Mar 18, 2024
1 parent 63e8e9d commit 7e8c2e3
Show file tree
Hide file tree
Showing 22 changed files with 2,012 additions and 1,471 deletions.
2 changes: 2 additions & 0 deletions src/core/public/chrome/chrome_service.mock.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,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
View file
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
View file
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
View file
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());

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

0 comments on commit 7e8c2e3

Please sign in to comment.