rappasoft / laravel-livewire-tables

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

[Bug]: Search param is broken after V3 upgrade #1516

Closed xrezut closed 11 months ago

xrezut commented 11 months ago

What happened?

So after upgrading to V3 of this package, the search no longer works.

One thing I noticed which broke my tests also is the search param for the query is now renamed to table-search. Previously it was search.

Before: http://localhost/admin/accounts?search=test Now: http://localhost/admin/accounts?table-search=test

However, even when updating the query param to the old one from v2 the table data still does not show the correct results. Has something changed in this version ?

The theme i am using is bootstrap-4, and I have not published any views so it should be using those from the package.

My code is as follows:

class AdminTable extends DataTableComponent
{
    protected $model = User::class;

    public function configure(): void
    {
        $this->setPrimaryKey('id');
        $this->setAdditionalSelects('id');
    }

    public function builder(): Builder
    {
        return User::query()->admins();
    }

    public function columns(): array
    {
        return [
            Column::make('Name')
                ->searchable()
                ->sortable(),
            Column::make('Email')
                ->searchable()
                ->sortable(),
        ];
    }
}

How to reproduce the bug

Search the table using the search input field. See the search query param is now "table-search", and the records are not filtered.

Package Version

3.1.0

PHP Version

8.1.x

Laravel Version

10.30.0

Alpine Version

No response

Theme

Bootstrap 4.x

Notes

No response

Error Message

No response

lrljoe commented 11 months ago

In your configure() add

$this->setSearchLive();

Does this fix the issue?

xrezut commented 11 months ago

@lrljoe Thanks a lot for the quick reply, yes that fixes the issue i was facing. I didn't see this mentioned anywhere or in the examples on V3.

For now i could add this to all my tables, however is there another approach for this or is this a known issue that will be fixed in a later version?

lrljoe commented 11 months ago

Having reviewed the v2 behaviour (defaults to live), I'll update v3 to use Live as default.

It's in the development branch now, so should go into master and then into the next release (sometime this week)

xrezut commented 11 months ago

Awesome work. Thanks! 💙

dmyers commented 10 months ago

@lrljoe Could it be possible to make more of the entangled values live? Like selectedItems for example which can be useful for disabling/enabling the select all checkbox if selectedItems.length > 0, but without live I can't seem to get it to work until after the next render or something.

lrljoe commented 10 months ago

@dmyers Hmmm, yep, that's certainly possible, can you open up a separate discussion thread for it, or a feature request issue.

More of them were live originally, but it causes an extra update call on first load which was painful for more complex tables, and that was the easiest fix.

Feel free to jump on the Rappasoft Discord to discuss any ideas etc, as myself and Anthony are both pretty active on there

Select All checkbox for bulk actions should work as unchecked/partial/checked as standard. You can target it with custom css if needed easily enough as an interim

tjhunkin-inhance commented 9 months ago

This still doesn't seem to work on the latest version v3.1.5

EDIT: columns need ->searchable() to be added for it to work