feat(ui): Reset Timeline when a user is ignored/unignored
#2467
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch updates the `RoomDataProvider` trait to add a new `client` method. This is going to be useful for what's coming next.
This patch updates `TimelineInner::subscribe` as follows: * It's no longer `async`, * It returns `impl Stream` instead of `(Vector<_>, impl Stream)`, * The first item returns by the `Stream` is `VectorDiff::Reset`, which replaces the initial `Vector<_>` from the tuple, * The `Stream` is a “switched stream”, it is reset to `VectorDiff::Reset` every time `BaseClient::ignore_user_list_changes` receives a new value, i.e. every time a user is ignored or unignored, the `Timeline` stream is reset. Consequently, `TimelineInner::subscribe_batched` is no longer `async` too. This propagates to `Timeline` where `subscribe` and `subscribe_batched` are no longer `async` too.
This patch updates `TimelineInner::subscribe` to clear the entire timeline when a user is ignored/unignored.
Hywan
force-pushed
the
feat-base-ignore-user-list-changes-reset-timeline
branch
from
1a9b926 to
301d8fd
Compare
Hywan
force-pushed
the
feat-base-ignore-user-list-changes-reset-timeline
branch
from
d4d924c to
95e0414
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2467 +/- ##
==========================================
+ Coverage 77.33% 77.39% +0.05%
==========================================
Files 184 184
Lines 19447 19456 +9
==========================================
+ Hits 15039 15057 +18
+ Misses 4408 4399 -9
☔ View full report in Codecov by Sentry. |
|
@jplatte Ready to review! |
Hywan
force-pushed
the
feat-base-ignore-user-list-changes-reset-timeline
branch
from
138221c to
2a6aae2
Compare
…rite`. This patch splits the `TimelineInnerStateLock::lock` method into `::read` and `::write`. The idea is that a read-only lock doesn't hold a clone of the lock release observer (`lock_release_ob: SharedObservable<()>`), it will not notify the observer. Then it's only the write lock that holds a clone of the lock release observer, and will notify it. This patch updates the code accordingly as best as possible.
Hywan
force-pushed
the
feat-base-ignore-user-list-changes-reset-timeline
branch
from
2a6aae2 to
6278bc0
Compare
jplatte
requested changes
Aug 31, 2023
Comment on lines
+153
to
+181
| let state = self.state.clone(); | ||
| let ignore_user_list_stream = self.client.subscribe_to_ignore_user_list_changes(); | ||
|
|
||
| stream! { | ||
| pin_mut!(ignore_user_list_stream); | ||
|
|
||
| loop { | ||
| let (timeline_items, timeline_stream) = { | ||
| let state = state.read().await; | ||
|
|
||
| (state.items.clone(), state.items.subscribe()) | ||
| }; | ||
|
|
||
| yield stream::once( | ||
| // Reset the stream with all its items. | ||
| ready(VectorDiff::Reset { values: timeline_items } ), | ||
| ) | ||
| .chain( | ||
| timeline_stream | ||
| ); | ||
|
|
||
| // When this is fired, let's loop. | ||
| let _ = ignore_user_list_stream.next().await; | ||
|
|
||
| // A user has been ignored/unignored? Let's clear the timeline. | ||
| state.write().await.clear(); | ||
| } | ||
| } | ||
| .switch() |
There was a problem hiding this comment.
I don't think this should be part of the "inner" bits of the timeline. Can you revert all the changes in this file and the unit tests please, and move the listening for the ignore user list changes one level higher, into the timeline module? Then we don't need to touch the RoomDataProvider trait either. (also, I think the RwLock thing should be a separate PR, if we want to do it at all – if we do the eyeball-im changes discussed today we probably don't need it)
|
This has been implemented in a different way in #2504. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR can be reviewed commit-by-commit for more comfort.
The global idea is that the
Timelineis cleared when a user is ignored/unignored.This PR also splits
TimelineInnerState::lockto::readand::write, and the lock release observer is notified only for the write lock.