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

[8.0] Add support for manual setting of filtered count (#1516) #1743

Merged
merged 1 commit into from
May 18, 2018

Conversation

patrickwarner
Copy link
Contributor

@patrickwarner patrickwarner commented May 17, 2018

Fix #1516

*
* @return int
*/
public function filteredCount()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should promote this function to the abstract class since this will also be used when using Collection?

@yajra yajra changed the title Add support for manual setting of filtered count (#1516) [8.0] Add support for manual setting of filtered count (#1516) May 17, 2018
@yajra
Copy link
Owner

yajra commented May 17, 2018

The collection dataTable failed the tests. I think moving the function to the abstract will fix the tests. Thanks!

Copy link
Owner

@yajra yajra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Collection tests are failing as the filteredCount method should be available on all engines.

@patrickwarner
Copy link
Contributor Author

I thought that may be a problem. I'll make the changes.

@patrickwarner
Copy link
Contributor Author

I've moved the function to the abstract class and did a bit of clean-up. Please run the tests again :)

@yajra yajra merged commit 071b4a1 into yajra:8.0 May 18, 2018
@yajra
Copy link
Owner

yajra commented May 18, 2018

Released on v8.6.0. BTW, if you can spare some more time. Please also submit a PR for docs on how you used this feature at this repo. Thanks a lot! 🍻

@patrickwarner
Copy link
Contributor Author

Of course! I will submit documentation within the next couple of days.

@yajra yajra added the need docs label Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants