filamentphp / filament

A collection of beautiful full-stack components for Laravel. The perfect starting point for your next app. Using Livewire, Alpine.js and Tailwind CSS.
https://filamentphp.com
MIT License
18.86k stars 2.93k forks source link

Table not updated when filter is removed via the filter indicator #3794

Closed craigkuhns closed 2 years ago

craigkuhns commented 2 years ago

Package

filament/filament

Package Version

v2.15.38

Laravel Version

v9.27.0

Livewire Version

No response

PHP Version

PHP 8.0.14

Problem description

I have a users model with a boolean 'confirmed' field and I've created a filament resource for it. I added this filter to the table in the UsersResource

->filters([
    Tables\Filters\Filter::make('confirmed')
        ->query(fn($query) => $query->where('confirmed', true)),
])

I enable the filter and the table updates as expected to only show confirmed users. However when I click the 'x' icon in the 'Confirmed' filter indicator, the filter is removed but the table results stay the same until the page is reloaded.

Expected behavior

The table should refresh to show all users and not just confirmed users.

Steps to reproduce

Detailed steps are in the reproduction repository readme

Reproduction repository

https://github.com/craigkuhns/filament-remove-filter-from-indicator-broken

Relevant log output

No response

craigkuhns commented 2 years ago

I dug around a bit and this is what I found. The default state for the checkbox form component is false. So on line 87 of packages/tables/src/Concerns/HasFilters.php when it resets the value, it should be reset too false but instead it is set explicitly to null. Changing line 87 from

is_array($field->getState()) ? [] : null,

to

is_array($field->getState()) ? [] : $field->getDefaultState(),

fixes that issue by resetting the state to the fields default state.

But that alone doesn't fix the issue. The checkbox default state is false. However, on line 51 of packages/tables/src/Filters/Filter.php the default state of the form component is set to the default state of the filter which is null. If you add $this->default(false) to both the toggle() and checkbox() methods of Filter, then when the default state of a checkbox or toggle field component is set it will be set to false instead of null.

I would think that line 98 of packages/tables/src/Concerns/HasFilters.php would also need to be updated to be is_array($field->getState()) ? [] : $field->getDefaultState(), instead of is_array($field->getState()) ? [] : null,.

I'm not sure if those changes would affect other types of filters or filters with custom forms. I tend to think it wouldn't as long as the filter default state is set to false only for checkbox and toggle components, leaving it null for all other components like it is now.

I can make a pull request for these changes if that would be helpful.

zepfietje commented 2 years ago

Thanks for looking further into the issue, @craigkuhns!

Like you said, I'd suggest opening a PR so we can test and discuss the changes more thoroughly. 👍

craigkuhns commented 2 years ago

Thanks, I created a pull request that should fix the issue.