-
Notifications
You must be signed in to change notification settings - Fork 326
implement automatic journal pagination #2058
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
Conversation
|
@yagebu Would you mind taking a look at the high level approach? If you think this is not going towards the right direction, I can definitely try a different approach. Several alternatives I can think of:
Also it may be useful to take into account the sorting preference. But I think that would significantly increased the complexity without adding much value, because majority of people would likely just default to sorting by date in descending order. |
|
I implemented lazy rendering using virtual lists on the client. #2080 |
I like the high-level approach. Just haven't gotten around to reviewing this fully yet. Will do a high-level review now.
I prefer this approach here over explicit or lazy pagination. I guess rendering in the frontend is orthogonal and preferable mid-term.
I think the sorting order should be taken into account but only doing so for date-sorting is fine. Not sure what the majority preference is actually. |
b5b9df9 to
19ef2bf
Compare
|
@yagebu Any chance taking another look? Anything you want me to add before merging? |
yagebu
left a comment
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 :)
|
I think I messed up something. Account pages now have the order reversed. I'll look into this. |
This PR implements automatic pagination for the journal report as a solution to improve performance opening journal page.
At high level, it involves
journal.htmlhandlespageparameter in request.paginatedattribute on the<fava-journal>custom element.FavaJournalchecks if there ispaginatedattribute set, it automatically starts fetching all the remaining pages and append them to the<ol>.With pagination, it's desirable for entries that show up first to be loaded first. Journal table currently renders in date ascendant but the default and most commonly useful order is date descendant, so this PR also changes the rendering order from
asctodescand negate the sort value.1000 is used as the fixed per-page number because based on my local testing, it is a reasonable balance between initial delay vs. number of pages. It could potentially go up to 2,000.