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

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

Closed
yasin-05 opened this issue Nov 1, 2023 · 7 comments · Fixed by #1511 or #1517
Closed

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

yasin-05 opened this issue Nov 1, 2023 · 7 comments · Fixed by #1511 or #1517
Assignees
Labels
bug Something isn't working Complete Version 3 Version 3 of Package

Comments

@yasin-05
Copy link

yasin-05 commented Nov 1, 2023

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

@yasin-05 yasin-05 added the bug Something isn't working label Nov 1, 2023
@lrljoe
Copy link
Collaborator

lrljoe commented Nov 1, 2023

In your configure() add

$this->setSearchLive();

Does this fix the issue?

@yasin-05
Copy link
Author

yasin-05 commented Nov 1, 2023

@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
Copy link
Collaborator

lrljoe commented Nov 1, 2023

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)

@lrljoe lrljoe added the Awaiting Next Release Currently merged into development awaiting a release to master label Nov 1, 2023
@lrljoe lrljoe self-assigned this Nov 1, 2023
This was linked to pull requests Nov 1, 2023
@yasin-05
Copy link
Author

yasin-05 commented Nov 1, 2023

Awesome work. Thanks! 💙

@dmyers
Copy link
Contributor

dmyers commented Nov 14, 2023

@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
Copy link
Collaborator

lrljoe commented Nov 14, 2023

@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

@lrljoe lrljoe added Complete Version 3 Version 3 of Package and removed Awaiting Next Release Currently merged into development awaiting a release to master labels Nov 26, 2023
@tjhunkin-inhance
Copy link

tjhunkin-inhance commented Dec 12, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Complete Version 3 Version 3 of Package
Projects
Status: Done
4 participants