-
Notifications
You must be signed in to change notification settings - Fork 8.6k
[SideNav] Make logo clickable and minor improvements #230392
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
Changes from all commits
ad26d69
e182493
542217b
3acaf2f
2958231
a626754
33afbfa
c30946b
795d603
2e20844
fc0310b
7cee80c
93facd7
0366217
780086c
dcba4db
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,17 +20,16 @@ import type { | |
| NavigationStructure, | ||
| SecondaryMenuItem, | ||
| SecondaryMenuSection, | ||
| SideNavLogo, | ||
| } from '@kbn/core-chrome-navigation/types'; | ||
|
|
||
| import { isActiveFromUrl } from '@kbn/shared-ux-chrome-navigation/src/utils'; | ||
| import { AppDeepLinkIdToIcon } from './hack_icons_mappings'; | ||
|
|
||
| export interface NavigationItems { | ||
| logoItem: LogoItem; | ||
| logoItem: SideNavLogo; | ||
| navItems: NavigationStructure; | ||
| } | ||
|
|
||
| export interface LogoItem { | ||
| logoType: string; | ||
| label: string; | ||
| activeItemId?: string; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -66,6 +65,18 @@ export const toNavigationItems = ( | |
| let primaryNodes: ChromeProjectNavigationNode[] = []; | ||
| let footerNodes: ChromeProjectNavigationNode[] = []; | ||
|
|
||
| let deepestActiveItemId: string | undefined; | ||
| let currentActiveItemIdLevel = -1; | ||
|
|
||
| const maybeMarkActive = (navNode: ChromeProjectNavigationNode, level: number) => { | ||
| if (deepestActiveItemId == null || currentActiveItemIdLevel < level) { | ||
| if (isActiveFromUrl(navNode.path, activeNodes, false)) { | ||
| deepestActiveItemId = navNode.id; | ||
| currentActiveItemIdLevel = level; | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| if (navigationTree.body.length === 1) { | ||
| const firstNode = navigationTree.body[0]; | ||
| if (!isRecentlyAccessedDefinition(firstNode)) { | ||
|
|
@@ -88,8 +99,18 @@ export const toNavigationItems = ( | |
| ); | ||
| } | ||
|
|
||
| const logoItem: LogoItem = { | ||
| logoType: warnIfMissing(logoNode, 'icon', 'logoKibana') as string, | ||
| if (logoNode) { | ||
| maybeMarkActive(logoNode, 0); | ||
| } else { | ||
| warnOnce( | ||
| 'Navigation tree is missing a logo node. The first level should contain a logo node with solution logo, name and home page href.' | ||
| ); | ||
| } | ||
|
|
||
| const logoItem: SideNavLogo = { | ||
| href: warnIfMissing(logoNode, 'href', '/missing-href-😭'), | ||
| iconType: warnIfMissing(logoNode, 'icon', 'logoKibana') as string, | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahhh, fair, I must've copied the line but |
||
| id: warnIfMissing(logoNode, 'id', 'kibana'), | ||
| label: warnIfMissing(logoNode, 'title', 'Kibana'), | ||
| }; | ||
|
|
||
|
|
@@ -167,6 +188,7 @@ export const toNavigationItems = ( | |
| label: null, | ||
| items: navNode.children.map((child) => { | ||
| warnUnsupportedNavNodeOptions(child); | ||
| maybeMarkActive(child, 2); | ||
| return { | ||
| id: child.id, | ||
| label: warnIfMissing(child, 'title', 'Missing Title 😭'), | ||
|
|
@@ -192,6 +214,7 @@ export const toNavigationItems = ( | |
| .filter((subChild) => subChild.sideNavStatus !== 'hidden') | ||
| .map((subChild) => { | ||
| warnUnsupportedNavNodeOptions(subChild); | ||
| maybeMarkActive(subChild, 2); | ||
| return { | ||
| id: subChild.id, | ||
| label: warnIfMissing(subChild, 'title', 'Missing Title 😭'), | ||
|
|
@@ -238,6 +261,8 @@ export const toNavigationItems = ( | |
| itemHref = warnIfMissing(navNode, 'href', 'missing-href-😭'); | ||
| } | ||
|
|
||
| maybeMarkActive(navNode, 1); | ||
|
|
||
| return { | ||
| id: navNode.id, | ||
| label: warnIfMissing(navNode, 'title', 'Missing Title 😭'), | ||
|
|
@@ -263,7 +288,7 @@ export const toNavigationItems = ( | |
| ); | ||
| } | ||
|
|
||
| return { logoItem, navItems: { primaryItems, footerItems } }; | ||
| return { logoItem, navItems: { primaryItems, footerItems }, activeItemId: deepestActiveItemId }; | ||
| }; | ||
|
|
||
| // ===================== | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,5 +7,12 @@ | |
| * License v3.0 only", or the "Server Side Public License, v 1". | ||
| */ | ||
|
|
||
| export { Navigation } from './src/components/navigation'; | ||
| export { Navigation, type NavigationProps } from './src/components/navigation'; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Dosant this is totally valid, I'm just wondering, couldn't we use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was fixing the storybook issue. I'm not sure if this was necessary.
Isn't it still problematic because it would reference private type? if not, then I am good with typeof
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, I remember seeing that one. It was complaining about a private type. Then yup, it's best if we export 😄 |
||
| export { useNavigation } from './src/hooks/use_navigation'; | ||
| export type { | ||
| MenuItem, | ||
| SecondaryMenuItem, | ||
| SecondaryMenuSection, | ||
| NavigationStructure, | ||
| SideNavLogo, | ||
| } from './types'; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😢