Skip to content

fix: render divider and header as li in menu#4763

Merged
apedroferreira merged 12 commits intomui:masterfrom
rkristelijn:feature/render-divider-as-li-in-menu-and-header-too
Mar 19, 2025
Merged

fix: render divider and header as li in menu#4763
apedroferreira merged 12 commits intomui:masterfrom
rkristelijn:feature/render-divider-as-li-in-menu-and-header-too

Conversation

@rkristelijn
Copy link
Contributor

@rkristelijn rkristelijn commented Mar 14, 2025

  • I've read and followed the contributing guide on how to create great pull requests.
  • I've updated the relevant documentation for any new or updated feature.
  • I've linked relevant GitHub issue with "Closes fix: render divider as li in menu #4762 ".
  • I've added a visual demonstration in the form of a screenshot or video.

Closes #4759

Copy link
Collaborator

@apedroferreira apedroferreira left a comment

Choose a reason for hiding this comment

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

Hi, you're right, thanks a lot for finding these issues and working on the fixes!
All good, just a small change in the order of props to be consistent.

@mui-bot
Copy link

mui-bot commented Mar 19, 2025

Netlify deploy preview

https://deploy-preview-4763--mui-toolpad-docs.netlify.app/

Generated by 🚫 dangerJS against d96a68d

@apedroferreira
Copy link
Collaborator

apedroferreira commented Mar 19, 2025

There's a failing CI check, sorry, can you please run pnpm prettier:all?
I also just added a commit to rebase on the master branch so be sure to pull that commit before adding changes.
Other than that hopefully everything should be passing! Thanks again!

… github.com:rkristelijn/toolpad into feature/render-divider-as-li-in-menu-and-header-too
@rkristelijn
Copy link
Contributor Author

rkristelijn commented Mar 19, 2025

I've ran npx pnpm prettier:all

Weirdly enough there is no pnpm dependency, so i ran against the latest which is 9.12.3.

❯ npm ls pnpm
[email protected] /path/to/toolpad
└── (empty)

❯ npx pnpm --version
9.12.3

I would recommend to either:

  1. add to README.md to use corepack enable && corepack prepare pnpm --activate
  2. add in .npmrc: [email protected] or other version
  3. (my preference) add the right version of pnpm as a dev dependency that can be used via npx (first uses local installs, then globals)

@apedroferreira apedroferreira merged commit ed2360c into mui:master Mar 19, 2025
15 checks passed
@apedroferreira
Copy link
Collaborator

I would recommend to either:

  1. add to README.md to use corepack enable && corepack prepare pnpm --activate
  2. add in .npmrc: [email protected] or other version
  3. (my preference) add the right version of pnpm as a dev dependency that can be used via npx (first uses local installs, then globals)

Yes, looks like we could improve the documentation about suggestion 1 at least. Feel free to even submit a PR with that or I can do it soon!

Other than that we do have "packageManager": "[email protected]" in the root package.json, and it doesn't seem like we're following number 3 organization-wide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: toolpad-core Abbreviated to "core"

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DashboardLayout renders inaccessible menu for divider

5 participants