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

feat(pagination): add async pagination #1237

Closed

Conversation

TheSharpieOne
Copy link
Contributor

@TheSharpieOne TheSharpieOne commented Sep 21, 2016

This checks the first box in this list for v1.0.0 and closes #644

This adds a new pagination prop to the Select.Aync component. It is a boolean which is false by default (false: things act as they did before it was added). When pagination is true loadOptions is given an additional argument; page.
page tells the user's loadOptions function which page to load (they can use to when building their request). Additional page results are appended to the options list. Changing the input value will reset the page back to 1.
loadOptions is called when the user scrolls to the bottom of the menu list, trigger the next page of results to be appended.
Caching is used! (Cache all the things!). If a user scrolls down enough to load additional pages, then changes the input (triggering new requests), then enters the original input, all of the pages for the original input will be loaded back and when the user scroll to the bottom again, it will pick up on the page it left off on.

Documentation has been updated to reflect the new prop and how it affects loadOptions' signature.

EDIT:
async-pagination

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 93.271% when pulling 1c8f2df on TheSharpieOne:feature/async-pagination into 2b14448 on JedWatson:master.

@TheSharpieOne TheSharpieOne mentioned this pull request Sep 21, 2016
@syndbg
Copy link

syndbg commented Oct 3, 2016

A separate callback prop pageChange would be super useful and needed if a developer would like to use debouncing on input change (currently loadOptions) and no debounce on pageChange.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 92.848% when pulling f34d3e4 on TheSharpieOne:feature/async-pagination into b3db918 on JedWatson:master.

@TheSharpieOne
Copy link
Contributor Author

Rebased to fix conflicts. @JedWatson or someone please review before it conflicts again or just gets lost in the pile. This feature is on the roadmap to v1.

@syndbg you can use the page argument to determine debounce or not to debounce. For instance, say you have a function which fetches the options, we'll call it fetchOptions, and you use something like lodash's debounce to get a new function, we'll call it waitnFetchOptions (this is pretty standard so far for a typical debounce setup). You can have a third function which is passed the Select.Aync as the loadOptions props which, based on the page argument calls waitnFetchOptions or fetchOptions like so:

function loadOptions (inputValue, page, callback) {
  if (page > 1) {
    fetchOptions(inputValue, page, callback);
  } else {
    waitnFetchOptions(inputValue, page, callback);
  }
}

It's a simple function, and since debouncing has been left to the user of the library to implement (in user-land), It is probably best to leave this in user-land as well.

@lucaas
Copy link

lucaas commented Oct 13, 2016

Thank you for your contribution, we would love pagination support for react-select.

Our API works with an offset parameter which is returned with the payload (as next_offset). This can not be simply calculated from a page argument (offset = page * pageSize), as items may be culled from the returned result.

We would like to save the next_offset value, and pass that in to loadOptions. Could this use case be supported?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 92.516% when pulling ab26c5a on TheSharpieOne:feature/async-pagination into 4487ca7 on JedWatson:master.

@TheSharpieOne
Copy link
Contributor Author

Rebased once again to remove conflicts....

@lucaas that is a pretty specific use case. The problem is, react-select's async doesn't know anything about the call or the response, only the options you pass to it. Meaning your code handles making the call and parsing the response. You could technically store that value in your code.

I do see this use case in things like AWS's dynamodb, where all you get is a pointer to where it left off from last time and thus, where to start from in the next query/page.

I'm not 100% sure what a clean implementation for this would look like. The callback does currently accept an object. It only looks for and uses the options key/property in the object. Another property could be passed in that object which would just be returned/sent the next time loadOptions is called. That value would need to be stored in the component's cache (if enabled) so it can be picked up where it left off when the input changes and changes back.

Honestly, after almost a month and 2 rebases to resolve conflicts, I don't think this feature will get merged anyways, but I'd love for some input from a collaborator about adding the offset/pointer.

@TheSharpieOne
Copy link
Contributor Author

Instead of me trying to keep this PR from conflicting by rebasing all of the time just for it to conflict again after another week of sitting here... Just let me know when you are ready for this feature and I will rebase then.

@syndbg
Copy link

syndbg commented Oct 24, 2016

@JedWatson can you take a look at this?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 92.535% when pulling f507725 on TheSharpieOne:feature/async-pagination into a8f4118 on JedWatson:master.

@LordSputnik
Copy link

@JedWatson this is something we'd really like to see in react-select too, let me know if there's anything that can be done to help get it in!

@dziulius
Copy link

dziulius commented Dec 2, 2016

Just merge this already. It's really annoying to use repo from github and maintain it by my self.

@rodrigoapereira
Copy link

@JedWatson Is there anything still pending for this to be merged?

We are using this, and this is working fine.

@nancruz
Copy link

nancruz commented Mar 10, 2017

@JedWatson My team, at work, has been waiting for a while for this feature. Is there anything holding you back from merging this feature?

@syndbg
Copy link

syndbg commented Mar 12, 2017

@JedWatson It's about time to make something regarding this PR. How can we help to get this merged?

@nivek91
Copy link

nivek91 commented Mar 31, 2017

@JedWatson anything I can do to help? Thanks

@nivek91
Copy link

nivek91 commented Mar 31, 2017

@TheSharpieOne I've testing your example and it doesn't work to me. When I get to the latest element, the second page is not being requested (just cloned your repo)

image

@TheSharpieOne
Copy link
Contributor Author

Here is what I see
async-pagination

Can you show what you are doing?

@nivek91
Copy link

nivek91 commented Mar 31, 2017

I'm on windows, branch feature/async of your repo, doing the same..
vid

@TheSharpieOne
Copy link
Contributor Author

TheSharpieOne commented Mar 31, 2017

I am on mac, branch feature/async-pagination of my repo (but I believe that is the branch you meant)
It looks like windows is probably not firing off the scroll bottom trigger. I don't have a windows machine right now. Can you drop a breakpoint here and see if it get triggered or if something funky is going with the values that determine if the menu is at the bottom or not.

@nivek91
Copy link

nivek91 commented Mar 31, 2017

yes, thats the branch, here is the breapoint:

if you want I can share my screen to troubleshoot it:
bottom

@TheSharpieOne
Copy link
Contributor Author

Definitely looks like something funky with the numbers not adding up.
This was/is code that is already in the codebase and is not part of my changes. That doesn't mean I can't fix it. Probably just need to do some rounding.

@xizi-xu
Copy link

xizi-xu commented Feb 6, 2018

Any update on this?

@louisemccomiskey
Copy link

Hi I'm also wondering if there is an update to when this will be ready please?

@TimNZ
Copy link

TimNZ commented Mar 1, 2018

@JedWatson can you spare 30 seconds illuminating us why you haven't merged this, or even engaging with the conversation?

@horaciosystem
Copy link

Hi @JedWatson, we all miss you. Could you please take a look on that, there's a lot of people waiting for it.

@ptahchiev
Copy link

+1 for this - I'd really like to see this merged :)

@paulocesarpcfj
Copy link

Please, could you merge this PR? Its really important to have this.

@daino3
Copy link

daino3 commented May 9, 2018

Would reallllllllly love this PR. Can we please get some attention on this? We're currently trying emulate this behavior with our own state / getOptions function which is just future tech-debt...

🙏 📿

@kenzouno1
Copy link

Please update this.

@nafg
Copy link
Contributor

nafg commented Jun 12, 2018

https://github.com/vtaits/react-select-async-paginate works

@rghose
Copy link

rghose commented Nov 27, 2018

Wow its been two years now! @TheSharpieOne seems like an issue with deploy..

@dbenchi
Copy link

dbenchi commented Dec 21, 2018

Can I know please why we do not have this PR merged?
The main question is: do we have a performance problem when mixing pagination with dropdown?

@TheSharpieOne
Copy link
Contributor Author

Closing this as at this point it never going to get merged. I'll reopen if a maintainer chimes in stating otherwise.

@ap47
Copy link

ap47 commented Jun 19, 2019

Reopen this!!!

@TheSharpieOne
Copy link
Contributor Author

development has moved on. This was for pre-1.0, it's now on 3.0 with a complete rewrite. The PR was open for over 2 years, it's never going to merge.

give https://github.com/vtaits/react-select-async-paginate a try.

@phazei
Copy link

phazei commented Jun 6, 2024

Pretty sad that it's been asked for so much and there was so much work put into this PR and @JedWatson never even bothered saying anything about it and here we are years later and it's still not a feature.

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.

[Question] Load on scroll/pagination