Skip to content
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
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -2949,6 +2949,7 @@ x-pack/solutions/observability/plugins/observability_shared/public/components/pr

# Shared UX
/src/core/packages/chrome @elastic/appex-sharedux
/src/core/packages/chrome/navigation @elastic/eui-team @elastic/appex-sharedux
/x-pack/platform/test/serverless/api_integration/test_suites/favorites @elastic/appex-sharedux # Assigned per https://github.com/elastic/kibana/pull/200985
/src/platform/test/api_integration/apis/short_url/**/*.ts @elastic/appex-sharedux # Assigned per https://github.com/elastic/kibana/pull/200209/files#r1846654156
/src/platform/test/api_integration/apis/unused_urls_task/**/*.ts @elastic/appex-sharedux # Assigned per https://github.com/elastic/kibana/pull/220138
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ const useNavigationItems = (
props: Pick<ChromeNavigationProps, 'navigationTree$' | 'navLinks$' | 'activeNodes$' | 'basePath'>
): NavigationItems | null => {
const state$ = useMemo(
() => combineLatest([props.navigationTree$, props.navLinks$, props.activeNodes$]),
[props.navigationTree$, props.navLinks$, props.activeNodes$]
() => combineLatest([props.navigationTree$, props.activeNodes$]),
[props.navigationTree$, props.activeNodes$]
);
const state = useObservable(state$);

Expand All @@ -106,8 +106,8 @@ const useNavigationItems = (

const memoizedItems = useMemo(() => {
if (!state) return null;
const [navigationTree, navLinks, activeNodes] = state;
return toNavigationItems(navigationTree, navLinks, activeNodes, panelStateManager);
const [navigationTree, activeNodes] = state;
return toNavigationItems(navigationTree, activeNodes, panelStateManager);
}, [state, panelStateManager]);

return memoizedItems;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ const createNavigationItems = (
tree: NavigationTreeDefinitionUI = navigationTree,
activeNodes: ChromeProjectNavigationNode[][] = []
) => {
return toNavigationItems(tree, [], activeNodes, mockPanelStateManager);
return toNavigationItems(tree, activeNodes, mockPanelStateManager);
};

beforeEach(() => {
Expand Down Expand Up @@ -96,7 +96,8 @@ describe('toNavigationItems', () => {
• No icon found for node \\"securityGroup:assets\\". Expected iconV2, icon, deepLink.euiIconType, deepLink.icon or a known deep link id. Using fallback icon \\"broom\\".
• No icon found for node \\"securityGroup:machineLearning\\". Expected iconV2, icon, deepLink.euiIconType, deepLink.icon or a known deep link id. Using fallback icon \\"broom\\".
• No icon found for node \\"stack_management\\". Expected iconV2, icon, deepLink.euiIconType, deepLink.icon or a known deep link id. Using fallback icon \\"broom\\".
• Accordion items are not supported in the new navigation. Flattening them \\"stack_management, monitoring, integrations\\" and dropping accordion node \\"node-2\\"."
• Accordion items are not supported in the new navigation. Flattening them \\"stack_management, monitoring, integrations\\" and dropping accordion node \\"node-2\\".
• ID \\"endpoints\\" is used 2 times in navigation items. Each navigation item must have a unique ID."
`);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
*/

import type {
ChromeNavLink,
ChromeProjectNavigationNode,
NavigationTreeDefinitionUI,
RecentlyAccessedDefinition,
Expand All @@ -28,6 +27,8 @@ import { isActiveFromUrl } from '@kbn/shared-ux-chrome-navigation/src/utils';
import { AppDeepLinkIdToIcon } from './known_icons_mappings';
import type { PanelStateManager } from './panel_state_manager';

const SKIP_WARNINGS = process.env.NODE_ENV === 'production';

export interface NavigationItems {
logoItem: SideNavLogo;
navItems: NavigationStructure;
Expand Down Expand Up @@ -59,9 +60,6 @@ export interface NavigationItems {
*/
export const toNavigationItems = (
navigationTree: NavigationTreeDefinitionUI,
// navLinks and activeNodes are not used yet, but are passed for future use
// they will be needed for isActive state management and possibly for rendering links
navLinks: Readonly<ChromeNavLink[]>,
activeNodes: ChromeProjectNavigationNode[][],
panelStateManager: PanelStateManager
): NavigationItems => {
Expand Down Expand Up @@ -322,8 +320,10 @@ export const toNavigationItems = (
);
}

// Check for duplicate icons
warnAboutDuplicateIcons(logoItem, primaryItems);
if (!SKIP_WARNINGS) {
warnAboutDuplicateIds(logoItem, primaryItems, footerItems);
warnAboutDuplicateIcons(logoItem, primaryItems, footerItems);
}

return {
logoItem,
Expand Down Expand Up @@ -376,6 +376,7 @@ function warnIfMissing<T extends { id: string }, K extends keyof T>(
const warnedMessages = new Set<string>();
let lastWarning = '';
function warnOnce(message: string) {
if (SKIP_WARNINGS) return;
if (!warnedMessages.has(message)) {
warnedMessages.add(message);
}
Expand Down Expand Up @@ -429,30 +430,84 @@ const isRecentlyAccessedDefinition = (
const filterEmpty = <T,>(arr: Array<T | null | undefined>): T[] =>
arr.filter((item) => item !== null && item !== undefined) as T[];

function warnAboutDuplicateIcons(logoItem: SideNavLogo, primaryItems: MenuItem[]) {
// Collect all items with icons
const itemsWithIcons = [logoItem, ...primaryItems]
.filter((item) => item.iconType && item.iconType !== FALLBACK_ICON)
.map((item) => ({ id: item.id, icon: String(item.iconType) }));

// Group items by icon and warn about duplicates
const iconGroups = new Map<string, string[]>();
itemsWithIcons.forEach(({ id, icon }) => {
const items = iconGroups.get(icon) || [];
iconGroups.set(icon, [...items, id]);
/**
* Generic function to detect and warn about duplicate values in navigation items.
* @param values - Array of values to check for duplicates
* @param formatWarning - Function to format the warning message for duplicates
*/
function warnAboutDuplicates(
values: string[],
formatWarning: (value: string, count: number) => string
) {
const valueGroups = new Map<string, number>();
values.forEach((value) => {
valueGroups.set(value, (valueGroups.get(value) || 0) + 1);
});

iconGroups.forEach((itemIds, iconType) => {
if (itemIds.length > 1) {
warnOnce(
`Icon "${iconType}" is used by multiple navigation items: ${itemIds.join(
', '
)}. Consider using unique icons for better UX.`
);
valueGroups.forEach((count, value) => {
if (count > 1) {
warnOnce(formatWarning(value, count));
}
});
}

function warnAboutDuplicateIcons(
logoItem: SideNavLogo,
primaryItems: MenuItem[],
footerItems: MenuItem[]
) {
if (SKIP_WARNINGS) return;
// Collect all items with icons (only logo + primary items, excluding fallback)
const icons = [logoItem, ...primaryItems, ...footerItems]
.filter(
(item) =>
item.iconType && item.iconType !== FALLBACK_ICON && typeof item.iconType === 'string'
)
.map((item) => String(item.iconType));

warnAboutDuplicates(
icons,
(icon) =>
`Icon "${icon}" is used by multiple navigation items. Consider using unique icons for better UX.`
);
}

function warnAboutDuplicateIds(
logoItem: SideNavLogo,
primaryItems: MenuItem[],
footerItems: MenuItem[]
) {
if (SKIP_WARNINGS) return;
// Collect all IDs from all items, including secondary menu items
let allIds: string[] = [logoItem.id];

// Helper to extract IDs from menu items including their secondary sections
const collectIds = (items: MenuItem[]) => {
items.forEach((item) => {
allIds.push(item.id);
if (item.sections) {
item.sections.forEach((section) => {
allIds.push(section.id);
section.items.forEach((secondaryItem) => {
allIds.push(secondaryItem.id);
});
});
}
});
};

collectIds(primaryItems);
collectIds(footerItems);

allIds = allIds.filter((id) => !id?.startsWith('node-')); // Filter out auto-generated IDs

warnAboutDuplicates(
allIds,
(id, count) =>
`ID "${id}" is used ${count} times in navigation items. Each navigation item must have a unique ID.`
);
}

const FALLBACK_ICON = 'broom' as const;
/**
* Finds an item href based on the last active item history for a panel opener.
Expand Down