Skip to content

Commit

Permalink
[Backport workspace] Left navigation menu adjustment (opensearch-proj…
Browse files Browse the repository at this point in the history 
…ect#192) (opensearch-project#233)

* [Workspace][Feature] Left navigation menu adjustment (opensearch-project#192)

* 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>

* resolve conflict

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

* update snapshots

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

* resolve conflicts

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

* restore snapshot

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

* update tests

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
yuye-aws and ruanyl authored Oct 26, 2023
1 parent 7e2ace0 commit f4dfa30
Show file tree
Hide file tree
Showing 17 changed files with 1,316 additions and 487 deletions.
4 changes: 2 additions & 2 deletions src/core/public/chrome/chrome_service.mock.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ const createStartContractMock = () => {
const startContract: DeeplyMockedKeys<InternalChromeStart> = {
getHeaderComponent: jest.fn(),
navLinks: {
setNavLinks: jest.fn(),
getNavLinks$: jest.fn(),
getFilteredNavLinks$: jest.fn(),
setFilteredNavLinks: jest.fn(),
getAllNavLinks$: jest.fn(),
has: jest.fn(),
get: jest.fn(),
getAll: jest.fn(),
Expand Down
2 changes: 1 addition & 1 deletion src/core/public/chrome/chrome_service.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ export class ChromeService {
homeHref={http.basePath.prepend('/app/home')}
isVisible$={this.isVisible$}
opensearchDashboardsVersion={injectedMetadata.getOpenSearchDashboardsVersion()}
navLinks$={navLinks.getFilteredNavLinks$()}
navLinks$={navLinks.getNavLinks$()}
customNavLink$={customNavLink$.pipe(takeUntil(this.stop$))}
recentlyAccessed$={recentlyAccessed.get$()}
navControlsLeft$={navControls.getLeft$()}
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 nav links, nav link with order smaller than 0 will be filtered
beforeEach(() => {
const navLinks = new Map<string, ChromeNavLink>();
start.getAllNavLinks$().subscribe((links) =>
links.forEach((link) => {
if (link.order !== undefined && link.order >= 0) {
navLinks.set(link.id, link);
}
})
);
start.setNavLinks(navLinks);
});

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
57 changes: 24 additions & 33 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 @@ -54,14 +54,14 @@ export interface ChromeNavLinks {
getNavLinks$(): Observable<Array<Readonly<ChromeNavLink>>>;

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

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

/**
* Get the state of a navlink at this point in time.
Expand Down Expand Up @@ -126,9 +126,6 @@ type LinksUpdater = (navLinks: Map<string, NavLinkWrapper>) => Map<string, NavLi

export class NavLinksService {
private readonly stop$ = new ReplaySubject(1);
private filteredNavLinks$ = new BehaviorSubject<ReadonlyMap<string, ChromeNavLink> | undefined>(
undefined
);

public start({ application, http }: StartDeps): ChromeNavLinks {
const appLinks$ = application.applications$.pipe(
Expand All @@ -145,7 +142,10 @@ 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(
Expand All @@ -154,42 +154,40 @@ export class NavLinksService {
})
)
.subscribe((navLinks) => {
navLinks$.next(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$)
);
},

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

getFilteredNavLinks$: () => {
return combineLatest([navLinks$, this.filteredNavLinks$]).pipe(
map(([navLinks, filteredNavLinks]) =>
filteredNavLinks === undefined
? sortNavLinks(navLinks)
: sortChromeNavLinks(filteredNavLinks)
),
takeUntil(this.stop$)
);
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 @@ -237,16 +235,9 @@ export class NavLinksService {
}
}

function sortNavLinks(navLinks: ReadonlyMap<string, NavLinkWrapper>) {
return sortBy(
[...navLinks.values()].map((link) => link.properties),
'order'
);
}

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

0 comments on commit f4dfa30

Please sign in to comment.