Skip to content

Commit

Permalink
[Stateful sidenav] Update feedback urls (#198143)
Browse files Browse the repository at this point in the history
  • Loading branch information
sebelga authored Oct 31, 2024
1 parent 2162c56 commit 89fe548
Show file tree
Hide file tree
Showing 29 changed files with 113 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import type {
ChromeSetProjectBreadcrumbsParams,
NavigationTreeDefinition,
AppDeepLinkId,
SolutionId,
} from '@kbn/core-chrome-browser';
import type { CustomBrandingStart } from '@kbn/core-custom-branding-browser';
import type {
Expand Down Expand Up @@ -343,7 +344,10 @@ export class ChromeService {
LinkId extends AppDeepLinkId = AppDeepLinkId,
Id extends string = string,
ChildrenId extends string = Id
>(id: string, navigationTree$: Observable<NavigationTreeDefinition<LinkId, Id, ChildrenId>>) {
>(
id: SolutionId,
navigationTree$: Observable<NavigationTreeDefinition<LinkId, Id, ChildrenId>>
) {
validateChromeStyle();
projectNavigation.initNavigation(id, navigationTree$);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ describe('initNavigation()', () => {

beforeAll(() => {
projectNavigation.initNavigation<any>(
'foo',
'es',
of({
body: [
{
Expand Down Expand Up @@ -185,7 +185,7 @@ describe('initNavigation()', () => {
const { projectNavigation: projNavigation, getNavigationTree: getNavTree } =
setupInitNavigation();
projNavigation.initNavigation<any>(
'foo',
'es',
of({
body: [
{
Expand All @@ -210,7 +210,7 @@ describe('initNavigation()', () => {
const { projectNavigation: projNavigation } = setupInitNavigation();

projNavigation.initNavigation<any>(
'foo',
'es',
of({
body: [
{
Expand Down Expand Up @@ -399,7 +399,7 @@ describe('initNavigation()', () => {

// 2. initNavigation() is called
projectNavigation.initNavigation<any>(
'foo',
'es',
of({
body: [
{
Expand Down Expand Up @@ -427,7 +427,7 @@ describe('initNavigation()', () => {
});

projectNavigation.initNavigation<any>(
'foo',
'es',
// @ts-expect-error - We pass a non valid cloudLink that is not TS valid
of({
body: [
Expand Down Expand Up @@ -533,7 +533,7 @@ describe('breadcrumbs', () => {
const obs = subj.asObservable();

if (initiateNavigation) {
projectNavigation.initNavigation('foo', obs);
projectNavigation.initNavigation('es', obs);
}

return {
Expand Down Expand Up @@ -740,7 +740,7 @@ describe('breadcrumbs', () => {
{ text: 'custom1', href: '/custom1' },
{ text: 'custom2', href: '/custom1/custom2' },
]);
projectNavigation.initNavigation('foo', of(mockNavigation)); // init navigation
projectNavigation.initNavigation('es', of(mockNavigation)); // init navigation

const breadcrumbs = await firstValueFrom(projectNavigation.getProjectBreadcrumbs$());
expect(breadcrumbs).toHaveLength(4);
Expand Down Expand Up @@ -779,7 +779,7 @@ describe('getActiveNodes$()', () => {
expect(activeNodes).toEqual([]);

projectNavigation.initNavigation<any>(
'foo',
'es',
of({
body: [
{
Expand Down Expand Up @@ -835,7 +835,7 @@ describe('getActiveNodes$()', () => {
expect(activeNodes).toEqual([]);

projectNavigation.initNavigation<any>(
'foo',
'es',
of({
body: [
{
Expand Down Expand Up @@ -889,15 +889,15 @@ describe('getActiveNodes$()', () => {

describe('solution navigations', () => {
const solution1: SolutionNavigationDefinition<any> = {
id: 'solution1',
id: 'es',
title: 'Solution 1',
icon: 'logoSolution1',
homePage: 'discover',
navigationTree$: of({ body: [{ type: 'navItem', link: 'app1' }] }),
};

const solution2: SolutionNavigationDefinition<any> = {
id: 'solution2',
id: 'oblt',
title: 'Solution 2',
icon: 'logoSolution2',
homePage: 'app2',
Expand All @@ -906,7 +906,7 @@ describe('solution navigations', () => {
};

const solution3: SolutionNavigationDefinition<any> = {
id: 'solution3',
id: 'security',
title: 'Solution 3',
icon: 'logoSolution3',
homePage: 'discover',
Expand Down Expand Up @@ -943,30 +943,30 @@ describe('solution navigations', () => {
}

{
projectNavigation.updateSolutionNavigations({ 1: solution1, 2: solution2 });
projectNavigation.updateSolutionNavigations({ es: solution1, oblt: solution2 });

const solutionNavs = await lastValueFrom(
projectNavigation.getSolutionsNavDefinitions$().pipe(take(1))
);
expect(solutionNavs).toEqual({ 1: solution1, 2: solution2 });
expect(solutionNavs).toEqual({ es: solution1, oblt: solution2 });
}

{
// Test partial update
projectNavigation.updateSolutionNavigations({ 3: solution3 }, false);
projectNavigation.updateSolutionNavigations({ security: solution3 }, false);
const solutionNavs = await lastValueFrom(
projectNavigation.getSolutionsNavDefinitions$().pipe(take(1))
);
expect(solutionNavs).toEqual({ 1: solution1, 2: solution2, 3: solution3 });
expect(solutionNavs).toEqual({ es: solution1, oblt: solution2, security: solution3 });
}

{
// Test full replacement
projectNavigation.updateSolutionNavigations({ 4: solution3 }, true);
projectNavigation.updateSolutionNavigations({ security: solution3 }, true);
const solutionNavs = await lastValueFrom(
projectNavigation.getSolutionsNavDefinitions$().pipe(take(1))
);
expect(solutionNavs).toEqual({ 4: solution3 });
expect(solutionNavs).toEqual({ security: solution3 });
}
});

Expand All @@ -980,8 +980,8 @@ describe('solution navigations', () => {
expect(activeSolution).toBeNull();
}

projectNavigation.changeActiveSolutionNavigation('2'); // Set **before** the navs are registered
projectNavigation.updateSolutionNavigations({ 1: solution1, 2: solution2 });
projectNavigation.changeActiveSolutionNavigation('oblt'); // Set **before** the navs are registered
projectNavigation.updateSolutionNavigations({ es: solution1, oblt: solution2 });

{
const activeSolution = await lastValueFrom(
Expand All @@ -994,7 +994,7 @@ describe('solution navigations', () => {
expect(activeSolution).toEqual(rest);
}

projectNavigation.changeActiveSolutionNavigation('1'); // Set **after** the navs are registered
projectNavigation.changeActiveSolutionNavigation('es'); // Set **after** the navs are registered

{
const activeSolution = await lastValueFrom(
Expand Down Expand Up @@ -1027,7 +1027,7 @@ describe('solution navigations', () => {

{
const fooSolution: SolutionNavigationDefinition<any> = {
id: 'fooSolution',
id: 'es',
title: 'Foo solution',
icon: 'logoSolution',
homePage: 'discover',
Expand All @@ -1053,8 +1053,8 @@ describe('solution navigations', () => {
}),
};

projectNavigation.changeActiveSolutionNavigation('foo');
projectNavigation.updateSolutionNavigations({ foo: fooSolution });
projectNavigation.changeActiveSolutionNavigation('es');
projectNavigation.updateSolutionNavigations({ es: fooSolution });

projectNavigation.setPanelSelectedNode('link2'); // Set the selected node using its id

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {
NavigationTreeDefinition,
SolutionNavigationDefinitions,
CloudLinks,
SolutionId,
} from '@kbn/core-chrome-browser';
import type { InternalHttpStart } from '@kbn/core-http-browser-internal';
import {
Expand Down Expand Up @@ -86,9 +87,9 @@ export class ProjectNavigationService {
private readonly solutionNavDefinitions$ = new BehaviorSubject<SolutionNavigationDefinitions>({});
// As the active definition **id** and the definitions are set independently, one before the other without
// any guarantee of order, we need to store the next active definition id in a separate BehaviorSubject
private readonly nextSolutionNavDefinitionId$ = new BehaviorSubject<string | null>(null);
private readonly nextSolutionNavDefinitionId$ = new BehaviorSubject<SolutionId | null>(null);
// The active solution navigation definition id that has been initiated and is currently active
private readonly activeSolutionNavDefinitionId$ = new BehaviorSubject<string | null>(null);
private readonly activeSolutionNavDefinitionId$ = new BehaviorSubject<SolutionId | null>(null);
private readonly location$ = new BehaviorSubject<Location>(createLocation('/'));
private deepLinksMap$: Observable<Record<string, ChromeNavLink>> = of({});
private cloudLinks$ = new BehaviorSubject<CloudLinks>({});
Expand Down Expand Up @@ -138,7 +139,7 @@ export class ProjectNavigationService {
return this.projectName$.asObservable();
},
initNavigation: <LinkId extends AppDeepLinkId = AppDeepLinkId>(
id: string,
id: SolutionId,
navTreeDefinition$: Observable<NavigationTreeDefinition<LinkId>>
) => {
this.initNavigation(id, navTreeDefinition$);
Expand Down Expand Up @@ -202,7 +203,7 @@ export class ProjectNavigationService {
* @param id Id for the navigation tree definition
* @param navTreeDefinition$ The navigation tree definition
*/
private initNavigation(id: string, navTreeDefinition$: Observable<NavigationTreeDefinition>) {
private initNavigation(id: SolutionId, navTreeDefinition$: Observable<NavigationTreeDefinition>) {
if (this.activeSolutionNavDefinitionId$.getValue() === id) return;

if (this.navigationChangeSubscription) {
Expand All @@ -220,7 +221,7 @@ export class ProjectNavigationService {
.pipe(
takeUntil(this.stop$),
map(([def, deepLinksMap, cloudLinks]) => {
return parseNavigationTree(def, {
return parseNavigationTree(id, def, {
deepLinks: deepLinksMap,
cloudLinks,
});
Expand Down Expand Up @@ -382,7 +383,7 @@ export class ProjectNavigationService {
this.projectHome$.next(homeHref);
}

private changeActiveSolutionNavigation(id: string | null) {
private changeActiveSolutionNavigation(id: SolutionId | null) {
if (this.nextSolutionNavDefinitionId$.getValue() === id) return;
this.nextSolutionNavDefinitionId$.next(id);
}
Expand All @@ -400,7 +401,7 @@ export class ProjectNavigationService {
if (!definitions[id]) return null;

// We strip out the sideNavComponent from the definition as it should only be used internally
const { sideNavComponent, ...definition } = definitions[id];
const { sideNavComponent, ...definition } = definitions[id]!;
return definition;
})
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import type {
CloudLinkId,
CloudLinks,
ItemDefinition,
SolutionId,
} from '@kbn/core-chrome-browser/src';
import type { Location } from 'history';
import type { MouseEventHandler } from 'react';
Expand Down Expand Up @@ -364,6 +365,7 @@ const isRecentlyAccessedDefinition = (
};

export const parseNavigationTree = (
id: SolutionId,
navigationTreeDef: NavigationTreeDefinition,
{ deepLinks, cloudLinks }: { deepLinks: Record<string, ChromeNavLink>; cloudLinks: CloudLinks }
): {
Expand All @@ -376,7 +378,7 @@ export const parseNavigationTree = (
const navigationTree: ChromeProjectNavigationNode[] = [];

// Contains UI layout information (body, footer) and render "special" blocks like recently accessed.
const navigationTreeUI: NavigationTreeDefinitionUI = { body: [] };
const navigationTreeUI: NavigationTreeDefinitionUI = { id, body: [] };

const initNodeAndChildren = (
node: GroupDefinition | ItemDefinition | NodeDefinition,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type {
NavigationTreeDefinitionUI,
CloudURLs,
SolutionNavigationDefinitions,
SolutionId,
} from '@kbn/core-chrome-browser';
import type { Observable } from 'rxjs';

Expand Down Expand Up @@ -66,7 +67,7 @@ export interface InternalChromeStart extends ChromeStart {
Id extends string = string,
ChildrenId extends string = Id
>(
id: string,
id: SolutionId,
navigationTree$: Observable<NavigationTreeDefinition<LinkId, Id, ChildrenId>>
): void;

Expand Down Expand Up @@ -117,6 +118,6 @@ export interface InternalChromeStart extends ChromeStart {
* @param id The id of the active solution navigation. If `null` is provided, the solution navigation
* will be replaced with the legacy Kibana navigation.
*/
changeActiveSolutionNavigation(id: string | null): void;
changeActiveSolutionNavigation(id: SolutionId | null): void;
};
}
1 change: 1 addition & 0 deletions packages/core/chrome/core-chrome-browser/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,5 @@ export type {
SolutionNavigationDefinitions,
EuiSideNavItemTypeEnhanced,
RenderAs,
SolutionId,
} from './src';
1 change: 1 addition & 0 deletions packages/core/chrome/core-chrome-browser/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export type {
PanelSelectedNode,
AppDeepLinkId,
AppId,
SolutionId,
CloudLinkId,
CloudLink,
CloudLinks,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ import type { AppId as SharedApp, DeepLinkId as SharedLink } from '@kbn/deeplink
import type { ChromeNavLink } from './nav_links';
import type { ChromeRecentlyAccessedHistoryItem } from './recently_accessed';

export type SolutionId = 'es' | 'oblt' | 'security';

/** @public */
export type AppId =
| DevToolsApp
Expand Down Expand Up @@ -414,6 +416,7 @@ export interface NavigationTreeDefinition<
* with their corresponding "deepLink"...)
*/
export interface NavigationTreeDefinitionUI {
id: SolutionId;
body: Array<ChromeProjectNavigationNode | RecentlyAccessedDefinition>;
footer?: Array<ChromeProjectNavigationNode | RecentlyAccessedDefinition>;
}
Expand All @@ -429,7 +432,7 @@ export interface NavigationTreeDefinitionUI {

export interface SolutionNavigationDefinition<LinkId extends AppDeepLinkId = AppDeepLinkId> {
/** Unique id for the solution navigation. */
id: string;
id: SolutionId;
/** Title for the solution navigation. */
title: string;
/** The navigation tree definition */
Expand All @@ -442,9 +445,9 @@ export interface SolutionNavigationDefinition<LinkId extends AppDeepLinkId = App
homePage?: LinkId;
}

export interface SolutionNavigationDefinitions {
[id: string]: SolutionNavigationDefinition;
}
export type SolutionNavigationDefinitions = {
[id in SolutionId]?: SolutionNavigationDefinition;
};

/**
* Temporary helper interface while we have to maintain both the legacy side navigation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ describe('Active node', () => {
];

const { findByTestId } = renderNavigation({
navTreeDef: of({ body: navigationBody }),
navTreeDef: of({ id: 'es', body: navigationBody }),
services: { activeNodes$: getActiveNodes$() },
});

Expand Down
Loading

0 comments on commit 89fe548

Please sign in to comment.