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.

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

Fix another case of read markers not updating: Ensure proper double linking of TimelineChunks #5564

Merged
merged 1 commit into from Mar 22, 2022

Conversation

SpiritCroc
Copy link
Contributor

We need both directions so getOffsetIndex() produces correct results in
all cases.

Signed-off-by: Tobias Büttner dev@spiritcroc.de

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other :

Content

Ensure that links in both directions are properly set up, so getOffsetIndex() produces correct results.

Motivation and context

I was running in a case where the read marker would not properly update to the most recent of my scrolled-to messages. It turned out that the tracker was thinking of a previously read event to be the most recent scrolled-to one, as LoadTimelineStrategy.getBuiltEventIndex() was returning a lower index than the others. Turned out that the chunk of that event had not linked nextChunk, even though there were more chunks being displayed. This resulted in a wrong result of getOffsetIndex() = 0.

Tests

Scroll around in a room affected by the bug and observe what happens in TimelineViewModel.observeEventDisplayedActions(). Close and re-open the room to see if read marker updated.

Tested devices

  • Physical
  • Emulator
  • OS version(s):

Checklist

We need both directions so getOffsetIndex() produces correct results in
all cases.
@SpiritCroc SpiritCroc force-pushed the timeline-chunk-double-linking branch from e01925d to 91259be Compare March 17, 2022 10:34
@ganfra ganfra self-assigned this Mar 22, 2022
@ganfra ganfra self-requested a review March 22, 2022 14:22
@ganfra
Copy link
Contributor

ganfra commented Mar 22, 2022

Makes sense, thanks for this

@ganfra ganfra merged commit 0734758 into element-hq:develop Mar 22, 2022
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.

None yet

2 participants