-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
✅ [outreachy] Pagination/daphne #5345
✅ [outreachy] Pagination/daphne #5345
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.
Thank you, this is a really good start. A Few comments inlined, 3 more here:
- Please review the CLA and if possible sign it, if you have any questions, ask via slack so we can discuss them with everyone else
- Make sure that you do not introduce unrelated format changes, this makes reviewing your work much harder
- (optional) add a feature that the current page and the number of items per page are added to the URL and read from the url (e.g. ?page=5&itemsPerPage=50)
@Naggayi-Daphne-Pearl look good overall! Can you run |
Thank you @Naggayi-Daphne-Pearl! This looks really good, I consider this as done! |
@Naggayi-Daphne-Pearl thanks for working on this during the outreachy application phase! As discussed the issue with pagination is that we can not solve it on the server side, so while it is visually reducing the number of items seen still the whole amount of items is loaded from the server. We might want to revisit this and think about a server side solution, but for now I will close this PR. |
No description provided.