MenuCRUD
MenuCRUD copied to clipboard
Revamp MenuCRUD
First of all,
this is a major and breaking changes. Please take your time to check thoroughly or just come back later if you got time to inspect, since this is not a major bug or functionality issue fixes.
The way the current MenuCRUD works is to just manage the menu item data. However in the real world website usage, we need more than that, for example multiple menu tree, multiple placement, etc.
I've changed the MenuCRUD functionality to similar to other CMS such as Wordpress styled menu builder. Where we can create multiple menus, assign them placement / location, and easily render them in the frontend / HTML / blade files.
Also in this add on, the menu tree builder (the getTree
method), only supports until 2 children / nested tree. I've changed the getTree
method to be able to build the tree in basically infinite children. The new function is using reference method to increase the performance rather than recursive method. CMIIW.
Source: https://stackoverflow.com/questions/2915748/convert-a-series-of-parent-child-relationships-into-a-hierarchical-tree/34087813#34087813
I've also updated the README.md
, hopefully I explain clear enough.
Please also check the service provider, since I've never made a laravel / composer plugin, I'm not sure if I wrote it correctly. 😅 https://github.com/Laravel-Backpack/MenuCRUD/compare/master...mahasadhu:mahasadhu/revamp-menu?expand=1#diff-9c65494514cffe4a90df1778b6da27a28c95a97f6dbb4fb438d056070e4eadef
This PR is still not perfect, for example, we cannot assign a single menu to multiple placement, no active menu detection method, etc. Basically I'm just copy pasting the most basic of what we usually do in our other projects using Backpack.
I'm open to suggestion, just tell me what you guys think, either this PR needs more features / functionalities, too complex, or maybe this should be in a separate add on.
Thanks for checking 🍻 😃
BOOM! Your first PR with us, thank you so much! Someone will take a look at it shortly.
Please keep in mind that:
- if this constitutes a breaking change, it might take quite a while for this to get merged; we try to emulate the Laravel release cycle as much as possible, so developers can upgrade both software once; this means a new big release every ~6 months;
- even if it's a non-breaking change, it might take a few days/weeks for the PR to get merged; unless it's a no-brainer, we like to have some community feedback on new features, before we merge them; this leads to higher-quality code, in the end; we learnt this the hard way :-)
- not all PRs get merged; sometimes we just have to hold out new features, to keep the packages lean; sometimes we don't include features that only apply to niche use cases;
- we're not perfect; if you think we're wrong, call us out on it; but in a kind way :-) we all make mistakes, best we learn from them and build better software together;
Thank you!
-- Justin Case The Backpack Robot
Nice work here @mahasadhu !
I agree - this is quite a big change in scope... so we should be careful about merging this. Happy to take a look and help guide this into something a lot of devs will use - either inside MenuCRUD or inside its own package, not really sure at this point.
I also agree that MenuCRUD is quite limited in terms of functionality. It only lets you edit one menu. Which is not enough in a lot of websites / web apps. Its purpose from the start wasn't to offer a general-purpose solution, but to scratch a personal itch and to provide a way to show off and test the Reorder
functionality in Backpack. I've always thought that maybe we create a package that does more, something like MultipleMenuCRUD
. That way:
- people who need just one menu install
MenuCRUD
- people who need multiple menus install
MultipleMenuCRUD
Otherwise, if you only have one menu and install an add-on that has multiple menus but you can't use them... it would make for a bad UX for the admin I think...
How's the interface? Can you show us a few quick screenshots of how it would look&feel from the admin's point of view?
Cheers!
@tabacitu - Thank you for checking, here's the screen record for the interface, hope you guys understand my mouse pointer gesture 😅
https://user-images.githubusercontent.com/13914485/103606383-31b7b380-4f51-11eb-9e27-332e0ab7f5ad.mp4
And here is the example for the current active page, I just added a "(current)"
string to the current page's menu label
https://user-images.githubusercontent.com/13914485/103606457-6a578d00-4f51-11eb-9b48-6b46e085765c.mp4
I like it a lot @mahasadhu!
I agree with @tabacitu, most of the people use a single menu, and having an UI for multiple is kinda bad UX. But, I don't like the idea of having 2 packages for the Menu, it would be nice if developers installed only one package and could "opt-in" for multiple menus or not. (I don't know how to do it 😅, just saying).
Thank you for the PR and for the videos, we'll take a better look at this on v4.2, I'm adding this there for now ✌
hi @promatik , thank you for checking
I see, hmm using my current PR, I think I'll just need to insert a default Menu
in the migration file. Using that default menu, backpack user which only need one Menu
can just manage the Menu Items
in that default Menu
.
So when the single_menu
option in config
is true
, backpack user can only access the Menu Items
CRUD in that default Menu
. Otherwise, user can edit both Menu
and Menu Items
This Menu
and Menu Items
is quite mouthful, I hope you understand what I'm trying to explain 🤣
By the way is there any way to make Backpack operations optional? Using if
maybe?
@mahasadhu I think there is no way to make an Operation optional. You always load the Operation. Inside the operation you may have conditions to load your content or not;
In my case I had an Operation for creating dummy entries, where it added a button to the list view only if the model supports factories, so the condition is on the setup; https://github.com/promatik/create-dummy-operation-for-backpack/blob/master/src/Http/Controllers/Operations/CreateDummyOperation.php#L37
@promatik ah I see, the good ol reflection way 😆
Thank you for the insight, will come back later when I managed to make the multi menu as opt-in
By the way @promatik could you please check this? https://github.com/Laravel-Backpack/CRUD/issues/3457
Hey @mahasadhu ,
Unfortunately we've decided not to do this in the current package. It's a big change, basically adding another layer - you don't have ONE menu, but MULTIPLE menus. Which is something most projects won't need. So it adds complexity for like 80% of the projects... that they wouldn't need.
I do have projects that COULD use this, and SHOULD use this though. So I'd definitely use this package if it existed. What do you think about creating an add-on for this, separate from MenuCRUD? Something like mahasadhu/multiple-menu-crud
? It's pretty easy to do that using our tutorial here, and if you do create and maintain it, we could promote it to the Backpack community.
Let us know if you want to build it. If not, maybe someone else in the community will do it. Sorry for the bad news. Cheers!