project-robius / robrix

A Matrix chat client written in pure Rust using the Makepad UI toolkit and the Robius app dev framework
MIT License
67 stars 11 forks source link

Maintain stability of the timeline UI view after the ignore user list changes #103

Open kevinaboos opened 1 month ago

kevinaboos commented 1 month ago

Currently, we maintain timeline view stability for live backwards pagination. This is realized by using the timeline diffs to figure out which offset we need to apply to the event indices.

However, this will not work for cases where the ignore user list changes, as that results in the entire timeline being cleared, so there is no offset to calculate from the timeline diffs.

One possible way to implement this is to save the unique ID of one (or more) event(s) that are currently being shown on each timeline, such that we can return the timeline to that point once the full re-pagination has completed. We may also want to store the "focused" state of the timeline -- whether it was focused on that event or in "live" mode (scrolled to the very bottom or very top).

Spun off from a commit comment in #102.

kevinaboos commented 1 month ago

107 addresses this in part, but still doesn't preserve event IDs for the entire duration of this cycle:

  1. ignore user list changes
  2. timelines are fully cleared
  3. we save 1 to 3 event IDs of the first 1 to 3 events visible in the PortalList
  4. we set the timeline into a "mode" where it
  5. the timeline continuously paginates until we observe one of the above saved event IDs
  6. we set the timeline back into its "regular" mode
  7. future updates work as normal, using the first_id logic as before.

We also likely need to track the focus status of the timeline's PortalList:

kevinaboos commented 1 month ago

The TimelineBuilder::with_focus() method implies you can change the mode of an existing Timeline, but I cannot actually find any methods that make this possible, even internal ones within the matrix-sdk....

kevinaboos commented 4 weeks ago

Based on discussions with Rik, the PortalList::scroll_position (the first_scroll field) is not necessarily the correct way to determine what offset a given item in the PortalList is currently scrolled to.

Instead, we should:

  1. get the absolute positional coordinates of the top of the portal list
  2. get the absolute positional coordinates of the top of the item (event) of interest
  3. subtract (2) - (1), and then set the PortalList's scroll position to that difference value, at the same time that we set the first_id to the ID of the event of interest determined in (2).
kevinaboos commented 2 weeks ago

In progress, see #134

kevinaboos commented 1 week ago

134 addresses this for all cases except a complete timeline clear, e.g., after a change in the ignored-user list. But we have to completely re-think how this works by instantiating a brand new Timeline, replacing the existing timeline and its UI-side subscriber, and then focusing the new Timeline on one of the many events that was previously visible before the timeline clear action.