-
Notifications
You must be signed in to change notification settings - Fork 338
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
Adding cursor pagination. Fixes #2155 #2173
Conversation
handlePrev(i: PaginatorCursor) { | ||
i.props.onPrev(); | ||
} | ||
|
||
handleNext(i: PaginatorCursor) { | ||
if (i.props.nextPage) { | ||
i.props.onNext(i.props.nextPage); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: these can be declared as regular functions instead of class methods since you are using linkEvent
.
prevPage?: PaginationCursor; | ||
nextPage?: PaginationCursor; | ||
onNext(val: PaginationCursor): any; | ||
onPrev(): any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't use what gets returned from this (if anything), so it could just return void
.
interface PaginatorCursorProps { | ||
prevPage?: PaginationCursor; | ||
nextPage?: PaginationCursor; | ||
onNext(val: PaginationCursor): any; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we use the return value for this? If not, we could make this return void
as well.
Description
Adds cursor pagination by adding a
pageCursor
in place ofpage
in the home and community urls.Since the API doesn't return a
previous_cursor
, I had to use the back button for a previous, but it works well.This breaks next page comment pagination (the first page shows fine), but we should probably do away with those at some point anyway.