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

[core] Fix docs segments and make navigation item segments optional #3838

Merged
merged 18 commits into from
Aug 5, 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
2 changes: 1 addition & 1 deletion docs/pages/toolpad/core/api/app-provider.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"navigation": {
"type": {
"name": "arrayOf",
"description": "Array&lt;{ action?: node, children?: Array&lt;object<br>&#124;&nbsp;{ kind: 'header', title: string }<br>&#124;&nbsp;{ kind: 'divider' }&gt;, icon?: node, kind?: 'page', segment: string, title?: string }<br>&#124;&nbsp;{ kind: 'header', title: string }<br>&#124;&nbsp;{ kind: 'divider' }&gt;"
"description": "Array&lt;{ action?: node, children?: Array&lt;object<br>&#124;&nbsp;{ kind: 'header', title: string }<br>&#124;&nbsp;{ kind: 'divider' }&gt;, icon?: node, kind?: 'page', segment?: string, title?: string }<br>&#124;&nbsp;{ kind: 'header', title: string }<br>&#124;&nbsp;{ kind: 'divider' }&gt;"
},
"default": "[]"
},
Expand Down
4 changes: 2 additions & 2 deletions packages/toolpad-core/src/AppProvider/AppProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export interface Branding {

export interface NavigationPageItem {
kind?: 'page';
segment: string;
segment?: string;
title?: string;
icon?: React.ReactNode;
action?: React.ReactNode;
Expand Down Expand Up @@ -210,7 +210,7 @@ AppProvider.propTypes /* remove-proptypes */ = {
),
icon: PropTypes.node,
kind: PropTypes.oneOf(['page']),
segment: PropTypes.string.isRequired,
segment: PropTypes.string,
title: PropTypes.string,
}),
PropTypes.shape({
Expand Down
39 changes: 36 additions & 3 deletions packages/toolpad-core/src/DashboardLayout/DashboardLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ import {
} from '../shared/context';
import type { Navigation, NavigationPageItem } from '../AppProvider';
import { ToolpadLogo } from './ToolpadLogo';
import { getItemTitle, hasSelectedNavigationChildren } from '../shared/navigation';
import {
getItemTitle,
getPageItemFullPath,
hasSelectedNavigationChildren,
} from '../shared/navigation';
import { useApplicationTitle } from '../shared/branding';

const DRAWER_WIDTH = 320; // px
Expand Down Expand Up @@ -144,13 +148,17 @@ interface DashboardSidebarSubNavigationProps {
basePath?: string;
depth?: number;
onSidebarItemClick?: (item: NavigationPageItem) => void;
validatedItemIds: Set<string>;
uniqueItemPaths: Set<string>;
}

function DashboardSidebarSubNavigation({
subNavigation,
basePath = '',
depth = 0,
onSidebarItemClick,
validatedItemIds,
uniqueItemPaths,
}: DashboardSidebarSubNavigationProps) {
const routerContext = React.useContext(RouterContext);

Expand Down Expand Up @@ -221,7 +229,7 @@ function DashboardSidebarSubNavigation({
);
}

const navigationItemFullPath = `${basePath}${basePath && !navigationItem.segment ? '' : '/'}${navigationItem.segment ?? ''}`;
const navigationItemFullPath = getPageItemFullPath(basePath, navigationItem);

const navigationItemId = `${depth}-${navigationItemIndex}`;

Expand All @@ -236,7 +244,7 @@ function DashboardSidebarSubNavigation({
const listItem = (
<ListItem sx={{ pt: 0, pb: 0 }}>
<NavigationListItemButton
selected={pathname === navigationItemFullPath}
selected={pathname === navigationItemFullPath && !navigationItem.children}
{...(navigationItem.children
? {
onClick: handleOpenFolderClick(navigationItemId),
Expand Down Expand Up @@ -269,6 +277,16 @@ function DashboardSidebarSubNavigation({
</ListItem>
);

if (process.env.NODE_ENV !== 'production' && !validatedItemIds.has(navigationItemId)) {
if (!uniqueItemPaths.has(navigationItemFullPath)) {
uniqueItemPaths.add(navigationItemFullPath);
} else {
console.warn(`Duplicate path in navigation: ${navigationItemFullPath}`);
}

validatedItemIds.add(navigationItemId);
}

return (
<React.Fragment key={navigationItemId}>
{listItem}
Expand All @@ -280,6 +298,8 @@ function DashboardSidebarSubNavigation({
basePath={navigationItemFullPath}
depth={depth + 1}
onSidebarItemClick={onSidebarItemClick}
validatedItemIds={validatedItemIds}
uniqueItemPaths={uniqueItemPaths}
/>
</Collapse>
) : null}
Expand Down Expand Up @@ -317,6 +337,9 @@ function DashboardLayout(props: DashboardLayoutProps) {

const [isMobileNavigationOpen, setIsMobileNavigationOpen] = React.useState(false);

const validatedItemIdsRef = React.useRef(new Set<string>());
const uniqueItemPathsRef = React.useRef(new Set<string>());

const handleSetMobileNavigationOpen = React.useCallback(
(newOpen: boolean) => () => {
setIsMobileNavigationOpen(newOpen);
Expand All @@ -334,13 +357,22 @@ function DashboardLayout(props: DashboardLayoutProps) {
}
}, []);

// If useEffect was used, the reset would also happen on the client render after SSR which we don't need
React.useMemo(() => {
validatedItemIdsRef.current = new Set();
uniqueItemPathsRef.current = new Set();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [navigation]);

const drawerContent = (
<React.Fragment>
<Toolbar />
<Box component="nav" sx={{ overflow: 'auto', pt: navigation[0]?.kind === 'header' ? 0 : 2 }}>
<DashboardSidebarSubNavigation
subNavigation={navigation}
onSidebarItemClick={handleNavigationItemClick}
validatedItemIds={validatedItemIdsRef.current}
uniqueItemPaths={uniqueItemPathsRef.current}
/>
</Box>
</React.Fragment>
Expand Down Expand Up @@ -383,6 +415,7 @@ function DashboardLayout(props: DashboardLayoutProps) {
sx={{
color: (theme) => (theme.vars ?? theme).palette.primary.main,
fontWeight: '700',
ml: 0.5,
}}
>
{applicationTitle}
Expand Down
12 changes: 7 additions & 5 deletions packages/toolpad-core/src/PageContainer/PageContainer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function createPageLookup(
): Map<string, BreadCrumbItem[]> {
const result = new Map<string, BreadCrumbItem[]>();

const resolveSegment = (segment: string) => `${base}${segment ? `/${segment}` : ''}` || '/';
const resolveSegment = (segment?: string) => `${base}${segment ? `/${segment}` : ''}` || '/';

const root = navigation.find((item) => isRootPage(item)) as NavigationPageItem | undefined;
const rootCrumb = root ? { path: resolveSegment(''), ...root } : undefined;
Expand All @@ -45,9 +45,11 @@ function createPageLookup(
continue;
}

const isNonProdEnv = process.env.NODE_ENV !== 'production';

const path = resolveSegment(item.segment);
if (result.has(path)) {
throw new Error(`Duplicate path in navigation: ${path}`);
if (isNonProdEnv && result.has(path)) {
console.warn(`Duplicate path in navigation: ${path}`);
}

const itemCrumb: BreadCrumbItem = { path, ...item };
Expand All @@ -63,8 +65,8 @@ function createPageLookup(
if (item.children) {
const childrenLookup = createPageLookup(item.children, navigationSegments, path);
for (const [childPath, childItems] of childrenLookup) {
if (result.has(childPath)) {
throw new Error(`Duplicate path in navigation: ${childPath}`);
if (isNonProdEnv && result.has(childPath)) {
console.warn(`Duplicate path in navigation: ${childPath}`);
}
result.set(childPath, childItems);
}
Expand Down
13 changes: 10 additions & 3 deletions packages/toolpad-core/src/shared/navigation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,20 @@ export const isPageItem = (item: NavigationItem): item is NavigationPageItem =>
getItemKind(item) === 'page';

export const getItemTitle = (item: NavigationPageItem | NavigationSubheaderItem) => {
return isPageItem(item) ? (item.title ?? item.segment) : item.title;
return isPageItem(item) ? (item.title ?? item.segment ?? '') : item.title;
};

export function getPageItemFullPath(basePath: string, navigationItem: NavigationPageItem) {
return `${basePath}${basePath && !navigationItem.segment ? '' : '/'}${navigationItem.segment ?? ''}`;
}

export function hasSelectedNavigationChildren(
navigationItem: NavigationItem,
basePath: string,
pathname: string,
): boolean {
if (isPageItem(navigationItem) && navigationItem.children) {
const navigationItemFullPath = `${basePath}${basePath && !navigationItem.segment ? '' : '/'}${navigationItem.segment ?? ''}`;
const navigationItemFullPath = getPageItemFullPath(basePath, navigationItem);

return navigationItem.children.some((nestedNavigationItem) => {
if (!isPageItem(nestedNavigationItem)) {
Expand All @@ -30,7 +34,10 @@ export function hasSelectedNavigationChildren(
);
}

const nestedNavigationItemFullPath = `${navigationItemFullPath}${navigationItemFullPath && !nestedNavigationItem.segment ? '' : '/'}${nestedNavigationItem.segment ?? ''}`;
const nestedNavigationItemFullPath = getPageItemFullPath(
navigationItemFullPath,
nestedNavigationItem,
);

return nestedNavigationItemFullPath === pathname;
});
Expand Down
1 change: 0 additions & 1 deletion playground/nextjs/src/app/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ const NAVIGATION: Navigation = [
title: 'Main items',
},
{
segment: '',
title: 'Dashboard',
icon: <DashboardIcon />,
},
Expand Down
Loading