Skip to content

Commit

Permalink
Workspace left nav bar (opensearch-project#286)
Browse files Browse the repository at this point in the history 
* revert unnecessary changes to recently viewed component

refactor nav link updater so that the displayed links can be customized,
this is majority required by workspace as with workspace, user would be
able to config what features(plugins) then want to see for a workspace,
this requires to filter out those links that are not configured by the
user.

Signed-off-by: Yulong Ruan <[email protected]>

* fix test snapshot

Signed-off-by: Yulong Ruan <[email protected]>

* tweak comments

Signed-off-by: Yulong Ruan <[email protected]>

---------

Signed-off-by: Yulong Ruan <[email protected]>
  • Loading branch information
@ruanyl @SuZhou-Joe
ruanyl authored and SuZhou-Joe committed Mar 18, 2024
1 parent 3cf8b8e commit 822ef54
Show file tree
Hide file tree
Showing 21 changed files with 1,543 additions and 1,850 deletions.
3 changes: 1 addition & 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,8 @@ const createStartContractMock = () => {
const startContract: DeeplyMockedKeys<InternalChromeStart> = {
getHeaderComponent: jest.fn(),
navLinks: {
setNavLinks: jest.fn(),
getNavLinks$: jest.fn(),
getAllNavLinks$: jest.fn(),
getLinkUpdaters$: jest.fn(),
has: jest.fn(),
get: jest.fn(),
getAll: jest.fn(),
Expand Down
7 changes: 6 additions & 1 deletion src/core/public/chrome/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,12 @@ export {
ChromeHelpExtensionMenuGitHubLink,
} from './ui/header/header_help_menu';
export { NavType } from './ui';
export { ChromeNavLink, ChromeNavLinks, ChromeNavLinkUpdateableFields } from './nav_links';
export {
ChromeNavLink,
ChromeNavLinks,
ChromeNavLinkUpdateableFields,
LinksUpdater,
} from './nav_links';
export { ChromeRecentlyAccessed, ChromeRecentlyAccessedHistoryItem } from './recently_accessed';
export { ChromeNavControl, ChromeNavControls } from './nav_controls';
export { ChromeDocTitle } from './doc_title';
2 changes: 1 addition & 1 deletion src/core/public/chrome/nav_links/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,4 @@
*/

export { ChromeNavLink, ChromeNavLinkUpdateableFields } from './nav_link';
export { ChromeNavLinks, NavLinksService } from './nav_links_service';
export { ChromeNavLinks, NavLinksService, LinksUpdater } from './nav_links_service';
9 changes: 1 addition & 8 deletions src/core/public/chrome/nav_links/nav_link.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,22 +93,15 @@ export interface ChromeNavLink {
* Disables a link from being clickable.
*
* @internalRemarks
* This is used by the ML and Graph plugins. They use this field
* This is only used by the ML and Graph plugins currently. 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: 17 additions & 126 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,12 +32,18 @@ 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' }],
['app3', { id: 'app3', order: 10, title: 'App 3', icon: 'app3' }],
[
'app2',
{
id: 'app2',
order: -10,
title: 'App 2',
euiIconType: 'canvasApp',
},
],
['chromelessApp', { id: 'chromelessApp', order: 20, title: 'Chromless App', chromeless: true }],
]);

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

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

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

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

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

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

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

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

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

it('removes all other links', async () => {
Expand Down
45 changes: 15 additions & 30 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,11 @@ export interface ChromeNavLinks {
getNavLinks$(): Observable<Array<Readonly<ChromeNavLink>>>;

/**
* Get an observable for a sorted list of all navlinks.
* Get an observable for the current link updaters. Link updater is used to modify the
* nav links, for example, filter the nav links or update a specific nav link's properties.
* {@link LinksUpdater}
*/
getAllNavLinks$(): Observable<Array<Readonly<ChromeNavLink>>>;

/**
* Set navlinks.
*/
setNavLinks(navLinks: ReadonlyMap<string, ChromeNavLink>): void;
getLinkUpdaters$(): BehaviorSubject<LinksUpdater[]>;

/**
* Get the state of a navlink at this point in time.
Expand Down Expand Up @@ -122,7 +119,7 @@ export interface ChromeNavLinks {
getForceAppSwitcherNavigation$(): Observable<boolean>;
}

type LinksUpdater = (navLinks: Map<string, NavLinkWrapper>) => Map<string, NavLinkWrapper>;
export type LinksUpdater = (navLinks: Map<string, NavLinkWrapper>) => Map<string, NavLinkWrapper>;

export class NavLinksService {
private readonly stop$ = new ReplaySubject(1);
Expand All @@ -142,10 +139,7 @@ export class NavLinksService {
// manual link modifications to be able to re-apply then after every
// availableApps$ changes.
const linkUpdaters$ = new BehaviorSubject<LinksUpdater[]>([]);
const displayedNavLinks$ = new BehaviorSubject<ReadonlyMap<string, ChromeNavLink> | undefined>(
undefined
);
const allNavLinks$ = new BehaviorSubject<ReadonlyMap<string, NavLinkWrapper>>(new Map());
const navLinks$ = new BehaviorSubject<ReadonlyMap<string, NavLinkWrapper>>(new Map());

combineLatest([appLinks$, linkUpdaters$])
.pipe(
Expand All @@ -154,40 +148,31 @@ export class NavLinksService {
})
)
.subscribe((navLinks) => {
allNavLinks$.next(navLinks);
navLinks$.next(navLinks);
});

const forceAppSwitcherNavigation$ = new BehaviorSubject(false);

return {
getNavLinks$: () => {
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);
return navLinks$.pipe(map(sortNavLinks), takeUntil(this.stop$));
},

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

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

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

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

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

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

0 comments on commit 822ef54

Please sign in to comment.