-
Notifications
You must be signed in to change notification settings - Fork 80
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
Fix RouteLayout Pagination #995
Conversation
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.
Pagination works great! 👍
<div> | ||
<Pagination | ||
bsSize='small' | ||
// onSelect={(e) => console.log('yoyo')} |
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.
yoyo
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.
Oooops!
<Pagination.Item | ||
active={pageNum === activePage} | ||
key={pageNum} | ||
onClick={(e) => this._onPaginationSelect(+e.target.text)} |
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.
onClick={(e) => this._onPaginationSelect(+e.target.text)} | |
onClick={() => this._onPaginationSelect(pageNum)} |
I'm not too familiar with what +e.target.text
does. Maybe this is more readable?
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.
e
is just a general shorthand for the event in the event handler, which in this case is the onClick event that happens when you click on the pagination menu. We need to use that to retrieve the page number that is clicked, otherwise pageNum would pass the current page number. e.target.text
is just how we can access the text (page number) that was clicked.
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.
Nice fix!
Checklist
dev
before they can be merged tomaster
)Description
Fix the pagination in the RouteLayout component, which has been the topic of some recent complaints.