Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(ui): Pagination clean up in the timeline #3355

Closed
Hywan opened this issue Apr 25, 2024 · 1 comment · Fixed by #3373
Closed

fix(ui): Pagination clean up in the timeline #3355

Hywan opened this issue Apr 25, 2024 · 1 comment · Fixed by #3373
Assignees
@bnjbvr

Comments

@Hywan
Copy link
Member

Hywan commented Apr 25, 2024

In #3329, there is 2 small problems that must be addressed:

  • When a pagination is cancelled, the status isn't reset. Typically, it starts with Idle, then a pagination is fired, the status becomes Paginating, then it's cancelled, the status stays Paginating. When restarting the pagination, it returns immediately because the system believes it's already paginating. We must address that.
  • The pagination code seems partially duplicated between timeline::pagination and event_cache::paginator. We must reconcile both.
@bnjbvr bnjbvr self-assigned this Apr 29, 2024
@bnjbvr
Copy link
Member

bnjbvr commented Apr 29, 2024

Thanks for opening the follow up.

This was referenced Apr 29, 2024
Merged
Merged
bnjbvr added a commit that referenced this issue May 1, 2024
@bnjbvr
event cache: reuse the paginator internally
0c59fc6 
Fixes #3355.
bnjbvr added a commit that referenced this issue May 1, 2024
@bnjbvr
event cache: reuse the paginator internally
28cd3c4 
Fixes #3355.
@bnjbvr bnjbvr mentioned this issue May 1, 2024
Merged
bnjbvr added a commit that referenced this issue May 2, 2024
@bnjbvr
event cache: reuse the paginator internally
48bc7fe 
Fixes #3355.
bnjbvr added a commit that referenced this issue May 16, 2024
@bnjbvr
event cache/timeline: reuse the Paginator when running back-paginat…
c8f6fe4 
…ions (#3373)

* event cache: reuse the paginator internally

Fixes #3355.

* event cache: move the `pagination_token_notifier` into the `RoomPaginationData` as well

* event cache: introduce a `RoomPagination` API object and move code around

Only code motion. No changes in functionality.

* event cache: remove "paginate" (et al.) in `RoomPagination` method names

No changes in functionality, just renamings.

* event_cache/timeline: have the event cache handle restarting a back-pagination that failed under our feet

When a timeline reset happens while we're back-paginating, the event
cache method to run back pagination would return an success result
indicating that the pagination token disappeared. After thinking about
it, it's not the best API in the world; ideally, the backpagination
mechanism would restart automatically.

Now, this was handled in the timeline before, and the reason it was
handled there was because it was possible to back-paginate and ask for a
certain number of events. I've removed that feature, so that
back-pagination on a live timeline matches the capabilities of a
focused-timeline back-pagination: one can only ask for a given number of
*events*, not timeline items.

As a matter of fact, this simplifies the code a lot by removing many
data structures, that were also exposed (and unused, since recent
changes) in the FFI layer.

* Address review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bnjbvr bnjbvr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

event cache/timeline: reuse the Paginator when running back-paginations matrix-org/matrix-rust-sdk
3 participants
@Hywan @bnjbvr and others