Skip to content

Conversation

@lucaslcode
Copy link

@lucaslcode lucaslcode commented Oct 22, 2021

Hello,

Firstly, thanks for wonderful library! Just the right amount of features while staying small. I also found the code to be very clearly written.

I found that the pagination implementation doesn't work when using filtering and sorting as they don't operate on the original rows. I did quite a large refactor which, for all of sorting, filtering and search, goes through the whole process on the original rows of original -> sorted -> filtered -> searched -> paginated.

It seems to be working correctly for me, but unfortunately I don't have the time right now to look at the tests and making new ones for the cases which were not working before/are now possible.

Combined with the fact this is a huge change, I imagine this PR won't be accepted, but I thought I'd leave it here anyway in case anyone wishes to continue with it or use it in it's current state.

@Buuntu Buuntu requested review from Buuntu and stramel November 3, 2021 17:27
@Buuntu
Copy link
Owner

Buuntu commented Nov 3, 2021

Thanks for putting in all this work, I will take a look at this and let you know what I think. At first glance I agree the PR is huge and would prefer to keep it to small incremental changes.

@codecov-commenter
Copy link

Codecov Report

Merging #48 (b66bc50) into master (083d61d) will decrease coverage by 0.47%.
The diff coverage is 99.09%.

Impacted file tree graph

@@             Coverage Diff             @@
##            master      #48      +/-   ##
===========================================
- Coverage   100.00%   99.52%   -0.48%     
===========================================
  Files            3        3              
  Lines          192      209      +17     
  Branches        41       53      +12     
===========================================
+ Hits           192      208      +16     
- Partials         0        1       +1     
Impacted Files Coverage Δ
src/hooks.ts 99.46% <99.09%> (-0.54%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 083d61d...b66bc50. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants