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

[eugenepro2]

This PR add ability to locate new admin route under existing route in sidebar.

For example, new route Brands

Screen.Recording.2024-12-12.at.16.41.57.mp4

[changeset-bot]

⚠️ No Changeset found

Latest commit: 6eb8ed6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel]

@eugenepro2 is attempting to deploy a commit to the medusajs Team on Vercel.

A member of the Team first needs to authorize it.

[kasperkristensen]

Thanks for the PR @eugenepro2! I have added a few comments, but really great addition which we would love to merge once these are addressed.

[kasperkristensen]

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.

[eugenepro2]

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?

[eugenepro2]

I found a more easy solution. We have menuItems and we can find nested route there. :)

[eugenepro2]

Added warning

[kasperkristensen]

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.

[eugenepro2]

Really good point 🚀. Added type.

[kasperkristensen]

Thanks for making the changes @eugenepro2, only one little thing to update and then this is good to merge 👍

[kasperkristensen]

@eugenepro2 final thing the unit tests are failing because they don't expect the field nested for the route items, so if you could update those then we are good to go

[eugenepro2]

@eugenepro2 final thing the unit tests are failing because they don't expect the field nested for the route items, so if you could update those then we are good to go

Yep, I'll do that.

[kasperkristensen]

LGTM thanks @eugenepro2 🚀

[kasperkristensen]

Sorry @eugenepro2, but one more thing, could I get you to remove the changes to the docs, and put those in a separate PR. Otherwise, the changes will go live a soon as we merge this, before the feature is released.

[eugenepro2]

Sorry @eugenepro2, but one more thing, could I get you to remove the changes to the docs, and put those in a separate PR. Otherwise, the changes will go live a soon as we merge this, before the feature is released.

@kasperkristensen Of course. Please check this PR. I'll create a new PR with updated docs for this feature.

[kasperkristensen]

Sorry @eugenepro2, but one more thing, could I get you to remove the changes to the docs, and put those in a separate PR. Otherwise, the changes will go live a soon as we merge this, before the feature is released.

@kasperkristensen Of course. Please check this PR. I'll create a new PR with updated docs for this feature.

@eugenepro2 There is still one change in a docs file, which blocks the PR. It's a new line character, if you can remove that then I will make sure this gets merged 👍

[eugenepro2]

@kasperkristensen

Sorry @eugenepro2, but one more thing, could I get you to remove the changes to the docs, and put those in a separate PR. Otherwise, the changes will go live a soon as we merge this, before the feature is released.

@kasperkristensen Of course. Please check this PR. I'll create a new PR with updated docs for this feature.

@eugenepro2 There is still one change in a docs file, which blocks the PR. It's a new line character, if you can remove that then I will make sure this gets merged 👍

Oh, Sorry for empty line 😅. Fixed