feat(timeline): add a virtual start of timeline item

[bnjbvr]

Might be better than letting the apps figure out whether they need to put one or not.

The next step would be to add a virtuel item for gaps as well, Gap { loading: bool }:

1. this would pave the way for inserting gaps at random positions in the timeline,
2. when the timeline is back-paginating, we'd have a gap with loading: true
3. when the timeline is idle and we suspect there might be an history we haven't loaded, we'd have a gap with loading: false. The next call to Timeline::paginate_backwards() would resolve it into a TimelineStart or another Gap { loading: false }.

Part of #4821.

[codecov]

Codecov Report

Attention: Patch coverage is 82.45614% with 10 lines in your changes missing coverage. Please review.

Project coverage is 86.45%. Comparing base (b83889d) to head (35f6a7e).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/matrix-sdk-ui/src/timeline/date_dividers.rs	41.66%	7 Missing ⚠️
...dk-ui/src/timeline/controller/state_transaction.rs	60.00%	2 Missing ⚠️
...rates/matrix-sdk-ui/src/timeline/controller/mod.rs	85.71%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4816   +/-   ##
=======================================
  Coverage   86.45%   86.45%           
=======================================
  Files         297      297           
  Lines       34616    34636   +20     
=======================================
+ Hits        29928    29946   +18     
- Misses       4688     4690    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Hywan]

Nice! I like having this marker. One note about a loop that can be simplified. Otherwise everything else is good.

One note while reading your code (it's NOT something I'm asking you to do, I can do it, just wanted to share my idea). I wonder if ObservableItems shouldn't get a method to push or insert specific timeline items, so that we move the logic of where to insert or shift index in there instead of having it inside the EventHandler:

• insert_timeline_start: will emit a PushFront with a TimelineStart
• insert_remote_timeline_item(index, item): depending of index and the presence of some items, it will emit a PushFront, Insert or PushBack
• insert_local_timeline_item: always push at the end etc.

The idea is to get a cleaner API and a central place where to have all the invariants regarding the position of the items. Thoughts?

[Hywan]

Can we have a VirtualTimelineItem::TimelineStart at another position than the first position? I don' think so.
Consequently, this loop is unecessary (and even dangerous as we will loop over all items if there is no TimelineStart):

if self.items.get(0).is_some_and(|item| item.is_timeline_start()) {
    return;
}

txn.items.push_front(…);
txn.commit();

[bnjbvr]

I was worried that the date separator or the read marker could go before, but maybe you're right and I could add a test that a date separator can't be first.

At least we can stop iterating on the first non virtual item, otherwise

[Hywan]

You're checking the first item in the EventHandler. Be sure to be consistent.

[bnjbvr]

Yeah, let's keep it simple and only check the first item. I think it's fine, because we're inserting read markers and date dividers before event items, only, so if the timeline start has been inserted, it should always remain the first item.

[bnjbvr]

One note while reading your code (it's NOT something I'm asking you to do, I can do it, just wanted to share my idea). I wonder if ObservableItems shouldn't get a method to push or insert specific timeline items, so that we move the logic of where to insert or shift index in there instead of having it inside the EventHandler:

* `insert_timeline_start`: will emit a `PushFront` with a `TimelineStart`

* `insert_remote_timeline_item(index, item)`: depending of `index` and the presence of some items, it will emit a `PushFront`, `Insert` or `PushBack`

* `insert_local_timeline_item`: always push at the end etc.

The idea is to get a cleaner API and a central place where to have all the invariants regarding the position of the items. Thoughts?

I think that makes sense overall, although I'm not quite sure what it buys us. More concise names would be sweet, like prepend_item / insert_item / push_item.

Overall, I think the most valuable part would be to make it possible in the tests to assert against somewhat different expectations, using one of the macros that asserts what's the next timeline update is: e.g. asserting PushBack against a Insert(index: vector.length(), value) would be accepted, and conversely (ditto for Insert(0) vs PushFront). This would help testing the timeline in a much more robust manner.

[Hywan]

Good to me. Thanks for the changes!