From 08a60336dbf565038c2614762073ccbb6a194f77 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 5 Jun 2024 17:07:54 +0200 Subject: [PATCH] chore(ui): Rewrite a bit `TimelineEventHandler::add`. This patch renames `TimelineEventHandler::add` to `add_new_item`. This patch also removes the `should_add` argument. The `add_new_item` is called conditionally everytime. I believe this is much cleaner. Otherwise the method should have been called `maybe_add_new_item` with a return type or something that indicates whether the item is added. This patch makes it clear and remove one possible state in `add_new_item`. --- .../src/timeline/event_handler.rs | 80 +++++++++++-------- 1 file changed, 48 insertions(+), 32 deletions(-) diff --git a/crates/matrix-sdk-ui/src/timeline/event_handler.rs b/crates/matrix-sdk-ui/src/timeline/event_handler.rs index ee67e1e3cf9..bc884b3d352 100644 --- a/crates/matrix-sdk-ui/src/timeline/event_handler.rs +++ b/crates/matrix-sdk-ui/src/timeline/event_handler.rs @@ -354,12 +354,14 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { self.handle_room_message_edit(re); } AnyMessageLikeEventContent::RoomMessage(c) => { - self.add(should_add, TimelineItemContent::message(c, relations, self.items)); + if should_add { + self.add_new_item(TimelineItemContent::message(c, relations, self.items)); + } } AnyMessageLikeEventContent::RoomEncrypted(c) => { // TODO: Handle replacements if the replaced event is also UTD let cause = UtdCause::determine(raw_event); - self.add(true, TimelineItemContent::unable_to_decrypt(c, cause)); + self.add_new_item(TimelineItemContent::unable_to_decrypt(c, cause)); // Let the hook know that we ran into an unable-to-decrypt that is added to the // timeline. @@ -370,7 +372,9 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { } } AnyMessageLikeEventContent::Sticker(content) => { - self.add(should_add, TimelineItemContent::Sticker(Sticker { content })); + if should_add { + self.add_new_item(TimelineItemContent::Sticker(Sticker { content })); + } } AnyMessageLikeEventContent::UnstablePollStart( UnstablePollStartEventContent::Replacement(c), @@ -381,10 +385,14 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { AnyMessageLikeEventContent::UnstablePollResponse(c) => self.handle_poll_response(c), AnyMessageLikeEventContent::UnstablePollEnd(c) => self.handle_poll_end(c), AnyMessageLikeEventContent::CallInvite(_) => { - self.add(should_add, TimelineItemContent::CallInvite); + if should_add { + self.add_new_item(TimelineItemContent::CallInvite); + } } AnyMessageLikeEventContent::CallNotify(_) => { - self.add(should_add, TimelineItemContent::CallNotify) + if should_add { + self.add_new_item(TimelineItemContent::CallNotify) + } } // TODO @@ -398,7 +406,9 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { TimelineEventKind::RedactedMessage { event_type } => { if event_type != MessageLikeEventType::Reaction { - self.add(should_add, TimelineItemContent::RedactedMessage); + if should_add { + self.add_new_item(TimelineItemContent::RedactedMessage); + } } } @@ -410,28 +420,37 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { } TimelineEventKind::RoomMember { user_id, content, sender } => { - self.add(should_add, TimelineItemContent::room_member(user_id, content, sender)); + if should_add { + self.add_new_item(TimelineItemContent::room_member(user_id, content, sender)); + } } TimelineEventKind::OtherState { state_key, content } => { - self.add( - should_add, - TimelineItemContent::OtherState(OtherState { state_key, content }), - ); + if should_add { + self.add_new_item(TimelineItemContent::OtherState(OtherState { + state_key, + content, + })); + } } TimelineEventKind::FailedToParseMessageLike { event_type, error } => { - self.add( - should_add, - TimelineItemContent::FailedToParseMessageLike { event_type, error }, - ); + if should_add { + self.add_new_item(TimelineItemContent::FailedToParseMessageLike { + event_type, + error, + }); + } } TimelineEventKind::FailedToParseState { event_type, state_key, error } => { - self.add( - should_add, - TimelineItemContent::FailedToParseState { event_type, state_key, error }, - ); + if should_add { + self.add_new_item(TimelineItemContent::FailedToParseState { + event_type, + state_key, + error, + }); + } } } @@ -636,7 +655,10 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { // don't have an event ID that could be referenced by responses yet. self.meta.poll_pending_events.apply(&event_id, &mut poll_state); } - self.add(should_add, TimelineItemContent::Poll(poll_state)); + + if should_add { + self.add_new_item(TimelineItemContent::Poll(poll_state)); + } } fn handle_poll_response(&mut self, c: UnstablePollResponseEventContent) { @@ -845,12 +867,8 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { } } - /// Add a new event item in the timeline. - fn add(&mut self, should_add: bool, content: TimelineItemContent) { - if !should_add { - return; - } - + /// Add a new event item in the timeline if `should_add` is true. + fn add_new_item(&mut self, content: TimelineItemContent) { self.result.item_added = true; let sender = self.ctx.sender.to_owned(); @@ -859,12 +877,10 @@ impl<'a, 'o> TimelineEventHandler<'a, 'o> { let mut reactions = self.pending_reactions().unwrap_or_default(); let kind: EventTimelineItemKind = match &self.ctx.flow { - Flow::Local { txn_id, abort_handle } => { - LocalEventTimelineItem { - send_state: EventSendState::NotSentYet, - transaction_id: txn_id.to_owned(), - abort_handle: abort_handle.clone(), - } + Flow::Local { txn_id, abort_handle } => LocalEventTimelineItem { + send_state: EventSendState::NotSentYet, + transaction_id: txn_id.to_owned(), + abort_handle: abort_handle.clone(), } .into(),