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

[Stateful sidenav] Dynamic links #183157

Merged
merged 43 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
839663b
Move search nav definition to plugin
sebelga May 7, 2024
3b9bc09
Don't add the search default definition
sebelga May 7, 2024
9b300ff
Remove search solution nav package
sebelga May 7, 2024
71a47e9
Sort keys ascending for linting
sebelga May 8, 2024
e496442
Set the correct breadcrumb root even when no active tree yet
sebelga May 8, 2024
e5e29e1
Add nav tree definition from es plugin
sebelga May 9, 2024
62629cc
Allow onClick to be passed for a navigation node
sebelga May 10, 2024
226f757
Add handler to update the side nav definition
sebelga May 10, 2024
d0d6471
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine May 10, 2024
19f32d8
[CI] Auto-commit changed files from 'node scripts/generate codeowners'
kibanamachine May 13, 2024
2235919
Fix i18n issue
sebelga May 13, 2024
d1373e1
Fix TS issue & update jest snapshot
sebelga May 14, 2024
894f7d3
Add dynamic links for search applications
sebelga May 14, 2024
260ffbe
Always return nav items when chrome style is project
sebelga May 14, 2024
6c38c1b
Add dynamic links for collections
sebelga May 14, 2024
346ec18
Update NavigationSectionUI to better handle accordion states
sebelga May 14, 2024
17fbdc3
Add comments
sebelga May 14, 2024
0c9f375
Fix TS issues
sebelga May 14, 2024
eec7e67
Update comments
sebelga May 15, 2024
da4c2a4
Add tests for isActiveFromUrl
sebelga May 15, 2024
cae3e0c
Refactors
sebelga May 15, 2024
462d98f
Refactor navigation section UI
sebelga May 15, 2024
0cb0d40
Update jest snapshot
sebelga May 15, 2024
c884291
Fix jest tests
sebelga May 15, 2024
ec0294a
Merge remote-tracking branch 'upstream/main' into stateful-sidenav/dy…
sebelga May 15, 2024
8b1004e
Merge remote-tracking branch 'upstream/main' into stateful-sidenav/dy…
sebelga May 20, 2024
c698f07
Add jest tests for analytics page template
sebelga May 20, 2024
2c8accb
Add jest tests for applications page template
sebelga May 20, 2024
f8f7526
Add jest tests for indices dynamic links
sebelga May 20, 2024
e65f4e8
Add missing data test subj
sebelga May 20, 2024
d771d9d
Fix TS issue
sebelga May 20, 2024
774e214
Fix arrow test subject not set
sebelga May 20, 2024
87e5d10
Merge remote-tracking branch 'upstream/main' into stateful-sidenav/dy…
sebelga May 21, 2024
a4f2578
Load navigation tree async
sebelga May 21, 2024
d932274
Cleanup
sebelga May 21, 2024
3be9f6e
Refactor
sebelga May 21, 2024
ca28657
Update comments
sebelga May 21, 2024
67fca04
Add jest test for custom onClick
sebelga May 21, 2024
51f544a
Merge branch 'main' into stateful-sidenav/dynamic-links
sebelga May 21, 2024
f2590cf
Merge branch 'main' into stateful-sidenav/dynamic-links
sebelga May 21, 2024
891581c
Remove stopPropagation calls
sebelga May 22, 2024
3271e19
Remove unnecessary useEffect dependency
sebelga May 22, 2024
d27b060
Merge branch 'main' into stateful-sidenav/dynamic-links
sebelga May 22, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,6 @@ packages/kbn-shared-ux-utility @elastic/appex-sharedux
x-pack/plugins/observability_solution/slo @elastic/obs-ux-management-team
x-pack/packages/kbn-slo-schema @elastic/obs-ux-management-team
x-pack/plugins/snapshot_restore @elastic/kibana-management
packages/solution-nav/es @elastic/appex-sharedux @elastic/enterprise-search-frontend
packages/solution-nav/oblt @elastic/appex-sharedux @elastic/obs-ux-management-team
packages/kbn-some-dev-log @elastic/kibana-operations
packages/kbn-sort-package-json @elastic/kibana-operations
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -820,7 +820,6 @@
"@kbn/slo-plugin": "link:x-pack/plugins/observability_solution/slo",
"@kbn/slo-schema": "link:x-pack/packages/kbn-slo-schema",
"@kbn/snapshot-restore-plugin": "link:x-pack/plugins/snapshot_restore",
"@kbn/solution-nav-es": "link:packages/solution-nav/es",
"@kbn/solution-nav-oblt": "link:packages/solution-nav/oblt",
"@kbn/sort-predicates": "link:packages/kbn-sort-predicates",
"@kbn/spaces-plugin": "link:x-pack/plugins/spaces",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function buildBreadcrumbs({
activeNodes: ChromeProjectNavigationNode[][];
solutionNavigations?: {
definitions: SolutionNavigationDefinitions;
activeId: string;
activeId: string | null;
onChange: (id: string) => void;
};
}): ChromeProjectBreadcrumb[] {
Expand Down Expand Up @@ -106,7 +106,7 @@ function buildRootCrumb({
cloudLinks: CloudLinks;
solutionNavigations?: {
definitions: SolutionNavigationDefinitions;
activeId: string;
activeId: string | null;
onChange: (id: string) => void;
};
}): ChromeProjectBreadcrumb {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ describe('initNavigation()', () => {
"href": "/app/discover",
"id": "discover",
"isElasticInternalLink": false,
"onClick": undefined,
"path": "rootNav:analytics.discover",
"sideNavStatus": "visible",
"title": "DISCOVER",
Expand All @@ -346,6 +347,7 @@ describe('initNavigation()', () => {
"href": "/app/dashboards",
"id": "dashboards",
"isElasticInternalLink": false,
"onClick": undefined,
"path": "rootNav:analytics.dashboards",
"sideNavStatus": "visible",
"title": "DASHBOARDS",
Expand All @@ -364,6 +366,7 @@ describe('initNavigation()', () => {
"href": "/app/visualize",
"id": "visualize",
"isElasticInternalLink": false,
"onClick": undefined,
"path": "rootNav:analytics.visualize",
"sideNavStatus": "visible",
"title": "VISUALIZE",
Expand All @@ -374,6 +377,7 @@ describe('initNavigation()', () => {
"icon": "stats",
"id": "rootNav:analytics",
"isElasticInternalLink": false,
"onClick": undefined,
"path": "rootNav:analytics",
"renderAs": "accordion",
"sideNavStatus": "visible",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ export class ProjectNavigationService {
chromeBreadcrumbs$,
this.projectName$,
this.solutionNavDefinitions$,
this.nextSolutionNavDefinitionId$,
this.activeSolutionNavDefinitionId$,
this.cloudLinks$,
]).pipe(
Expand All @@ -172,12 +173,13 @@ export class ProjectNavigationService {
chromeBreadcrumbs,
projectName,
solutionNavDefinitions,
nextSolutionNavDefinitionId,
activeSolutionNavDefinitionId,
cloudLinks,
]) => {
const solutionNavigations =
Object.keys(solutionNavDefinitions).length > 0 &&
activeSolutionNavDefinitionId !== null
(nextSolutionNavDefinitionId !== null || activeSolutionNavDefinitionId !== null)
? {
definitions: solutionNavDefinitions,
activeId: activeSolutionNavDefinitionId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import type {
ItemDefinition,
} from '@kbn/core-chrome-browser/src';
import type { Location } from 'history';
import type { MouseEventHandler } from 'react';
import { getPresets } from './navigation_presets';

const wrapIdx = (index: number): string => `[${index}]`;
Expand Down Expand Up @@ -212,15 +213,17 @@ function getNodeStatus(
deepLink,
cloudLink,
sideNavStatus,
onClick,
}: {
link?: string;
deepLink?: ChromeNavLink;
cloudLink?: CloudLinkId;
sideNavStatus?: SideNavNodeStatus;
onClick?: MouseEventHandler<HTMLButtonElement | HTMLElement>;
},
{ cloudLinks }: { cloudLinks: CloudLinks }
): SideNavNodeStatus | 'remove' {
if (link && !deepLink) {
if (link && !deepLink && !onClick) {
// If a link is provided, but no deepLink is found, don't render anything
return 'remove';
}
Expand Down Expand Up @@ -259,7 +262,7 @@ function validateNodeProps<
LinkId extends AppDeepLinkId = AppDeepLinkId,
Id extends string = string,
ChildrenId extends string = Id
>({ id, link, href, cloudLink, renderAs }: NodeDefinition<LinkId, Id, ChildrenId>) {
>({ id, link, href, cloudLink, renderAs, onClick }: NodeDefinition<LinkId, Id, ChildrenId>) {
if (link && cloudLink) {
throw new Error(
`[Chrome navigation] Error in node [${id}]. Only one of "link" or "cloudLink" can be provided.`
Expand All @@ -275,9 +278,9 @@ function validateNodeProps<
`[Chrome navigation] Error in node [${id}]. If renderAs is set to "panelOpener", a "link" must also be provided.`
);
}
if (renderAs === 'item' && !link) {
if (renderAs === 'item' && !link && !onClick) {
throw new Error(
`[Chrome navigation] Error in node [${id}]. If renderAs is set to "item", a "link" must also be provided.`
`[Chrome navigation] Error in node [${id}]. If renderAs is set to "item", a "link" or "onClick" must also be provided.`
);
}
}
Expand All @@ -302,14 +305,15 @@ const initNavNode = <
): ChromeProjectNavigationNode | null => {
validateNodeProps(node);

const { cloudLink, link, children, ...navNodeFromProps } = node;
const { cloudLink, link, children, onClick, ...navNodeFromProps } = node;
const deepLink = link !== undefined ? deepLinks[link] : undefined;
const sideNavStatus = getNodeStatus(
{
link,
deepLink,
cloudLink,
sideNavStatus: navNodeFromProps.sideNavStatus,
onClick,
},
{ cloudLinks }
);
Expand All @@ -324,14 +328,15 @@ const initNavNode = <
const href = isElasticInternalLink ? cloudLinks[cloudLink]?.href : node.href;
const path = parentNodePath ? `${parentNodePath}.${id}` : id;

if (href && !isAbsoluteLink(href)) {
if (href && !isAbsoluteLink(href) && !onClick) {
throw new Error(`href must be an absolute URL. Node id [${id}].`);
}

const navNode: ChromeProjectNavigationNode = {
...navNodeFromProps,
id,
href: getNavigationNodeHref({ href, deepLink }),
onClick,
path,
title,
deepLink,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export const getSolutionNavSwitcherBreadCrumb = ({
cloudLinks,
}: {
definitions: SolutionNavigationDefinitions;
activeId: string;
activeId: string | null;
onChange: (id: string) => void;
cloudLinks: CloudLinks;
}): ChromeProjectBreadcrumb => {
Expand Down
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 @@ -56,4 +56,5 @@ export type {
NavigationTreeDefinitionUI,
SolutionNavigationDefinition,
SolutionNavigationDefinitions,
EuiSideNavItemTypeEnhanced,
} 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 @@ -55,4 +55,5 @@ export type {
NavigationTreeDefinitionUI,
SolutionNavigationDefinition,
SolutionNavigationDefinitions,
EuiSideNavItemTypeEnhanced,
} from './project_navigation';
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
* Side Public License, v 1.
*/

import type { ComponentType } from 'react';
import type { ComponentType, MouseEventHandler } from 'react';
import type { Location } from 'history';
import type { EuiThemeSizes, IconType } from '@elastic/eui';
import type { EuiSideNavItemType, EuiThemeSizes, IconType } from '@elastic/eui';
import type { Observable } from 'rxjs';
import type { AppId as DevToolsApp, DeepLinkId as DevToolsLink } from '@kbn/deeplinks-devtools';
import type {
Expand Down Expand Up @@ -128,6 +128,10 @@ interface NodeDefinitionBase {
* href for absolute links only. Internal links should use "link".
*/
href?: string;
/**
* Custom handler to execute when clicking on the node. This handler takes precedence over the "link" or "href" props.
*/
onClick?: MouseEventHandler<HTMLButtonElement | HTMLElement>;
/**
* Optional status to indicate if the breadcrumb should be hidden when this node is active.
* @default 'visible'
Expand Down Expand Up @@ -434,3 +438,15 @@ export interface SolutionNavigationDefinition<LinkId extends AppDeepLinkId = App
export interface SolutionNavigationDefinitions {
[id: string]: SolutionNavigationDefinition;
}

/**
* Temporary helper interface while we have to maintain both the legacy side navigation
* and the new "solution view" one. The legacy uses EuiSideNavItemType and its properties are not fully compatible
* with the NodeDefinition. Solution teams declare their "classic" navigation using the EuiSideNavItemType.
* Converting those to the `NodeDefinition` require some additional props.
*/
export type EuiSideNavItemTypeEnhanced<T = unknown> = Omit<EuiSideNavItemType<T>, 'items'> & {
items?: Array<EuiSideNavItemTypeEnhanced<unknown>>;
iconToString?: string;
nameToString?: string;
};
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,40 @@ describe('builds navigation tree', () => {
}
});

test('should allow custom onClick handler for links', async () => {
const navigateToUrl = jest.fn();
const onClick = jest.fn();

const node: ChromeProjectNavigationNode = {
id: 'group1',
title: 'Group 1',
path: 'group1',
defaultIsCollapsed: false,
children: [
{
id: 'item1',
title: 'Item 1',
href: 'https://foo',
path: 'group1.item1',
onClick,
},
],
};

const { findByTestId } = renderNavigation({
navTreeDef: of({
body: [node],
}),
services: { navigateToUrl },
});

const navItem = await findByTestId(/nav-item-group1.item1\s/);
navItem.click();

expect(navigateToUrl).not.toHaveBeenCalled();
expect(onClick).toHaveBeenCalledWith(expect.objectContaining({ type: 'click' }));
});

test('should not render the group if it does not have children', async () => {
const navTree: NavigationTreeDefinitionUI = {
body: [
Expand Down