CRUD icon indicating copy to clipboard operation
CRUD copied to clipboard

Added a minimum threshold for nesting action buttons

Open susanu opened this issue 1 year ago • 2 comments
trafficstars

WHY

If you have the nest action buttons within a dropdown enabled globally, you probably have some cruds with just 1 action button and you don't want a dropdown with just a button.

BEFORE - What was wrong? What was happening before this PR?

No matter the number of action buttons, the links were nested within a dropdown.

AFTER - What is happening after this PR?

Give the ability to disable the dropdown by setting the minimum threshold for buttons.

HOW

How did you achieve that, in technical terms?

Added a minimum threshold and check the number of buttons before nesting.

Is it a breaking change?

No, because the threshold is set to 1 which preserves the current behavior

How can we test the before & after?

Create a CRUD with 2 operations, list and create/update/delete or show

susanu avatar Jul 19 '24 09:07 susanu

Hello @susanu

Thanks a lot for this, i will ask @pxpm to check and merge if everything is ok.

Thanks again.

Cheers.

jcastroa87 avatar Jul 19 '24 14:07 jcastroa87

Hey @susanu thanks a lot for the PR. This is mostly what I had in mind, good job 🙏

I think in terms of implementation there is not much to change here, but like I said I want to keep the possibility of extending this functionality open in the future. One of the things I would like to consider in a future implementation is something in the likes of: lineButtonsAsDropdown**Minimum**Threshold, for situations when you want to force say 2 links to appear on the page, and the rest as a dropdown. Does not matter if entry has 3, 4 ,5 actions, first two appear on page, the rest on the dropdown.

I am thinking in terms of naming mostly, and how we could potentially frame it in a future improvement here to consider both scenarios I've named.

I will discuss this with the rest of the team and get back to you.

Once again, thanks, I think this is a very clean and easy to maintain PR. 🙏

Cheers

pxpm avatar Jul 19 '24 15:07 pxpm

Hey @susanu I've created https://github.com/Laravel-Backpack/CRUD/pull/5667 by pulling this PR and adding the changes I proposed previously.

I think it now is flexible enough for multiple use cases. I've renamed the things a bit here and there. Mind giving your opinion on the PR ? Does it look good to you ?

Thank you very much for pushing this foward 🙏

Cheers

pxpm avatar Sep 18 '24 10:09 pxpm