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

Add pagination, small refactor and correction #124

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

xileftenurb
Copy link

these are small change in preparation of my feature adding, all
explained here with there rational.

  • FilterItems.ts

refactor of getFiltered and getFilteredOut so they share the same logic
code for filtering. this prevent error code duplication and error going
with it. also added some doc comment to better distinguish both
function.

I also maked these two function a bit purer by moving the searchTerm to
an optional parameter instead of passing by a shared object. this is a
better coding practice and permit to get filtered with a different
search term without changing the options

small refactor of shuffle. there was no reason to call getFiltered
twice, since getFiltered return a new array each time
same for the call of slice(), since a map alway return a new array.
the do-while was followed by a call to "shuffle" in an empty block, so you checked if the
new shuffle is different from the last, but shuffled one last time after
that. I removed it.

the shouldBeFiltered function have a misleading name, depending what a
"filtered" element mean for you (it is the element that are kept, or the
element that are removed?). I refactored it to use typescript "guard"
feature, removing the need of using explicit casting (that is error
prone). I also checked for "all" direcly in it, so we cannot forget to
check for it before calling the function.

  • utils/shuffle.ts

I added typescript generic to keep the type information of the return
type the same as the one of the parameter.
I also corrected a small comment error. array.slice() do a shallow
clone, not a deep clone, and this is what we want.

  • Filterizr.ts and Filterizr.test.ts

updated with the change of parameter of getFiltered

  • tsconfig.json

added "lib" : ["es2016", "esnext", "dom"] to compiler option. this
removed some error found by my VisualStudio code intellisence of element
not found. by default, lib is only es2015+dom, and the library use some
2016 and next feature.

Felix Brunet added 2 commits August 14, 2019 11:29
these are small change in preparation of my feature adding, all
explained here with there rational.

- FilterItems.ts

refactor of `getFiltered` and `getFilteredOut` so they share the same logic
code for filtering. this prevent error code duplication and error going
with it. also added some doc comment to better distinguish both
function.

I also maked these two function a bit purer by moving the searchTerm to
an optional parameter instead of passing by a shared object. this is a
better coding practice and permit to get filtered with a different
search term without changing the options

small refactor of `shuffle`. there was no reason to call `getFiltered`
twice, since `getFiltered` return a new array each time
same for the call of `slice()`, since a map alway return a new array.
the do-while was followed by a call to "shuffle" in an empty block, so you checked if the
new shuffle is different from the last, but shuffled one last time after
that. I removed it.

the `shouldBeFiltered` function have a misleading name, depending what a
"filtered" element mean for you (it is the element that are kept, or the
element that are removed?). I refactored it to use typescript "guard"
feature, removing the need of using explicit casting (that is error
prone). I also checked for "all" direcly in it, so we cannot forget to
check for it before calling the function.

- utils/shuffle.ts

I added typescript generic to keep the type information of the return
type the same as the one of the parameter.
I also corrected a small comment error. array.slice() do a shallow
clone, not a deep clone, and this is what we want.

- Filterizr.ts and Filterizr.test.ts

updated with the change of parameter of getFiltered

- tsconfig.json

added "lib" : ["es2016", "esnext", "dom"] to compiler option. this
removed some error found by my VisualStudio code intellisence of element
not found. by default, lib is only es2015+dom, and the library use some
2016 and next feature.
@xileftenurb xileftenurb changed the title Small refactor and correction Add pagination, small refactor and correction Aug 15, 2019
@xileftenurb
Copy link
Author

A new commit, adding a first version of pagination feature,

I added new test to be sure it work with other feature, and I fixed some test who did not work or where not optimal :

  • Test who use async code need to use the "done" callback to tell the tester when they are finished.
  • Also, the "animationend" event is not triggered in test by lack of proper browser, so we need to trigger it manually.

this should fix #113 once it's done, and be a part of what needed to fix #90.

@SkuterPL
Copy link

SkuterPL commented Jan 6, 2020

@up
Hi,
Do you still work on this future to get full support?

@xileftenurb
Copy link
Author

I did not work on this since my last commit, but I have a project waiting who still need the filter + pagination, so I will have to restart working on this eventually.

@SkuterPL
Copy link

SkuterPL commented Jan 6, 2020

I have one project in which I would like to do pagination, so I could test it for you. When are you going to back to your project?

@tbntdima
Copy link

Hey @xileftenurb, I think a lot of people need pagination in their projects, so I wanted to implement it myself, but then saw your PR (looks pretty cool btw). I have a suggestion, maybe instead of passing category into .filter(category), we can pass a filter function, which will get the array of items, and the user will be able to sort it by himself. This will increase the flexibility 1000 times.
For example, then I will be able to implement pagination by myself, just by cutting items array.
With this we will be able to do almost anything, even do custom soring. What do you think? 🤔

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.

None yet

4 participants