Allow subscribing to RoomInfo updates
#2411
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2411 +/- ##
==========================================
- Coverage 77.33% 77.29% -0.04%
==========================================
Files 183 183
Lines 19397 19399 +2
==========================================
- Hits 15000 14995 -5
- Misses 4397 4404 +7
☔ View full report in Codecov by Sentry. |
jplatte
force-pushed
the
jplatte/room-info-observable
branch
from
eb0a380 to
0262dd9
Compare
There was a problem hiding this comment.
It looks good. The idea is to get notified when RoomInfo has been updated, but we don't really know which part has been modified. I wonder if that would be desired to avoid unnecessary updates on the client-user-app side? And to clarify: yeah, knowing which part has been modified is difficult.
|
This works well client side but we do get a couple of extra updates whenever we enter a room. I wouldn't expect any updates when that happens 🤔 |
jplatte
force-pushed
the
jplatte/room-info-observable
branch
from
0262dd9 to
666a3f3
Compare
jplatte
force-pushed
the
jplatte/room-info-observable
branch
from
666a3f3 to
17f055a
Compare
jplatte
force-pushed
the
jplatte/room-info-observable
branch
from
17f055a to
e6ab9b9
Compare
| // Look for a local event in the `Timeline`. | ||
| // | ||
| // First off, let's see if a `Timeline` exists… | ||
| if let Some(timeline) = self.timeline.read().await.clone() { | ||
| // If it contains a `latest_event`… | ||
| if let Some(timeline_last_event) = timeline.latest_event().await { | ||
| // If it's a local echo… | ||
| if timeline_last_event.is_local_echo() { | ||
| return Ok(RoomInfo::new( | ||
| &self.inner, | ||
| Some(Arc::new(EventTimelineItem(timeline_last_event))), | ||
| ) | ||
| .await?); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Otherwise, fallback to the classical path. | ||
| let latest_event = match self.inner.latest_event() { |
There was a problem hiding this comment.
This code is duplicated from matrix_sdk_ui::room_list_service::Room::latest_event. Don't you believe we can refactor to get this code in a single-place?
There was a problem hiding this comment.
We might be able to, but I think the latest event feature is going to transform a lot still before it will be done, so it's probably not worth it (especially as there are some small differences).
|
@jplatte Do you think it's just multiple updates, or a bug, in #2411 (comment)? |
|
Sounds like it might be a bug, but hard to tell. Doesn't sound like it's a problem outside of causing a bit of inefficiency though, so we could merge and investigate afterwards, right? |
|
Waiting on feedback from the next test run before merging either way though. |
Not ideal but I'm fine with that. |
Yeah, I agree. It's fine. I'm going to try testing it soon, I promise 😁 |
|
Just tested it and apart from those 2 random updates at the begining it works great. I think we can merge it |
Note to self: Probably should add a changelog entry.