omines / datatables-bundle

DataTables bundle for Symfony
https://omines.github.io/datatables-bundle/
MIT License
251 stars 113 forks source link

added globalSearchable check for ArrayAdapter #341

Closed ToshY closed 1 month ago

ToshY commented 2 months ago

Fixes #340

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.56%. Comparing base (4f3dcb4) to head (6115808).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #341 +/- ## ======================================= Coverage 94.56% 94.56% ======================================= Files 37 37 Lines 975 975 ======================================= Hits 922 922 Misses 53 53 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

ToshY commented 2 months ago

Hey @curry684 👋

Would be nice if you could spare some time to review this.

Thanks in advance.

curry684 commented 1 month ago

Hey @ToshY, I was away on holiday last week. Why did you close the PR, is it wrong or irrelevant?

ToshY commented 1 month ago

Well, I intended to use the bundle for a longer period of time, but after a week of fiddling around with it I realised I don't need most of the functionality/complexitiy that the bundle provides. So I decided I no longer needed to use it and I just could write the minimal code needed to work with the js datatable library.

While the fix worked (tested manually), I'm not sure if this is relevant to other users as I'm guessing the majority is using the other adapters anyway. So if you still think it's relevant, it can be merged. For me however it has become irrelevant.

curry684 commented 1 month ago

I'll merge it anyway if it's tested to be fixed, looks good anyway although the ArrayAdapter is formally unsupported and just for testing/prototyping (in most cases a custom AbstractAdapter is less work, faster and tons more memory efficient).