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.

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

Added a minimum threshold for nesting action buttons #5568

Closed
wants to merge 3 commits into from

Conversation

susanu
Copy link
Contributor

@susanu susanu commented Jul 19, 2024

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

@jcastroa87
Copy link
Member

Hello @susanu

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

Thanks again.

Cheers.

@jcastroa87 jcastroa87 requested a review from pxpm July 19, 2024 14:15
@pxpm
Copy link
Contributor

pxpm commented Jul 19, 2024

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
Copy link
Contributor

pxpm commented Sep 18, 2024

Hey @susanu I've created #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 pxpm closed this Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants