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

feat(dashboard): ability to locate new admin route under existing route #10587

Merged
merged 16 commits into from
Dec 19, 2024
Merged

feat(dashboard): ability to locate new admin route under existing route #10587

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
3bc1a52
feat: add ability to locate new admin route under existing route in s…
eugenepro2 Dec 12, 2024
8bbc242
refactor: add type NESTED_ROUTE_POSITIONS
eugenepro2 Dec 13, 2024
c4ece3e
refactor: validate if the value is one of the allowed
eugenepro2 Dec 13, 2024
be915c1
feat: check if parent item is a nested route under existing route
eugenepro2 Dec 13, 2024
fa79525
Merge branch 'develop' into feat/nested-navigation
kasperkristensen Dec 17, 2024
e85c982
fix: fix typo
eugenepro2 Dec 17, 2024
2c6ed68
Merge branch 'develop' into feat/nested-navigation
eugenepro2 Dec 17, 2024
71525ff
Merge branch 'develop' into feat/nested-navigation
kasperkristensen Dec 17, 2024
4efe68e
feat: update unit test
eugenepro2 Dec 17, 2024
902bc18
Merge branch 'develop' into feat/nested-navigation
kasperkristensen Dec 17, 2024
680065e
Merge branch 'develop' into feat/nested-navigation
kasperkristensen Dec 17, 2024
66d08de
fix: remove changes from docs
eugenepro2 Dec 18, 2024
add9062
Merge branch 'develop' into feat/nested-navigation
kasperkristensen Dec 18, 2024
d63c986
fix: remove empty line
eugenepro2 Dec 19, 2024
a2a87f2
Merge branch 'develop' into feat/nested-navigation
eugenepro2 Dec 19, 2024
6eb8ed6
Merge branch 'develop' into feat/nested-navigation
kasperkristensen Dec 19, 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
11 changes: 11 additions & 0 deletions packages/admin/admin-sdk/src/config/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,17 @@ export interface RouteConfig {
* An optional icon to display in the sidebar together with the label. If no label is provided, the icon will be ignored.
*/
icon?: ComponentType

/**
* The nested route to display under existing route in the sidebar.
*/
nested?:
| "/orders"
| "/products"
| "/inventory"
| "/customers"
| "/promotions"
| "/price-list"
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to move this to the admin-shared package both as a const:

export const NESTED_ROUTE_POSITIONS = ["/orders", ...] as const

And as a type:

export type NestedRoutePosition = (typeof NESTED_ROUTE_POSITIONS)[number]

We should then use the const to validate that the user has passed a valid nested value in generate-menu-items.ts. And use the type in this file, and in the dashboard project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really good point 🚀. Added type.

}

export type CustomFormField<
Expand Down
72 changes: 33 additions & 39 deletions packages/admin/admin-vite-plugin/src/routes/generate-menu-items.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,17 @@ import {
} from "../utils"
import { getRoute } from "./helpers"

type RouteConfig = {
eugenepro2 marked this conversation as resolved.
Show resolved Hide resolved
label: boolean
icon: boolean
nested?: string
}

type MenuItem = {
icon?: string
label: string
path: string
nested?: string
}

