rappasoft / laravel-livewire-tables

A dynamic table component for Laravel Livewire
https://rappasoft.com/docs/laravel-livewire-tables/v2/introduction
MIT License
1.7k stars 320 forks source link

Update ComponentUtilities.php, updated method accept "null" value for second parameter #1663

Closed viliusvsx closed 4 months ago

viliusvsx commented 4 months ago

Sometimes I update variables with null values. Without this PR, I get an error, that I cannot pass the null value in the "updated" method

P.S. I don't think that test is failing because of my PR.

All Submissions:

New Feature Submissions:

  1. [ ] Does your submission pass tests and did you add any new tests needed for your feature?
  2. [x] Did you update all templates (if applicable)?
  3. [x] Did you add the relevant documentation (if applicable)?
  4. [x] Did you test locally to make sure your feature works as intended?

Changes to Core Features:

lrljoe commented 4 months ago

Good point. Let me check the Livewire Core code, but shouldn't be an issue to merge this bit in.

I will be separating this bit out so that it's a bit more maintainable, but that's on my to-do list rather than anything else

Will need to test it, but may make it into the next release, worth noting that you should check if it's "null", before checking it it's equal to "", just to avoid any Exceptions being thrown!

NOTE: Changing base to "develop", as that's where things get merged in

lrljoe commented 4 months ago

Having swiftly reviewed the PR, I think there's a better way, which is to advance a pending update:

So the present chunk of code relating to "search" in the updated() method in ComponentUtilities is replaced with this in "WithSearch":

    public function updatedSearch(string|array|null $value): void
    {
        $this->resetComputedPage();

        // Clear bulk actions on search
        $this->clearSelected();
        $this->setSelectAllDisabled();

        if (is_null($value) || $value === '') {
            $this->clearSearch();
        }
    }

This way, we segregate off the "updated()" logic into the relevant Trait.

Unless you have any objection, I'll use the above approach to get the fix in. Please do reach out on the official Discord if you want to discuss it further!

lrljoe commented 4 months ago

Have created the following PR that uses my above logic: https://github.com/rappasoft/laravel-livewire-tables/pull/1666