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

[8.x] [Stateful sidenav] Update feedback urls (#198143) #198480

Merged
merged 1 commit into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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