type MenuItemResult = {
Expand All @@ -32,13 +39,10 @@ export async function generateMenuItems(sources: Set<string>) {
const files = await getFilesFromSources(sources)
const results = await getMenuItemResults(files)

const imports = results.map((result) => result.import).flat()
const imports = results.map((result) => result.import)
const code = generateCode(results)

return {
imports,
code,
}
return { imports, code }
}

function generateCode(results: MenuItemResult[]): string {
Expand All @@ -53,10 +57,12 @@ function generateCode(results: MenuItemResult[]): string {
}

function formatMenuItem(route: MenuItem): string {
const { label, icon, path, nested } = route
return `{
label: ${route.label},
icon: ${route.icon ? route.icon : "undefined"},
path: "${route.path}",
label: ${label},
icon: ${icon || "undefined"},
path: "${path}",
nested: ${nested ? `"${nested}"` : "undefined"}
}`
}

Expand Down Expand Up @@ -107,70 +113,58 @@ function generateImport(file: string, index: number): string {
}

function generateMenuItem(
config: { label: boolean; icon: boolean },
config: RouteConfig,
file: string,
index: number
): MenuItem {
const configName = generateRouteConfigName(index)
const routePath = getRoute(file)

return {
label: `${configName}.label`,
icon: config.icon ? `${configName}.icon` : undefined,
path: routePath,
path: getRoute(file),
nested: config.nested
}
}

async function getRouteConfig(
file: string
): Promise<{ label: boolean; icon: boolean } | null> {
async function getRouteConfig(file: string): Promise<RouteConfig | null> {
const code = await fs.readFile(file, "utf-8")

let ast: ParseResult<File> | null = null

try {
ast = parse(code, getParserOptions(file))
} catch (e) {
logger.error(`An error occurred while parsing the file.`, {
file,
error: e,
})
logger.error(`An error occurred while parsing the file.`, { file, error: e })
return null
}

let config: { label: boolean; icon: boolean } | null = null
let config: RouteConfig | null = null

try {
traverse(ast, {
ExportNamedDeclaration(path) {
const properties = getConfigObjectProperties(path)
if (!properties) return

if (!properties) {
return
}

const hasLabel = properties.some(
(prop) =>
isObjectProperty(prop) && isIdentifier(prop.key, { name: "label" })
const hasProperty = (name: string) => properties.some(
(prop) => isObjectProperty(prop) && isIdentifier(prop.key, { name })
)

if (!hasLabel) {
return
}
const hasLabel = hasProperty("label")
if (!hasLabel) return

const hasIcon = properties.some(
(prop) =>
isObjectProperty(prop) && isIdentifier(prop.key, { name: "icon" })
const nested = properties.find(
eugenepro2 marked this conversation as resolved.
Show resolved Hide resolved
(prop) => isObjectProperty(prop) && isIdentifier(prop.key, { name: "nested" })
)

config = { label: hasLabel, icon: hasIcon }
config = {
label: hasLabel,
icon: hasProperty("icon"),
nested: nested ? (nested as any).value.value : undefined
}
},
})
} catch (e) {
logger.error(`An error occurred while traversing the file.`, {
file,
error: e,
})
logger.error(`An error occurred while traversing the file.`, { file, error: e })
}

return config
Expand Down
15 changes: 14 additions & 1 deletion packages/admin/dashboard/src/components/layout/main-layout/main-layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,19 @@ const Searchbar = () => {
const CoreRouteSection = () => {
const coreRoutes = useCoreRoutes()

const { getMenu } = useDashboardExtension()

const menuItems = getMenu("coreExtensions")

menuItems.forEach((item) => {
if (item.nested) {
const route = coreRoutes.find((route) => route.to === item.nested)
if (route) {
route.items?.push(item)
}
}
})

return (
<nav className="flex flex-col gap-y-1 py-3">
<Searchbar />
Expand All @@ -298,7 +311,7 @@ const ExtensionRouteSection = () => {
const { t } = useTranslation()
const { getMenu } = useDashboardExtension()

const menuItems = getMenu("coreExtensions")
const menuItems = getMenu("coreExtensions").filter((item) => !item.nested)

if (!menuItems.length) {
return null
Expand Down
1 change: 1 addition & 0 deletions packages/admin/dashboard/src/components/layout/nav-item/nav-item.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export type INavItem = {
items?: NestedItemProps[]
type?: ItemType
from?: string
nested?: string
}

const BASE_NAV_LINK_CLASSES =
Expand Down
1 change: 1 addition & 0 deletions ...dmin/dashboard/src/extensions/dashboard-extension-manager/dashboard-extension-manager.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ export class DashboardExtensionManager {
to: item.path,
icon: item.icon ? <item.icon /> : undefined,
items: [],
nested: item.nested,
Copy link
Contributor

Choose a reason for hiding this comment

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

Here it would be good to log a warning if the item has a nested field
but also has children since they will be ignored.

If you create a nested route for /brand like this:

// /src/admin/routes/brands/page.tsx
import { defineRouteConfig } from "@medusajs/admin-sdk";

const BrandsPage = () => {
  return <div>Brands</div>;
};

export const config = defineRouteConfig({
    label: "Brands",
    nested: "/products",
})

export default BrandsPage;

// src/admin/routes/brands/create/route.tsx
import { defineRouteConfig } from "@medusajs/admin-sdk";

const CreateBrandPage = () => {
  return <div>Create Brand Page</div>;
};

export const config = defineRouteConfig({
    label: "Create Brand", // This would normally generate a nested menu item under "Brands".
})

export default CreateBrandPage;

The nested MenuItem is created, but never used.

So when looping over the items, it would be great to explicitly warn the user why this item is not being used. We need to keep track of items that have a nested field, if we later come across an item which is a child of the item with a nested we should skip it, and warn that we are doing so. A little further up in the function you can see we do something similar for other cases.

Copy link
Contributor Author

@eugenepro2 eugenepro2 Dec 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Good point. But I find out some challange here. The new parameter nested passing only on the first route.
That is means that we cannot check parameter nested on the nested page create. I hope I described it clear)

Possible solution: locate a new route under existing route by path:

// /src/admin/routes/products/brands/page.tsx - located under products route
import { defineRouteConfig } from "@medusajs/admin-sdk";

const BrandsPage = () => {
  return <div>Brands</div>;
};

export const config = defineRouteConfig({
    label: "Brands",
})

export default BrandsPage;

// /src/admin/routes/products/brands/create/page.tsx - will be ignored in sidebar
import { defineRouteConfig } from "@medusajs/admin-sdk";

const CreateBrandPage = () => {
  return <div>Create Brand Page</div>;
};

export const config = defineRouteConfig({
    label: "Create Brand",
})

export default CreateBrandPage;

In this case nested param will be deleted and it would be easy to find parent path.

What do you think about that? Or maybe you have a better solution for that?

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 found a more easy solution. We have menuItems and we can find nested route there. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added warning

}

if (parentPath !== "/" && tempRegistry[parentPath]) {
Expand Down
1 change: 1 addition & 0 deletions packages/admin/dashboard/src/extensions/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export type MenuItemExtension = {
label: string
path: string
icon?: ComponentType
nested?: string
}

export type WidgetExtension = {
Expand Down
26 changes: 26 additions & 0 deletions www/apps/book/app/learn/fundamentals/admin/ui-routes/page.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,32 @@ Some caveats for nested UI routes in the sidebar:
- Nested routes in setting pages aren't shown in the sidebar to follow the admin's design conventions.
- The `icon` configuration is ignored for the sidebar item of nested UI route to follow the admin's design conventions.


### Route Under Existing Admin Route
You can locate a route under an existing route. For example, you can locate a route under the orders route in the sidebar.

```tsx title="src/admin/routes/orders/nested/page.tsx"
import { defineRouteConfig } from "@medusajs/admin-sdk"
import { Container, Heading } from "@medusajs/ui"

const NestedOrdersPage = () => {
return (
<Container className="divide-y p-0">
<div className="flex items-center justify-between px-6 py-4">
<Heading level="h1">Nested Orders Page</Heading>
</div>
</Container>
)
}

export const config = defineRouteConfig({
label: "Nested Orders",
nested: "/orders",
})

export default NestedOrdersPage
```

---

## Create Settings Page
Expand Down