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.

Sign up for GitHub

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

ms2/layoutmenu: show selected item #310

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

ms2/layoutmenu: show selected item #310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
75628f4
feat(ms2/layoutmenu): show selected item
FelipeTrost Jun 20, 2024
2546b21
Merge branch 'main' into ms2/selected-page-in-menu
FelipeTrost Jun 24, 2024
cb986d0
Merge branch 'main' into ms2/selected-page-in-menu
FelipeTrost Jun 28, 2024
6727611
Merge branch 'main' into ms2/selected-page-in-menu
FelipeTrost Jul 7, 2024
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
29 changes: 27 additions & 2 deletions src/management-system-v2/app/(dashboard)/[environmentId]/layout-client.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import cn from 'classnames';
import Link from 'next/link';
import { signIn, useSession } from 'next-auth/react';
import { create } from 'zustand';
import { useRouter } from 'next/navigation';
import { usePathname, useRouter, useSearchParams } from 'next/navigation';
import { Environment } from '@/lib/data/environment-schema';
import UserAvatar from '@/components/user-avatar';
import { spaceURL } from '@/lib/utils';
Expand Down Expand Up @@ -46,6 +46,7 @@ const Layout: FC<
}) => {
const session = useSession();
const router = useRouter();
const pathname = usePathname();

const mobileDrawerOpen = useLayoutMobileDrawer((state) => state.open);
const setMobileDrawerOpen = useLayoutMobileDrawer((state) => state.set);
Expand All @@ -59,7 +60,31 @@ const Layout: FC<
(item) => !(breakpoint.xs && item && 'type' in item && item.type === 'divider'),
);

const menu = <Menu theme="light" mode="inline" items={layoutMenuItems} />;
let activeKey: string | undefined;
for (const layoutItem of layoutMenuItems) {
if (
layoutItem === null ||
!('type' in layoutItem) ||
layoutItem.type !== 'group' ||
!layoutItem.children
)
continue;

activeKey = layoutItem?.children.find((item) => {
return item?.key && pathname.includes(item.key as string);
})?.key as string;

if (activeKey) break;
}
Comment on lines +63 to +78
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could fail if we have a menu item like /executions as well as /executions/monitoring or sth. like that. If you want to do it that way, the check should be 1:1. Alternatively a context could be set through a Content component prop that gets read here, so we aren't limited by the url and still set it through the individual pages.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think the best way is through some form of url mathing, otherwise it would become to annoying to have to set a context value every time, and also there could be some issues with intercepting routes. What do you think of checking that the url ends in a string? This could also cause conflicts, but it is far less likely.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain that more, what do you mean with ends with a string? How would you make sure /executions/list or whatever could be at the end marks the /executions list item in that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the url is ".../executions/list", then url.endsWith("/executions") would be false, and url.endsWith("/executions/list") would be true

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I misunderstood then maybe. I though the intention was to always mark an item, even on sub-sites (like only executions is in the menu, so /executions/list would still mark executions item in menu).


const menu = (
<Menu
theme="light"
mode="inline"
items={layoutMenuItems}
selectedKeys={activeKey ? [activeKey] : []}
/>
);

return (
<UserSpacesContext.Provider value={userEnvironments}>
Expand Down
14 changes: 7 additions & 7 deletions src/management-system-v2/app/(dashboard)/[environmentId]/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,14 @@ const DashboardLayout = async ({

if (can('view', 'Process'))
children.push({
key: 'processes',
key: '/processes',
label: <Link href={spaceURL(activeEnvironment, `/processes`)}>Process List</Link>,
icon: <FileOutlined />,
});

if (can('view', 'Template'))
children.push({
key: 'templates',
key: '/templates',
label: <Link href={spaceURL(activeEnvironment, `/templates`)}>Templates</Link>,
icon: <ProfileOutlined />,
});
Expand All @@ -90,7 +90,7 @@ const DashboardLayout = async ({
const children: MenuProps['items'] = [];

children.push({
key: 'executions',
key: '/executions',
label: <Link href={spaceURL(activeEnvironment, `/executions`)}>Instances</Link>,
icon: <LuBoxes />,
});
Expand All @@ -110,7 +110,7 @@ const DashboardLayout = async ({

if (process.env.ENABLE_MACHINE_CONFIG) {
layoutMenuItems.push({
key: 'machine-config',
key: '/machine-config',
label: <Link href={spaceURL(activeEnvironment, `/machine-config`)}>Machine Config</Link>,
icon: <LuTable2 />,
});
Expand All @@ -130,14 +130,14 @@ const DashboardLayout = async ({

if (can('manage', 'User'))
children.push({
key: 'users',
key: '/users',
label: <Link href={spaceURL(activeEnvironment, `/iam/users`)}>Users</Link>,
icon: <UserOutlined />,
});

if (ability.can('manage', 'RoleMapping') || ability.can('manage', 'Role'))
children.push({
key: 'roles',
key: '/roles',
label: <Link href={spaceURL(activeEnvironment, `/iam/roles`)}>Roles</Link>,
icon: <UnlockOutlined />,
});
Expand All @@ -162,7 +162,7 @@ const DashboardLayout = async ({
type: 'group',
children: [
{
key: 'general-settings',
key: '/general-settings',
label: (
<Link href={spaceURL(activeEnvironment, `/general-settings`)}>General Settings</Link>
),
Expand Down