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

[Feature Request]: conditional deselected() #1842

Closed
markebjones opened this issue Aug 13, 2024 · 8 comments · Fixed by #1846
Closed

[Feature Request]: conditional deselected() #1842

markebjones opened this issue Aug 13, 2024 · 8 comments · Fixed by #1846

Comments

@markebjones
Copy link

Overview

I would like to be able to conditionally determine if the column should be deselected, or not, similarly to how I can conditionally determine if the column should be hidden, or not.

Great repo, obviously, thanks so much!

Detailed explanation

My suggestion is to either:

  1. adapt deselected() to support an optional boolean argument that would deselect if true, with a default of true
  2. introduce deselectIf() with a mandatory boolean argument that would deselect if true

Notes

No response

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 13, 2024

Good suggestion.

I'm in the process of actually updating the Column behaviours, so will see if this can be added in at the same time.

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 14, 2024

So, introducing
selectedIf()
and
deselectedIf()

Where you use non-table properties to determine whether or not to select the column, is straight forward.

Making it more complex:
You can't use the Row, as that'll change with each row.
There's no point in passing the Column instance

So are you happy with just a straight forward callback, e.g.

            Column::make('Something', 'description')
              ->deselectedIf(fn() => Auth::user()->can('Do-Something'))
              ->sortable(),

and

            Column::make('Something Else', 'name')
              ->selectedIf(fn() => 1 > 2)
              ->sortable(),

I can't see any use case for any other parameter other than a generic callback

@markebjones
Copy link
Author

It looks great. This is exactly what I'm looking for. Thanks so much!

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 14, 2024

No problem, I'll need to write docs and tests, but should be done by the end of the week

@lrljoe lrljoe linked a pull request Aug 15, 2024 that will close this issue
9 tasks
@lrljoe
Copy link
Collaborator

lrljoe commented Aug 15, 2024

Just linking in the PR, I'll need to double check it all before I can do a release, but it's all on-track as you can see!

@lrljoe
Copy link
Collaborator

lrljoe commented Aug 16, 2024

@markebjones - should be present now as of v3.4.6

@lrljoe lrljoe closed this as completed Aug 16, 2024
@markebjones
Copy link
Author

Great. Thank you. I will test it over the weekend!

@markebjones
Copy link
Author

Seems to work perfectly. Thank you. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants