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

Smarter table pagination #1706

Closed
wants to merge 4 commits into from
Closed

Smarter table pagination #1706

wants to merge 4 commits into from

Conversation

benheng
Copy link
Contributor

@benheng benheng commented Feb 7, 2018

Make table pagination more obvious by fetching ahead to see if there is more data. If not, hide the "next" button.

kapture 2018-02-07 at 15 30 45

@andyhuang91
Copy link

Do we want to hide the button or disable it? Also, should we continue to display the current page number?

this.setState({chunkNum, rowChunkSize, sortBy, lastPage});
const lookaheadNum = (chunkNum * rowChunkSize) + 1;
this.setState({ lookahead: true });
return this.props.fetchDataFromApi(lookaheadNum, 1).then((lookaheadResponse) => {

Choose a reason for hiding this comment

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

Is this something that we always want to do for UITable or should this be configurable. You might also want to consider doing both of these requests in parallel and using Promise.all. That might make it unnecessary to separately keep track of the lookahead request state.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it makes sense in the current UITable design since it only applies to tables that have the fetchDataFromApi prop set. Do you foresee tables that have this prop set but won't have a lookahead call?

I had considered running the requests in parallel, but I thought it might be more complicated because the order of completion is undefined. Since the data prop of the UI table is supplied by the parent component and that is supplied by the requestApi object.

@benheng
Copy link
Contributor Author

benheng commented Mar 12, 2018

We're going to opt for better back-end support instead of making two API calls from the front-end.

@benheng benheng closed this Mar 12, 2018
@benheng benheng deleted the pagination branch April 24, 2018 14:52
@benheng benheng restored the pagination branch April 24, 2018 14:52
@benheng benheng deleted the pagination branch April 24, 2018 14:53
@ssalinas ssalinas removed this from the 0.20.0 milestone Jun 11, 2018
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

3 participants