Skip to content

Conversation

@upsuper
Copy link
Contributor

@upsuper upsuper commented Jul 6, 2025

This PR implements automatic pagination for the journal report as a solution to improve performance opening journal page.

At high level, it involves

  • journal.html handles page parameter in request.
    • When it's not set, and the total number of entries is larger than a per-page number, it assign the total number of pages to paginated attribute on the <fava-journal> custom element.
    • When it's set, it slices the entries and only render the given page of the entries.
  • FavaJournal checks if there is paginated attribute 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 asc to desc and 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.

@upsuper
Copy link
Contributor Author

upsuper commented Jul 25, 2025

@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:

  • Utilize some JSON API endpoint (as oppose to paginating on journal.html).
  • Use explicit pagination (i.e. showing page indicators underneath).
  • Use lazy pagination (i.e. only load remaining when scrolling to it).

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.

@deirn
Copy link

deirn commented Jul 26, 2025

I implemented lazy rendering using virtual lists on the client. #2080

@yagebu
Copy link
Member

yagebu commented Jul 27, 2025

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.

I like the high-level approach. Just haven't gotten around to reviewing this fully yet. Will do a high-level review now.

Several alternatives I can think of:

  • Utilize some JSON API endpoint (as oppose to paginating on journal.html).
  • Use explicit pagination (i.e. showing page indicators underneath).
  • Use lazy pagination (i.e. only load remaining when scrolling to it).

I prefer this approach here over explicit or lazy pagination. I guess rendering in the frontend is orthogonal and preferable mid-term.

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 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.

@upsuper upsuper force-pushed the journal-pagination branch from b5b9df9 to 19ef2bf Compare July 27, 2025 12:43
@upsuper upsuper requested a review from yagebu August 1, 2025 13:07
@upsuper
Copy link
Contributor Author

upsuper commented Aug 10, 2025

@yagebu Any chance taking another look? Anything you want me to add before merging?

Copy link
Member

@yagebu yagebu left a comment

Choose a reason for hiding this comment

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

Nice :)

@yagebu yagebu merged commit 1ecf6fb into beancount:main Sep 5, 2025
15 checks passed
@upsuper upsuper deleted the journal-pagination branch September 5, 2025 20:26
@upsuper
Copy link
Contributor Author

upsuper commented Sep 24, 2025

I think I messed up something. Account pages now have the order reversed. I'll look into this.

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.

3 participants